Skip to content

Commit

Permalink
traverse: fix shorthand chain
Browse files Browse the repository at this point in the history
- support "attached" value flags like `-v=arg` and `-varg`
- use shorthand for nospace in shorthand series

Some logs are wrong and there are likely some edge cases failing.
But tests are fine and works well so far so issues will be fixed in subsequent PRs.
  • Loading branch information
rsteube committed Jul 25, 2023
1 parent 843cf2a commit d0887d3
Show file tree
Hide file tree
Showing 9 changed files with 314 additions and 99 deletions.
2 changes: 1 addition & 1 deletion defaultActions_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
45 changes: 45 additions & 0 deletions example/cmd/chain.go
Original file line number Diff line number Diff line change
@@ -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)
}),
)

}
80 changes: 80 additions & 0 deletions example/cmd/chain_test.go
Original file line number Diff line number Diff line change
@@ -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"))
})
}
1 change: 1 addition & 0 deletions example/cmd/root_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
67 changes: 21 additions & 46 deletions internal/pflagfork/flag.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ const (

type Flag struct {
*pflag.Flag
Prefix string
Args []string
}

func (f Flag) Nargs() int {
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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
}
}
94 changes: 90 additions & 4 deletions internal/pflagfork/flagset.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}

Expand All @@ -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
Expand Down
48 changes: 48 additions & 0 deletions internal/pflagfork/flagset_test.go
Original file line number Diff line number Diff line change
@@ -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")
}

0 comments on commit d0887d3

Please sign in to comment.