Skip to content

Commit

Permalink
fix: support any flags order
Browse files Browse the repository at this point in the history
This change contains the minimum change required to support some of the requirements described in gnolang#731

What it brings:
- Fork of `ffcli` in `tm2/pkg/commands/ffcli`. Note that the legacy `ff` is still used (so env var and config file override is still supported).
- Accepts flags declared after the command arguments.
This is one of the main complaints about `ffcli`. Now use can run `
-
  • Loading branch information
tbruyelle committed Apr 19, 2023
1 parent ac896ff commit 7de0d2a
Show file tree
Hide file tree
Showing 3 changed files with 176 additions and 2 deletions.
2 changes: 1 addition & 1 deletion tm2/pkg/commands/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ func NewCommand(
LongHelp: meta.LongHelp,
ShortUsage: meta.ShortUsage,
Options: meta.Options,
FlagSet: flag.NewFlagSet(meta.Name, flag.ExitOnError),
FlagSet: flag.NewFlagSet(meta.Name, flag.ContinueOnError),
Exec: exec,
},
cfg: config,
Expand Down
163 changes: 162 additions & 1 deletion tm2/pkg/commands/commands_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package commands

import (
"context"
"flag"
"testing"

"github.com/gnolang/gno/tm2/pkg/commands/ffcli"
"github.com/jaekwon/testify/require"
"github.com/stretchr/testify/assert"

"github.com/gnolang/gno/tm2/pkg/commands/ffcli"
)

type configDelegate func(*flag.FlagSet)
Expand All @@ -20,6 +23,164 @@ func (c *mockConfig) RegisterFlags(fs *flag.FlagSet) {
}
}

func TestCommandFlagsOrder(t *testing.T) {
type flags struct {
b bool
s string
x bool
}
tests := []struct {
name string
osArgs []string
expectedArgs []string
expectedFlags flags
expectedError string
}{
{
name: "no args no flags",
osArgs: []string{},
expectedArgs: []string{},
expectedFlags: flags{},
},
{
name: "only args",
osArgs: []string{"bar", "baz"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{},
},
{
name: "only flags",
osArgs: []string{"-b", "-s", "str"},
expectedArgs: []string{},
expectedFlags: flags{b: true, s: "str"},
},
{
name: "unknow flag",
osArgs: []string{"-y", "-s", "str"},
expectedArgs: []string{},
expectedError: "error parsing commandline arguments: flag provided but not defined: -y",
},
{
name: "flags before args",
osArgs: []string{"-b", "-s", "str", "bar", "baz"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str"},
},
{
name: "flags after args",
osArgs: []string{"bar", "baz", "-b", "-s", "str"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str"},
},
{
name: "flags around args",
osArgs: []string{"-b", "bar", "baz", "-s", "str"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str"},
},
{
name: "flags between args",
osArgs: []string{"bar", "-b", "-s", "str", "baz"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str"},
},
{
name: "subcommand no flags no args",
osArgs: []string{"sub"},
expectedArgs: []string{},
expectedFlags: flags{},
},
{
name: "subcommand only args",
osArgs: []string{"sub", "bar", "baz"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{},
},
{
name: "subcommand flag before subcommand",
osArgs: []string{"-x", "sub"},
expectedError: "error parsing commandline arguments: flag provided but not defined: -x",
},
{
name: "subcommand only flags",
osArgs: []string{"-b", "sub", "-x", "-s", "str"},
expectedArgs: []string{},
expectedFlags: flags{b: true, s: "str", x: true},
},
{
name: "subcommand flags before args",
osArgs: []string{"-b", "sub", "-x", "-s", "str", "bar", "baz"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str", x: true},
},
{
name: "subcommand flags after args",
osArgs: []string{"-b", "sub", "bar", "baz", "-x", "-s", "str"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str", x: true},
},
{
name: "subcommand flags around args",
osArgs: []string{"-b", "sub", "-x", "bar", "baz", "-s", "str"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str", x: true},
},
{
name: "subcommand flags between args",
osArgs: []string{"-b", "sub", "bar", "-x", "baz", "-s", "str"},
expectedArgs: []string{"bar", "baz"},
expectedFlags: flags{b: true, s: "str", x: true},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
var (
args []string
flags flags
)
// Create a cmd main that takes 2 flags -b and -s
cmd := NewCommand(
Metadata{Name: "main"},
&mockConfig{
configFn: func(fs *flag.FlagSet) {
fs.BoolVar(&flags.b, "b", false, "a boolan")
fs.StringVar(&flags.s, "s", "", "a string")
},
},
func(_ context.Context, a []string) error {
args = a
return nil
},
)
// Add a sub command to cmd with a single flag -x
cmd.AddSubCommands(
NewCommand(
Metadata{Name: "sub"},
&mockConfig{
configFn: func(fs *flag.FlagSet) {
fs.BoolVar(&flags.x, "x", false, "a boolan")
},
},
func(_ context.Context, a []string) error {
args = a
return nil
},
),
)

err := cmd.ParseAndRun(context.Background(), tt.osArgs)

if tt.expectedError != "" {
require.EqualError(t, err, tt.expectedError)
return
}
require.NoError(t, err)
require.Equal(t, args, tt.expectedArgs, "wrong args")
require.Equal(t, flags, tt.expectedFlags, "wrong flags")
})
}
}

func TestCommand_AddSubCommands(t *testing.T) {
t.Parallel()

Expand Down
13 changes: 13 additions & 0 deletions tm2/pkg/commands/ffcli/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,19 @@ func (c *Command) Parse(args []string) error {
return subcommand.Parse(c.args[1:])
}
}
// args[0] is not a sub command, so it's an command argument.
c.args = c.args[:1]
// continue flag parsing after args[0]
for c.FlagSet.NArg() > 1 {
err := ff.Parse(c.FlagSet, c.FlagSet.Args()[1:], c.Options...)
if err != nil {
return err
}
if c.FlagSet.NArg() > 0 {
// Arg(0) is an arg, not a flag
c.args = append(c.args, c.FlagSet.Arg(0))
}
}
}

c.selected = c
Expand Down

0 comments on commit 7de0d2a

Please sign in to comment.