From 156d47e6964c769561edf7946aa168be6d4d4661 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sun, 28 Mar 2021 08:18:46 -0400 Subject: [PATCH 1/3] Fix: (issue#1254) short options handling needs to proceed from last terminated error --- flag_test.go | 31 +++++++++++++++++++++++++++++++ parse.go | 5 +++-- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/flag_test.go b/flag_test.go index c563d6f346..f662feefa8 100644 --- a/flag_test.go +++ b/flag_test.go @@ -2059,3 +2059,34 @@ func TestTimestampFlagApply_WithDestination(t *testing.T) { expect(t, err, nil) expect(t, *fl.Destination.timestamp, expectedResult) } + +// Test issue #1254 +// StringSlice() with UseShortOptionHandling causes duplicated entries, depending on the ordering of the flags +func TestSliceShortOptionHandle(t *testing.T) { + _ = (&App{ + Commands: []*Command{ + { + Name: "foobar", + UseShortOptionHandling: true, + Action: func(ctx *Context) error { + if ctx.Bool("i") != true { + t.Errorf("bool i not set") + } + if ctx.Bool("t") != true { + t.Errorf("bool i not set") + } + ss := ctx.StringSlice("net") + if !reflect.DeepEqual(ss, []string{"foo"}) { + t.Errorf("Got different slice(%v) than expected", ss) + } + return nil + }, + Flags: []Flag{ + &StringSliceFlag{Name: "net"}, + &BoolFlag{Name: "i"}, + &BoolFlag{Name: "t"}, + }, + }, + }, + }).Run([]string{"run", "foobar", "--net=foo", "-it"}) +} diff --git a/parse.go b/parse.go index 7df17296a4..57df48bc2a 100644 --- a/parse.go +++ b/parse.go @@ -46,8 +46,9 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComple return err } - // swap current argument with the split version - args = append(args[:i], append(shortOpts, args[i+1:]...)...) + // Start processing only from failed argument and not + // from beginning + args = append(shortOpts, args[i+1:]...) argsWereSplit = true break } From dd7065671f833817d58c8f4b9a4c0f46cc4e51c7 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Wed, 28 Apr 2021 20:44:02 -0400 Subject: [PATCH 2/3] Rebase from master --- parse.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/parse.go b/parse.go index 57df48bc2a..7df17296a4 100644 --- a/parse.go +++ b/parse.go @@ -46,9 +46,8 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string, shellComple return err } - // Start processing only from failed argument and not - // from beginning - args = append(shortOpts, args[i+1:]...) + // swap current argument with the split version + args = append(args[:i], append(shortOpts, args[i+1:]...)...) argsWereSplit = true break } From 8c5f1fb359a8f7c2995e823b7248a0ba0b46149f Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Sat, 23 Apr 2022 19:30:34 -0400 Subject: [PATCH 3/3] Changes from code review --- flag_test.go | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/flag_test.go b/flag_test.go index f662feefa8..ecb7847ed3 100644 --- a/flag_test.go +++ b/flag_test.go @@ -2063,17 +2063,19 @@ func TestTimestampFlagApply_WithDestination(t *testing.T) { // Test issue #1254 // StringSlice() with UseShortOptionHandling causes duplicated entries, depending on the ordering of the flags func TestSliceShortOptionHandle(t *testing.T) { - _ = (&App{ + wasCalled := false + err := (&App{ Commands: []*Command{ { Name: "foobar", UseShortOptionHandling: true, Action: func(ctx *Context) error { + wasCalled = true if ctx.Bool("i") != true { - t.Errorf("bool i not set") + t.Error("bool i not set") } if ctx.Bool("t") != true { - t.Errorf("bool i not set") + t.Error("bool i not set") } ss := ctx.StringSlice("net") if !reflect.DeepEqual(ss, []string{"foo"}) { @@ -2089,4 +2091,10 @@ func TestSliceShortOptionHandle(t *testing.T) { }, }, }).Run([]string{"run", "foobar", "--net=foo", "-it"}) + if err != nil { + t.Fatal(err) + } + if !wasCalled { + t.Fatal("Action callback was never called") + } }