Skip to content

Commit

Permalink
fix: accept flags after command's args
Browse files Browse the repository at this point in the history
  • Loading branch information
tbruyelle committed Apr 19, 2023
1 parent ac896ff commit 9a0356a
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 9a0356a

Please sign in to comment.