From b4605ff6f3f31e45eaa05276a33cbd7599df893e Mon Sep 17 00:00:00 2001 From: fiatjaf Date: Tue, 25 Jun 2024 23:14:25 -0300 Subject: [PATCH 1/5] reorder flags so they can come after arguments. --- command.go | 81 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/command.go b/command.go index 590de63ea9..6f15226384 100644 --- a/command.go +++ b/command.go @@ -744,6 +744,10 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { return cmd.Args(), cmd.flagSet.Parse(append([]string{"--"}, args.Tail()...)) } + tracef("reordering flags so they appear before the arguments") + + args = reorderArgs(cmd.Flags, args) + tracef("walking command lineage for persistent flags (cmd=%[1]q)", cmd.Name) for pCmd := cmd.parent; pCmd != nil; pCmd = pCmd.parent { @@ -1255,3 +1259,80 @@ func makeFlagNameVisitor(names *[]string) func(*flag.Flag) { } } } + +// reorderArgs moves all flags (via reorderedArgs) before the rest of +// the arguments (remainingArgs) as this is what flag expects. +func reorderArgs(commandFlags []Flag, args Args) Args { + var remainingArgs, reorderedArgs []string + + tail := args.Tail() + + nextIndexMayContainValue := false + for i, arg := range tail { + // if we're expecting an option-value, check if this arg is a value, in + // which case it should be re-ordered next to its associated flag + if isFlag, _ := argIsFlag(commandFlags, arg); nextIndexMayContainValue && !isFlag { + nextIndexMayContainValue = false + reorderedArgs = append(reorderedArgs, arg) + } else if arg == "--" { + // don't reorder any args after the -- delimiter As described in the POSIX spec: + // https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap12.html#tag_12_02 + // > Guideline 10: + // > The first -- argument that is not an option-argument should be accepted + // > as a delimiter indicating the end of options. Any following arguments + // > should be treated as operands, even if they begin with the '-' character. + + // make sure the "--" delimiter itself is at the start + remainingArgs = append([]string{"--"}, remainingArgs...) + remainingArgs = append(remainingArgs, tail[i+1:]...) + break + // checks if this is an arg that should be re-ordered + } else if isFlag, isBooleanFlag := argIsFlag(commandFlags, arg); isFlag { + // we have determined that this is a flag that we should re-order + reorderedArgs = append(reorderedArgs, arg) + + // if this arg does not contain a "=", then the next index may contain the value for this flag + nextIndexMayContainValue = !strings.Contains(arg, "=") && !isBooleanFlag + + // simply append any remaining args + } else { + remainingArgs = append(remainingArgs, arg) + } + } + + return &stringSliceArgs{append([]string{args.First()}, append(reorderedArgs, remainingArgs...)...)} +} + +// argIsFlag checks if an arg is one of our command flags +func argIsFlag(commandFlags []Flag, arg string) (isFlag bool, isBooleanFlag bool) { + if arg == "-" || arg == "--" { + // `-` is never a flag + // `--` is an option-value when following a flag, and a delimiter indicating the end of options in other cases. + return false, false + } + // flags always start with a - + if !strings.HasPrefix(arg, "-") { + return false, false + } + // this line turns `--flag` into `flag` + if strings.HasPrefix(arg, "--") { + arg = strings.Replace(arg, "-", "", 2) + } + // this line turns `-flag` into `flag` + if strings.HasPrefix(arg, "-") { + arg = strings.Replace(arg, "-", "", 1) + } + // this line turns `flag=value` into `flag` + arg = strings.Split(arg, "=")[0] + // look through all the flags, to see if the `arg` is one of our flags + for _, flag := range commandFlags { + for _, key := range flag.Names() { + if key == arg { + _, isBooleanFlag = flag.(*BoolFlag) + return true, isBooleanFlag + } + } + } + // return false if this arg was not one of our flags + return false, false +} From 3a8b0280e2c5b077830c4d430182e91d6f6cd0ad Mon Sep 17 00:00:00 2001 From: fiatjaf Date: Fri, 12 Jul 2024 18:21:13 -0300 Subject: [PATCH 2/5] fix it for subcommands and make it optional. --- command.go | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/command.go b/command.go index 6f15226384..ed1b8b45c6 100644 --- a/command.go +++ b/command.go @@ -109,6 +109,9 @@ type Command struct { // single-character bool arguments into one // i.e. foobar -o -v -> foobar -ov UseShortOptionHandling bool `json:"useShortOptionHandling"` + // Boolean to enable reordering flags before passing them to the parser + // such that `cli -f ` behaves the same as `cli -f ` + AllowFlagsAfterArguments bool `json:"AllowFlagsAfterArguments"` // Enable suggestions for commands and flags Suggest bool `json:"suggest"` // Allows global flags set by libraries which use flag.XXXVar(...) directly @@ -744,9 +747,10 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { return cmd.Args(), cmd.flagSet.Parse(append([]string{"--"}, args.Tail()...)) } - tracef("reordering flags so they appear before the arguments") - - args = reorderArgs(cmd.Flags, args) + if cmd.AllowFlagsAfterArguments { + tracef("reordering flags so they appear before the arguments") + args = reorderArgs(cmd, args) + } tracef("walking command lineage for persistent flags (cmd=%[1]q)", cmd.Name) @@ -1262,16 +1266,24 @@ func makeFlagNameVisitor(names *[]string) func(*flag.Flag) { // reorderArgs moves all flags (via reorderedArgs) before the rest of // the arguments (remainingArgs) as this is what flag expects. -func reorderArgs(commandFlags []Flag, args Args) Args { +func reorderArgs(cmd *Command, args Args) Args { var remainingArgs, reorderedArgs []string tail := args.Tail() nextIndexMayContainValue := false for i, arg := range tail { + + subCmd := cmd.Command(arg) + if subCmd != nil { + cmd = subCmd + reorderedArgs = append(reorderedArgs, arg) + continue + } + // if we're expecting an option-value, check if this arg is a value, in // which case it should be re-ordered next to its associated flag - if isFlag, _ := argIsFlag(commandFlags, arg); nextIndexMayContainValue && !isFlag { + if isFlag, _ := argIsFlag(cmd.Flags, arg); nextIndexMayContainValue && !isFlag { nextIndexMayContainValue = false reorderedArgs = append(reorderedArgs, arg) } else if arg == "--" { @@ -1287,7 +1299,7 @@ func reorderArgs(commandFlags []Flag, args Args) Args { remainingArgs = append(remainingArgs, tail[i+1:]...) break // checks if this is an arg that should be re-ordered - } else if isFlag, isBooleanFlag := argIsFlag(commandFlags, arg); isFlag { + } else if isFlag, isBooleanFlag := argIsFlag(cmd.Flags, arg); isFlag { // we have determined that this is a flag that we should re-order reorderedArgs = append(reorderedArgs, arg) From 380173a84e1668ac62a09c0484da0d4ce445431d Mon Sep 17 00:00:00 2001 From: fiatjaf Date: Sat, 13 Jul 2024 11:01:18 -0300 Subject: [PATCH 3/5] address comments. --- command.go | 82 +++++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 59 insertions(+), 23 deletions(-) diff --git a/command.go b/command.go index ed1b8b45c6..98376c17f8 100644 --- a/command.go +++ b/command.go @@ -110,8 +110,9 @@ type Command struct { // i.e. foobar -o -v -> foobar -ov UseShortOptionHandling bool `json:"useShortOptionHandling"` // Boolean to enable reordering flags before passing them to the parser - // such that `cli -f ` behaves the same as `cli -f ` - AllowFlagsAfterArguments bool `json:"AllowFlagsAfterArguments"` + // such that `cli -f ` behaves the same as `cli -f `. + // This only has effect when set at the top-level command. + AllowFlagsAfterArguments bool `json:"allowFlagsAfterArguments"` // Enable suggestions for commands and flags Suggest bool `json:"suggest"` // Allows global flags set by libraries which use flag.XXXVar(...) directly @@ -1273,7 +1274,6 @@ func reorderArgs(cmd *Command, args Args) Args { nextIndexMayContainValue := false for i, arg := range tail { - subCmd := cmd.Command(arg) if subCmd != nil { cmd = subCmd @@ -1281,9 +1281,10 @@ func reorderArgs(cmd *Command, args Args) Args { continue } + isFlag, isBooleanFlag := cmd.argIsFlag(arg) // if we're expecting an option-value, check if this arg is a value, in // which case it should be re-ordered next to its associated flag - if isFlag, _ := argIsFlag(cmd.Flags, arg); nextIndexMayContainValue && !isFlag { + if nextIndexMayContainValue && !isFlag { nextIndexMayContainValue = false reorderedArgs = append(reorderedArgs, arg) } else if arg == "--" { @@ -1299,15 +1300,14 @@ func reorderArgs(cmd *Command, args Args) Args { remainingArgs = append(remainingArgs, tail[i+1:]...) break // checks if this is an arg that should be re-ordered - } else if isFlag, isBooleanFlag := argIsFlag(cmd.Flags, arg); isFlag { + } else if isFlag { // we have determined that this is a flag that we should re-order reorderedArgs = append(reorderedArgs, arg) // if this arg does not contain a "=", then the next index may contain the value for this flag nextIndexMayContainValue = !strings.Contains(arg, "=") && !isBooleanFlag - - // simply append any remaining args } else { + // simply append any remaining args remainingArgs = append(remainingArgs, arg) } } @@ -1316,35 +1316,71 @@ func reorderArgs(cmd *Command, args Args) Args { } // argIsFlag checks if an arg is one of our command flags -func argIsFlag(commandFlags []Flag, arg string) (isFlag bool, isBooleanFlag bool) { +func (cmd *Command) argIsFlag(arg string) (isFlag bool, isBooleanFlag bool) { if arg == "-" || arg == "--" { // `-` is never a flag // `--` is an option-value when following a flag, and a delimiter indicating the end of options in other cases. return false, false } + // flags always start with a - if !strings.HasPrefix(arg, "-") { return false, false } + // this line turns `--flag` into `flag` - if strings.HasPrefix(arg, "--") { - arg = strings.Replace(arg, "-", "", 2) - } - // this line turns `-flag` into `flag` - if strings.HasPrefix(arg, "-") { - arg = strings.Replace(arg, "-", "", 1) - } - // this line turns `flag=value` into `flag` - arg = strings.Split(arg, "=")[0] - // look through all the flags, to see if the `arg` is one of our flags - for _, flag := range commandFlags { - for _, key := range flag.Names() { - if key == arg { - _, isBooleanFlag = flag.(*BoolFlag) - return true, isBooleanFlag + arg, _ = strings.CutPrefix(arg, "--") + + if cmd.useShortOptionHandling() && strings.HasPrefix(arg, "-") { + // assume this is a bunch of short flags bundled together + shortFlags := strings.Split(arg[1:], "") + for s, shortFlag := range shortFlags { + // look through all the flags, to see if the `arg` is one of our flags + for _, flag := range cmd.Flags { + for _, key := range flag.Names() { + if key == shortFlag { + _, isBooleanFlag = flag.(boolFlag) + if isBooleanFlag { + // this is the default, expected case + continue + } else { + // only the last flag may not be a boolean + if s == len(shortFlag)-1 { + // this is an arg and not a boolean arg (it expects a value right after it) + return true, false + } else { + // this is not a valid flag bundle for this command + return false, false + } + } + } else { + // this short mode doesn't correspond to any flags for this Command + return false, false + } + } + } + + // in this case we didn't exit, which means all the short flags are booleans + return true, true + } + } else { + // this line turns `-flag` into `flag` + arg, _ = strings.CutPrefix(arg, "-") + + // this line turns `flag=value` into `flag` + arg = strings.Split(arg, "=")[0] + + // look through all the flags, to see if the `arg` is one of our flags + for _, flag := range cmd.Flags { + for _, key := range flag.Names() { + if key == arg { + _, isBooleanFlag = flag.(boolFlag) + return true, isBooleanFlag + } } } } + // return false if this arg was not one of our flags return false, false } From ebd1e3ed26b9a43b7689cb1e672cc1fb5e15f6e9 Mon Sep 17 00:00:00 2001 From: fiatjaf Date: Sat, 13 Jul 2024 19:39:55 -0300 Subject: [PATCH 4/5] fix failing test. --- command.go | 2 +- command_test.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index 98376c17f8..db6947160d 100644 --- a/command.go +++ b/command.go @@ -112,7 +112,7 @@ type Command struct { // Boolean to enable reordering flags before passing them to the parser // such that `cli -f ` behaves the same as `cli -f `. // This only has effect when set at the top-level command. - AllowFlagsAfterArguments bool `json:"allowFlagsAfterArguments"` + AllowFlagsAfterArguments bool `json:"allowFlagsAfterArguments,omitempty"` // Enable suggestions for commands and flags Suggest bool `json:"suggest"` // Allows global flags set by libraries which use flag.XXXVar(...) directly diff --git a/command_test.go b/command_test.go index 6a38816046..718323024a 100644 --- a/command_test.go +++ b/command_test.go @@ -60,6 +60,7 @@ func buildExtendedTestCommand() *Command { Hidden: true, }, } + cmd.AllowFlagsAfterArguments = true cmd.Commands = []*Command{{ Aliases: []string{"c"}, Flags: []Flag{ @@ -4385,6 +4386,7 @@ func TestJSONExportCommand(t *testing.T) { "sliceFlagSeparator": "", "disableSliceFlagSeparator": false, "useShortOptionHandling": false, + "allowFlagsAfterArguments": true, "suggest": false, "allowExtFlags": false, "skipFlagParsing": false, From bb036558919f39a70172e9d5a95a0827ab82bfb8 Mon Sep 17 00:00:00 2001 From: fiatjaf Date: Sun, 14 Jul 2024 20:21:33 -0300 Subject: [PATCH 5/5] fix UseShortOptionHandling case on reorderFlags. --- command.go | 55 +++++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 23 deletions(-) diff --git a/command.go b/command.go index db6947160d..8e32d4631a 100644 --- a/command.go +++ b/command.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "reflect" + "slices" "sort" "strings" "unicode" @@ -1282,6 +1283,7 @@ func reorderArgs(cmd *Command, args Args) Args { } isFlag, isBooleanFlag := cmd.argIsFlag(arg) + // if we're expecting an option-value, check if this arg is a value, in // which case it should be re-ordered next to its associated flag if nextIndexMayContainValue && !isFlag { @@ -1336,33 +1338,40 @@ func (cmd *Command) argIsFlag(arg string) (isFlag bool, isBooleanFlag bool) { shortFlags := strings.Split(arg[1:], "") for s, shortFlag := range shortFlags { // look through all the flags, to see if the `arg` is one of our flags - for _, flag := range cmd.Flags { - for _, key := range flag.Names() { - if key == shortFlag { - _, isBooleanFlag = flag.(boolFlag) - if isBooleanFlag { - // this is the default, expected case - continue - } else { - // only the last flag may not be a boolean - if s == len(shortFlag)-1 { - // this is an arg and not a boolean arg (it expects a value right after it) - return true, false - } else { - // this is not a valid flag bundle for this command - return false, false - } - } - } else { - // this short mode doesn't correspond to any flags for this Command - return false, false - } + var flag Flag + idx := slices.IndexFunc(cmd.Flags, func(f Flag) bool { return slices.Contains(f.Names(), shortFlag) }) + if idx != -1 { + flag = cmd.Flags[idx] + } else { + vpf := cmd.VisiblePersistentFlags() + idx := slices.IndexFunc(vpf, func(f Flag) bool { return slices.Contains(f.Names(), shortFlag) }) + if idx != -1 { + flag = vpf[idx] + } else { + // this short flag doesn't correspond to any flags for this Command + return false, false } } - // in this case we didn't exit, which means all the short flags are booleans - return true, true + // got a flag match + _, isBooleanFlag = flag.(boolFlag) + if isBooleanFlag { + // this is the default, most common case + continue + } else { + // only the last flag may not be a boolean + if s == len(shortFlag)-1 { + // this is an arg and not a boolean arg (it expects a value right after it) + return true, false + } else { + // this is not a valid flag bundle for this command + return false, false + } + } } + + // in this case we didn't exit, which means all the short flags are booleans + return true, true } else { // this line turns `-flag` into `flag` arg, _ = strings.CutPrefix(arg, "-")