Merge pull request #840 from rsteube/traverse-fix-shorthandchain

traverse: fix shorthand chain

defaultActions_test.go

@@ -39,7 +39,7 @@ func TestActionFlags(t *testing.T) {
 
 	cmd.Flag("alpha").Changed = true
 	a := actionFlags(cmd).Invoke(Context{Value: "-a"})
-	assertEqual(t, ActionValuesDescribed("b", "", "h", "help for this command").Tag("flags").NoSpace().Invoke(Context{}).Prefix("-a"), a)
+	assertEqual(t, ActionValuesDescribed("b", "", "h", "help for this command").Tag("flags").NoSpace('b', 'h').Invoke(Context{}).Prefix("-a"), a)
 }
 
 func TestActionExecCommandEnv(t *testing.T) {

example/cmd/chain.go

@@ -0,0 +1,45 @@
+package cmd
+
+import (
+	"github.com/rsteube/carapace"
+	"github.com/spf13/cobra"
+)
+
+var chainCmd = &cobra.Command{
+	Use:                "chain",
+	Short:              "shorthand chain",
+	Run:                func(cmd *cobra.Command, args []string) {},
+	DisableFlagParsing: true,
+}
+
+func init() {
+	carapace.Gen(chainCmd).Standalone()
+
+	rootCmd.AddCommand(chainCmd)
+
+	carapace.Gen(chainCmd).PositionalAnyCompletion(
+		carapace.ActionCallback(func(c carapace.Context) carapace.Action {
+			cmd := &cobra.Command{}
+			carapace.Gen(cmd).Standalone()
+
+			cmd.Flags().CountP("count", "c", "")
+			cmd.Flags().BoolP("bool", "b", false, "")
+			cmd.Flags().StringP("value", "v", "", "")
+			cmd.Flags().StringP("optarg", "o", "", "")
+
+			cmd.Flag("optarg").NoOptDefVal = " "
+
+			carapace.Gen(cmd).FlagCompletion(carapace.ActionMap{
+				"value":  carapace.ActionValues("val1", "val2"),
+				"optarg": carapace.ActionValues("opt1", "opt2"),
+			})
+
+			carapace.Gen(cmd).PositionalCompletion(
+				carapace.ActionValues("p1", "positional1"),
+			)
+
+			return carapace.ActionExecute(cmd)
+		}),
+	)
+
+}

example/cmd/chain_test.go

@@ -0,0 +1,80 @@
+package cmd
+
+import (
+	"testing"
+
+	"github.com/rsteube/carapace"
+	"github.com/rsteube/carapace/pkg/sandbox"
+	"github.com/rsteube/carapace/pkg/style"
+)
+
+func TestShorthandChain(t *testing.T) {
+	sandbox.Package(t, "github.com/rsteube/carapace/example")(func(s *sandbox.Sandbox) {
+		s.Run("chain", "-b").
+			Expect(carapace.ActionStyledValues(
+				"c", style.Default,
+				"o", style.Yellow,
+				"v", style.Blue,
+			).Prefix("-b").
+				NoSpace('c', 'o').
+				Tag("flags"))
+
+		s.Run("chain", "-bc").
+			Expect(carapace.ActionStyledValues(
+				"c", style.Default,
+				"o", style.Yellow,
+				"v", style.Blue,
+			).Prefix("-bc").
+				NoSpace('c', 'o').
+				Tag("flags"))
+
+		s.Run("chain", "-bcc").
+			Expect(carapace.ActionStyledValues(
+				"c", style.Default,
+				"o", style.Yellow,
+				"v", style.Blue,
+			).Prefix("-bcc").
+				NoSpace('c', 'o').
+				Tag("flags"))
+
+		s.Run("chain", "-bcco").
+			Expect(carapace.ActionStyledValues(
+				"c", style.Default,
+				"v", style.Blue,
+			).Prefix("-bcco").
+				NoSpace('c').
+				Tag("flags"))
+
+		s.Run("chain", "-bcco", "").
+			Expect(carapace.ActionValues(
+				"p1",
+				"positional1",
+			))
+
+		s.Run("chain", "-bcco=").
+			Expect(carapace.ActionValues(
+				"opt1",
+				"opt2",
+			).Prefix("-bcco="))
+
+		s.Run("chain", "-bccv", "").
+			Expect(carapace.ActionValues(
+				"val1",
+				"val2",
+			))
+
+		s.Run("chain", "-bccv=").
+			Expect(carapace.ActionValues(
+				"val1",
+				"val2",
+			).Prefix("-bccv="))
+
+		s.Run("chain", "-bccv", "val1", "-c").
+			Expect(carapace.ActionStyledValues(
+				"c", style.Default,
+				"o", style.Yellow,
+			).Prefix("-c").
+				NoSpace('c', 'o').
+				Tag("flags"))
+	})
+}

example/cmd/root_test.go

@@ -72,6 +72,7 @@ func TestRoot(t *testing.T) {
 					"injection", "just trying to break things",
 				).Style(style.Magenta).Tag("test commands"),
 				carapace.ActionValuesDescribed(
+					"chain", "shorthand chain",
 					"completion", "Generate the autocompletion script for the specified shell",
 					"group", "group example",
 					"help", "Help about any command",

internal/pflagfork/flag.go

@@ -21,6 +21,8 @@ const (
 
 type Flag struct {
 	*pflag.Flag
+	Prefix string
+	Args   []string
 }
 
 func (f Flag) Nargs() int {
@@ -53,52 +55,6 @@ func (f Flag) IsRepeatable() bool {
 	return false
 }
 
-func (f Flag) Split(arg string) []string {
-	delimiter := string(f.OptargDelimiter())
-	return strings.SplitAfterN(arg, delimiter, 2)
-}
-
-func (f Flag) Matches(arg string, posix bool) bool {
-	if !strings.HasPrefix(arg, "-") { // not a flag
-		return false
-	}
-
-	switch {
-
-	case strings.HasPrefix(arg, "--"):
-		name := strings.TrimPrefix(arg, "--")
-		name = strings.SplitN(name, string(f.OptargDelimiter()), 2)[0]
-
-		switch f.Mode() {
-		case ShorthandOnly, NameAsShorthand:
-			return false
-		default:
-			return name == f.Name
-		}
-
-	case !posix:
-		name := strings.TrimPrefix(arg, "-")
-		name = strings.SplitN(name, string(f.OptargDelimiter()), 2)[0]
-
-		if name == "" {
-			return false
-		}
-
-		switch f.Mode() {
-		case ShorthandOnly:
-			return name == f.Shorthand
-		default:
-			return name == f.Name || name == f.Shorthand
-		}
-
-	default:
-		if f.Shorthand != "" {
-			return strings.HasSuffix(arg, f.Shorthand)
-		}
-		return false
-	}
-}
-
 func (f Flag) TakesValue() bool {
 	switch f.Value.Type() {
 	case "bool", "boolSlice", "count":
@@ -173,3 +129,22 @@ func (f Flag) Definition() string {
 
 	return definition
 }
+
+func (f Flag) Consumes(arg string) bool {
+	switch {
+	case f.Flag == nil:
+		return false
+	case !f.TakesValue():
+		return false
+	case f.IsOptarg():
+		return false
+	case len(f.Args) == 0:
+		return true
+	case f.Nargs() > 1 && len(f.Args) < f.Nargs():
+		return true
+	case f.Nargs() < 0 && !strings.HasPrefix(arg, "-"):
+		return true
+	default:
+		return false
+	}
+}

internal/pflagfork/flagset.go

@@ -29,7 +29,7 @@ func (f FlagSet) IsPosix() bool {
 }
 
 func (f FlagSet) IsShorthandSeries(arg string) bool {
-	re := regexp.MustCompile("^-(?P<shorthand>[^-=]+)")
+	re := regexp.MustCompile("^-(?P<shorthand>[^-].*)")
 	return re.MatchString(arg) && f.IsPosix()
 }
 
@@ -48,20 +48,106 @@ func (f FlagSet) IsMutuallyExclusive(flag *pflag.Flag) bool {
 
 func (f *FlagSet) VisitAll(fn func(*Flag)) {
 	f.FlagSet.VisitAll(func(flag *pflag.Flag) {
-		fn(&Flag{flag})
+		fn(&Flag{Flag: flag, Args: []string{}})
 	})
 
 }
 
 func (fs FlagSet) LookupArg(arg string) (result *Flag) {
 	isPosix := fs.IsPosix()
-	fs.VisitAll(func(f *Flag) {
+
+	switch {
+	case strings.HasPrefix(arg, "--"):
+		return fs.lookupPosixLonghandArg(arg)
+	case isPosix:
+		return fs.lookupPosixShorthandArg(arg)
+	case !isPosix:
+		return fs.lookupNonPosixShorthandArg(arg)
+	}
+	return
+}
+
+func (fs FlagSet) ShorthandLookup(name string) *Flag {
+	if f := fs.FlagSet.ShorthandLookup(name); f != nil {
+		return &Flag{
+			Flag: f,
+			Args: []string{},
+		}
+	}
+	return nil
+}
+
+func (fs FlagSet) lookupPosixLonghandArg(arg string) (flag *Flag) {
+	if !strings.HasPrefix(arg, "--") {
+		return nil
+	}
+
+	fs.VisitAll(func(f *Flag) { // TODO needs to be sorted to try longest matching first
+		if flag != nil || f.Mode() != Default {
+			return
+		}
+
+		splitted := strings.SplitAfterN(arg, string(f.OptargDelimiter()), 2)
+		if strings.TrimSuffix(splitted[0], string(f.OptargDelimiter())) == "--"+f.Name {
+			flag = f
+			flag.Prefix = splitted[0]
+			if len(splitted) > 1 {
+				flag.Args = splitted[1:]
+			}
+		}
+	})
+	return
+}
+
+func (fs FlagSet) lookupPosixShorthandArg(arg string) *Flag {
+	if !strings.HasPrefix(arg, "-") || !fs.IsPosix() || len(arg) < 2 {
+		return nil
+	}
+
+	for index, r := range arg[1:] {
+		index += 1
+		flag := fs.ShorthandLookup(string(r))
+
+		switch {
+		case flag == nil:
+			return flag
+		case len(arg) == index+1:
+			flag.Prefix = arg
+			return flag
+		case arg[index+1] == byte(flag.OptargDelimiter()) && len(arg) > index+2:
+			flag.Prefix = arg[:index+2]
+			flag.Args = []string{arg[index+2:]}
+			return flag
+		case arg[index+1] == byte(flag.OptargDelimiter()):
+			flag.Prefix = arg[:index+2]
+			flag.Args = []string{""}
+			return flag
+		case !flag.IsOptarg() && len(arg) > index+1:
+			flag.Prefix = arg[:index+1]
+			flag.Args = []string{arg[index+1:]}
+			return flag
+		}
+	}
+	return nil
+}
+
+func (fs FlagSet) lookupNonPosixShorthandArg(arg string) (result *Flag) { // TODO pretty much duplicates longhand lookup
+	if !strings.HasPrefix(arg, "-") {
+		return nil
+	}
+
+	fs.VisitAll(func(f *Flag) { // TODO needs to be sorted to try longest matching first
 		if result != nil {
 			return
 		}
 
-		if f.Matches(arg, isPosix) {
+		splitted := strings.SplitAfterN(arg, string(f.OptargDelimiter()), 2)
+		if strings.TrimSuffix(splitted[0], string(f.OptargDelimiter())) == "-"+f.Shorthand {
 			result = f
+			result.Prefix = splitted[0]
+			if len(splitted) > 1 {
+				result.Args = splitted[1:]
+			}
 		}
 	})
 	return

internal/pflagfork/flagset_test.go

@@ -0,0 +1,48 @@
+package pflagfork
+
+import (
+	"reflect"
+	"testing"
+
+	"github.com/spf13/pflag"
+)
+
+func TestLookupPosixShorthandArg(t *testing.T) {
+	_test := func(arg, name, prefix string, args ...string) {
+		t.Run(arg, func(t *testing.T) {
+			if args == nil {
+				args = []string{}
+			}
+
+			fs := &FlagSet{pflag.NewFlagSet("test", pflag.PanicOnError)}
+
+			fs.BoolP("bool", "b", false, "")
+			fs.CountP("count", "c", "")
+			fs.StringP("string", "s", "", "")
+
+			f := fs.lookupPosixShorthandArg(arg)
+			if f == nil || f.Name != name {
+				t.Fatalf("should be " + name)
+			}
+
+			if f.Prefix != prefix {
+				t.Fatalf("prefix doesnt match actual: %#v, expected: %#v", f.Prefix, prefix)
+			}
+
+			if !reflect.DeepEqual(f.Args, args) {
+				t.Fatalf("args dont match %v: actual: %#v expected: %#v", arg, f.Args, args)
+			}
+
+		})
+	}
+
+	_test("-b=", "bool", "-b=", "")
+	_test("-b=t", "bool", "-b=", "t")
+	_test("-b=true", "bool", "-b=", "true")
+	_test("-ccb", "bool", "-ccb")
+	_test("-ccb=", "bool", "-ccb=", "")
+	_test("-ccb=t", "bool", "-ccb=", "t")
+	_test("-ccb=true", "bool", "-ccb=", "true")
+	_test("-ccbs=val1", "string", "-ccbs=", "val1")
+	_test("-ccbsval1", "string", "-ccbs", "val1")
+}