From f1e1987e7f10fd4dd6142e1f10c893ba9954014c Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 30 Jun 2024 10:40:14 -0400 Subject: [PATCH 1/2] Fix:(issue_1930) Fix for invalid bool counts --- command.go | 6 ------ command_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++------- flag.go | 43 -------------------------------------- flag_test.go | 28 +++++++++++++++++-------- 4 files changed, 66 insertions(+), 66 deletions(-) diff --git a/command.go b/command.go index 04b3474cb4..75d8b7ec33 100644 --- a/command.go +++ b/command.go @@ -800,12 +800,6 @@ func (cmd *Command) parseFlags(args Args) (Args, error) { return cmd.Args(), err } - tracef("normalizing flags (cmd=%[1]q)", cmd.Name) - - if err := normalizeFlags(cmd.Flags, cmd.flagSet); err != nil { - return cmd.Args(), err - } - tracef("done parsing flags (cmd=%[1]q)", cmd.Name) return cmd.Args(), nil diff --git a/command_test.go b/command_test.go index 20abfb758e..b1db2bad84 100644 --- a/command_test.go +++ b/command_test.go @@ -3204,16 +3204,55 @@ func TestCommand_Bool(t *testing.T) { } func TestCommand_Value(t *testing.T) { - set := flag.NewFlagSet("test", 0) - set.Int("myflag", 12, "doc") - parentSet := flag.NewFlagSet("test", 0) - parentSet.Int("top-flag", 13, "doc") - pCmd := &Command{flagSet: parentSet} - cmd := &Command{flagSet: set, parent: pCmd} + subCmd := &Command{ + Name: "test", + Flags: []Flag{ + &IntFlag{ + Name: "myflag", + Usage: "doc", + Aliases: []string{"m", "mf"}, + }, + }, + Action: func(ctx context.Context, c *Command) error { + return nil + }, + } + + cmd := &Command{ + Flags: []Flag{ + &IntFlag{ + Name: "top-flag", + Usage: "doc", + Aliases: []string{"t", "tf"}, + }, + }, + Commands: []*Command{ + subCmd, + }, + } + err := cmd.Run(buildTestContext(t), []string{"main", "--top-flag", "13", "test", "--myflag", "14"}) + assert.NoError(t, err) r := require.New(t) - r.Equal(12, cmd.Value("myflag")) - r.Equal(13, cmd.Value("top-flag")) + r.Equal(int64(13), cmd.Value("top-flag")) + r.Equal(int64(13), cmd.Value("t")) + r.Equal(int64(13), cmd.Value("tf")) + + r.Equal(int64(14), subCmd.Value("myflag")) + r.Equal(int64(14), subCmd.Value("m")) + r.Equal(int64(14), subCmd.Value("mf")) + + // use only aliases here + err = cmd.Run(buildTestContext(t), []string{"main", "-tf", "15", "test", "-m", "16"}) + assert.NoError(t, err) + + r.Equal(int64(15), cmd.Value("top-flag")) + r.Equal(int64(15), cmd.Value("t")) + r.Equal(int64(15), cmd.Value("tf")) + + r.Equal(int64(16), subCmd.Value("myflag")) + r.Equal(int64(16), subCmd.Value("m")) + r.Equal(int64(16), subCmd.Value("mf")) r.Nil(cmd.Value("unknown-flag")) } diff --git a/flag.go b/flag.go index f7c64c81a4..76a5d71669 100644 --- a/flag.go +++ b/flag.go @@ -2,7 +2,6 @@ package cli import ( "context" - "errors" "flag" "fmt" "io" @@ -198,48 +197,6 @@ func newFlagSet(name string, flags []Flag) (*flag.FlagSet, error) { return set, nil } -func copyFlag(name string, ff *flag.Flag, set *flag.FlagSet) { - switch ff.Value.(type) { - case Serializer: - _ = set.Set(name, ff.Value.(Serializer).Serialize()) - default: - _ = set.Set(name, ff.Value.String()) - } -} - -func normalizeFlags(flags []Flag, set *flag.FlagSet) error { - visited := make(map[string]bool) - set.Visit(func(f *flag.Flag) { - visited[f.Name] = true - }) - for _, f := range flags { - parts := f.Names() - if len(parts) == 1 { - continue - } - var ff *flag.Flag - for _, name := range parts { - name = strings.Trim(name, " ") - if visited[name] { - if ff != nil { - return errors.New("Cannot use two forms of the same flag: " + name + " " + ff.Name) - } - ff = set.Lookup(name) - } - } - if ff == nil { - continue - } - for _, name := range parts { - name = strings.Trim(name, " ") - if !visited[name] { - copyFlag(name, ff, set) - } - } - } - return nil -} - func visibleFlags(fl []Flag) []Flag { var visible []Flag for _, f := range fl { diff --git a/flag_test.go b/flag_test.go index 6f67066411..052115c332 100644 --- a/flag_test.go +++ b/flag_test.go @@ -83,28 +83,38 @@ func TestBoolFlagCountFromCommand(t *testing.T) { expectedCount int }{ { - input: []string{"-tf", "-w", "-huh"}, + input: []string{"main", "-tf", "-w", "-huh"}, expectedVal: true, expectedCount: 3, }, { - input: []string{}, + input: []string{"main", "-huh"}, + expectedVal: true, + expectedCount: 1, + }, + { + input: []string{"main"}, expectedVal: false, expectedCount: 0, }, } + bf := &BoolFlag{Name: "tf", Aliases: []string{"w", "huh"}} for _, bct := range boolCountTests { - set := flag.NewFlagSet("test", 0) - cmd := &Command{flagSet: set} - tf := &BoolFlag{Name: "tf", Aliases: []string{"w", "huh"}} + cmd := &Command{ + Flags: []Flag{ + bf, + }, + } r := require.New(t) - r.NoError(tf.Apply(set)) - r.NoError(set.Parse(bct.input)) + r.NoError(cmd.Run(buildTestContext(t), bct.input)) - r.Equal(bct.expectedVal, tf.Get(cmd)) - r.Equal(bct.expectedCount, cmd.Count("tf")) + r.Equal(bct.expectedVal, cmd.Value(bf.Name)) + r.Equal(bct.expectedCount, cmd.Count(bf.Name)) + for _, alias := range bf.Aliases { + r.Equal(bct.expectedCount, cmd.Count(alias)) + } } } From 890d3ee7eeb6b1e3472552daa97ee10f7b5141d6 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 1 Jul 2024 07:57:53 -0400 Subject: [PATCH 2/2] Split test into multiple runs to make it more self documenting --- command_test.go | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/command_test.go b/command_test.go index b1db2bad84..6a38816046 100644 --- a/command_test.go +++ b/command_test.go @@ -3230,30 +3230,34 @@ func TestCommand_Value(t *testing.T) { subCmd, }, } - err := cmd.Run(buildTestContext(t), []string{"main", "--top-flag", "13", "test", "--myflag", "14"}) - assert.NoError(t, err) + t.Run("flag name", func(t *testing.T) { + r := require.New(t) + err := cmd.Run(buildTestContext(t), []string{"main", "--top-flag", "13", "test", "--myflag", "14"}) - r := require.New(t) - r.Equal(int64(13), cmd.Value("top-flag")) - r.Equal(int64(13), cmd.Value("t")) - r.Equal(int64(13), cmd.Value("tf")) + r.NoError(err) + r.Equal(int64(13), cmd.Value("top-flag")) + r.Equal(int64(13), cmd.Value("t")) + r.Equal(int64(13), cmd.Value("tf")) - r.Equal(int64(14), subCmd.Value("myflag")) - r.Equal(int64(14), subCmd.Value("m")) - r.Equal(int64(14), subCmd.Value("mf")) + r.Equal(int64(14), subCmd.Value("myflag")) + r.Equal(int64(14), subCmd.Value("m")) + r.Equal(int64(14), subCmd.Value("mf")) + }) - // use only aliases here - err = cmd.Run(buildTestContext(t), []string{"main", "-tf", "15", "test", "-m", "16"}) - assert.NoError(t, err) + t.Run("flag aliases", func(t *testing.T) { + r := require.New(t) + err := cmd.Run(buildTestContext(t), []string{"main", "-tf", "15", "test", "-m", "16"}) - r.Equal(int64(15), cmd.Value("top-flag")) - r.Equal(int64(15), cmd.Value("t")) - r.Equal(int64(15), cmd.Value("tf")) + r.NoError(err) + r.Equal(int64(15), cmd.Value("top-flag")) + r.Equal(int64(15), cmd.Value("t")) + r.Equal(int64(15), cmd.Value("tf")) - r.Equal(int64(16), subCmd.Value("myflag")) - r.Equal(int64(16), subCmd.Value("m")) - r.Equal(int64(16), subCmd.Value("mf")) - r.Nil(cmd.Value("unknown-flag")) + r.Equal(int64(16), subCmd.Value("myflag")) + r.Equal(int64(16), subCmd.Value("m")) + r.Equal(int64(16), subCmd.Value("mf")) + r.Nil(cmd.Value("unknown-flag")) + }) } func TestCommand_Value_InvalidFlagAccessHandler(t *testing.T) {