From 4381738fb5799d9300598562adbb86c0944d8eb6 Mon Sep 17 00:00:00 2001 From: "[[ BOT ]] Lynn Cyrin" Date: Thu, 22 Aug 2019 20:43:52 -0700 Subject: [PATCH 01/33] wip regression test --- app_test.go | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/app_test.go b/app_test.go index bccede4..a0d17d1 100644 --- a/app_test.go +++ b/app_test.go @@ -1187,6 +1187,43 @@ func TestRequiredFlagAppRunBehavior(t *testing.T) { } } +func TestAppRunPassThroughRegression(t *testing.T) { + tdata := []struct { + testCase string + appFlags []Flag + appRunInput []string + appCommands []Command + expectedAnError bool + }{ + { // docker run --rm ubuntu bash + appRunInput: []string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}, + appCommands: []Command{{ + Name: "myCommand", + Flags: []Flag{StringFlag{Name: "someFlag"}}, + }}, + }, + } + for _, test := range tdata { + t.Run(test.testCase, func(t *testing.T) { + // setup + app := NewApp() + app.Flags = test.appFlags + app.Commands = test.appCommands + + // logic under test + err := app.Run(test.appRunInput) + + // assertions + if test.expectedAnError && err == nil { + t.Errorf("expected an error, but there was none") + } + if !test.expectedAnError && err != nil { + t.Errorf("did not expected an error, but there was one: %s", err) + } + }) + } +} + func TestAppHelpPrinter(t *testing.T) { oldPrinter := HelpPrinter defer func() { From e265f5087fd795a5314286e8e5ec19d91f58f4ae Mon Sep 17 00:00:00 2001 From: "[[ BOT ]] Lynn Cyrin" Date: Thu, 22 Aug 2019 21:22:37 -0700 Subject: [PATCH 02/33] cleanup tests --- app_test.go | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/app_test.go b/app_test.go index a0d17d1..9caa543 100644 --- a/app_test.go +++ b/app_test.go @@ -1189,13 +1189,12 @@ func TestRequiredFlagAppRunBehavior(t *testing.T) { func TestAppRunPassThroughRegression(t *testing.T) { tdata := []struct { - testCase string - appFlags []Flag - appRunInput []string - appCommands []Command - expectedAnError bool + testCase string + appRunInput []string + appCommands []Command }{ - { // docker run --rm ubuntu bash + { + testCase: "test_case", appRunInput: []string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}, appCommands: []Command{{ Name: "myCommand", @@ -1207,17 +1206,13 @@ func TestAppRunPassThroughRegression(t *testing.T) { t.Run(test.testCase, func(t *testing.T) { // setup app := NewApp() - app.Flags = test.appFlags app.Commands = test.appCommands // logic under test err := app.Run(test.appRunInput) // assertions - if test.expectedAnError && err == nil { - t.Errorf("expected an error, but there was none") - } - if !test.expectedAnError && err != nil { + if err != nil { t.Errorf("did not expected an error, but there was one: %s", err) } }) From 6e8917398c47b08f8069779668b8767f24abe559 Mon Sep 17 00:00:00 2001 From: "[[ BOT ]] Lynn Cyrin" Date: Thu, 22 Aug 2019 21:23:34 -0700 Subject: [PATCH 03/33] comment out tests --- app_test.go | 62 ++++++++++++++++++++++++++--------------------------- 1 file changed, 31 insertions(+), 31 deletions(-) diff --git a/app_test.go b/app_test.go index 9caa543..84329ee 100644 --- a/app_test.go +++ b/app_test.go @@ -1187,37 +1187,37 @@ func TestRequiredFlagAppRunBehavior(t *testing.T) { } } -func TestAppRunPassThroughRegression(t *testing.T) { - tdata := []struct { - testCase string - appRunInput []string - appCommands []Command - }{ - { - testCase: "test_case", - appRunInput: []string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}, - appCommands: []Command{{ - Name: "myCommand", - Flags: []Flag{StringFlag{Name: "someFlag"}}, - }}, - }, - } - for _, test := range tdata { - t.Run(test.testCase, func(t *testing.T) { - // setup - app := NewApp() - app.Commands = test.appCommands - - // logic under test - err := app.Run(test.appRunInput) - - // assertions - if err != nil { - t.Errorf("did not expected an error, but there was one: %s", err) - } - }) - } -} +// func TestAppRunPassThroughRegression(t *testing.T) { +// tdata := []struct { +// testCase string +// appRunInput []string +// appCommands []Command +// }{ +// { +// testCase: "test_case", +// appRunInput: []string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}, +// appCommands: []Command{{ +// Name: "myCommand", +// Flags: []Flag{StringFlag{Name: "someFlag"}}, +// }}, +// }, +// } +// for _, test := range tdata { +// t.Run(test.testCase, func(t *testing.T) { +// // setup +// app := NewApp() +// app.Commands = test.appCommands + +// // logic under test +// err := app.Run(test.appRunInput) + +// // assertions +// if err != nil { +// t.Errorf("did not expected an error, but there was one: %s", err) +// } +// }) +// } +// } func TestAppHelpPrinter(t *testing.T) { oldPrinter := HelpPrinter From 74cd3bc3fe03403d513c2084c48f7cbdd1d5bba6 Mon Sep 17 00:00:00 2001 From: "[[ BOT ]] Lynn Cyrin" Date: Thu, 22 Aug 2019 21:29:33 -0700 Subject: [PATCH 04/33] cleanup tests --- app_test.go | 47 ++++++++++++++++------------------------------- 1 file changed, 16 insertions(+), 31 deletions(-) diff --git a/app_test.go b/app_test.go index 84329ee..37950f8 100644 --- a/app_test.go +++ b/app_test.go @@ -1187,37 +1187,22 @@ func TestRequiredFlagAppRunBehavior(t *testing.T) { } } -// func TestAppRunPassThroughRegression(t *testing.T) { -// tdata := []struct { -// testCase string -// appRunInput []string -// appCommands []Command -// }{ -// { -// testCase: "test_case", -// appRunInput: []string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}, -// appCommands: []Command{{ -// Name: "myCommand", -// Flags: []Flag{StringFlag{Name: "someFlag"}}, -// }}, -// }, -// } -// for _, test := range tdata { -// t.Run(test.testCase, func(t *testing.T) { -// // setup -// app := NewApp() -// app.Commands = test.appCommands - -// // logic under test -// err := app.Run(test.appRunInput) - -// // assertions -// if err != nil { -// t.Errorf("did not expected an error, but there was one: %s", err) -// } -// }) -// } -// } +func TestRegression(t *testing.T) { + // setup + app := NewApp() + app.Commands = []Command{{ + Name: "myCommand", + Flags: []Flag{StringFlag{Name: "someFlag"}}, + }} + + // logic under test + err := app.Run([]string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}) + + // assertions + if err != nil { + t.Errorf("did not expected an error, but there was one: %s", err) + } +} func TestAppHelpPrinter(t *testing.T) { oldPrinter := HelpPrinter From 4fb1878febcd7f6e4b8c8b29b5ba9e7dfb77b370 Mon Sep 17 00:00:00 2001 From: "[[ BOT ]] Lynn Cyrin" Date: Thu, 22 Aug 2019 21:42:07 -0700 Subject: [PATCH 05/33] show test failures --- build.go | 68 +++++++++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 57 insertions(+), 11 deletions(-) diff --git a/build.go b/build.go index 2a38f8b..125d25f 100644 --- a/build.go +++ b/build.go @@ -6,12 +6,13 @@ import ( "bufio" "bytes" "fmt" - "github.com/urfave/cli" "io/ioutil" "log" "os" "os/exec" "strings" + + "github.com/urfave/cli" ) var packages = []string{"cli", "altsrc"} @@ -52,7 +53,13 @@ func main() { } func VetActionFunc(_ *cli.Context) error { - return exec.Command("go", "vet").Run() + cmd := exec.Command("go", "vet") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + return cmd.Run() } func TestActionFunc(c *cli.Context) error { @@ -67,10 +74,13 @@ func TestActionFunc(c *cli.Context) error { coverProfile := fmt.Sprintf("--coverprofile=%s.coverprofile", pkg) - err := exec.Command( - "go", "test", "-v", coverProfile, packageName, - ).Run() + cmd := exec.Command("go", "test", "-v", coverProfile, packageName) + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err := cmd.Run() if err != nil { return err } @@ -142,16 +152,34 @@ func GfmrunActionFunc(_ *cli.Context) error { return err } - return exec.Command("gfmrun", "-c", fmt.Sprint(counter), "-s", "README.md").Run() + cmd := exec.Command("gfmrun", "-c", fmt.Sprint(counter), "-s", "README.md") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + return cmd.Run() } func TocActionFunc(_ *cli.Context) error { - err := exec.Command("node_modules/.bin/markdown-toc", "-i", "README.md").Run() + cmd := exec.Command("node_modules/.bin/markdown-toc", "-i", "README.md") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err := cmd.Run() if err != nil { return err } - err = exec.Command("git", "diff", "--exit-code").Run() + cmd = exec.Command("git", "diff", "--exit-code") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err = cmd.Run() if err != nil { return err } @@ -160,17 +188,35 @@ func TocActionFunc(_ *cli.Context) error { } func GenActionFunc(_ *cli.Context) error { - err := exec.Command("go", "generate", "flag-gen/main.go").Run() + cmd := exec.Command("go", "generate", "flag-gen/main.go") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err := cmd.Run() if err != nil { return err } - err = exec.Command("go", "generate", "cli.go").Run() + cmd = exec.Command("go", "generate", "cli.go") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err = cmd.Run() if err != nil { return err } - err = exec.Command("git", "diff", "--exit-code").Run() + cmd = exec.Command("git", "diff", "--exit-code") + + cmd.Stdin = os.Stdin + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + + err = cmd.Run() if err != nil { return err } From 15dc34ea12041906545f1466fd1af97c8f97ab69 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 08:54:06 -0700 Subject: [PATCH 06/33] more accurate regression reproduction --- app_test.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/app_test.go b/app_test.go index 37950f8..2006bb8 100644 --- a/app_test.go +++ b/app_test.go @@ -1191,12 +1191,17 @@ func TestRegression(t *testing.T) { // setup app := NewApp() app.Commands = []Command{{ - Name: "myCommand", - Flags: []Flag{StringFlag{Name: "someFlag"}}, + Name: "command", + Flags: []Flag{ + StringFlag{ + Name: "flagone", + }, + }, + Action: func(c *Context) error { return nil }, }} // logic under test - err := app.Run([]string{"myCLI", "myCommand", "--someFlag", "someInput", "docker", "run", "--rm", "ubuntu", "bash"}) + err := app.Run([]string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}) // assertions if err != nil { From c542fb3bed0cbf052655db02a80ea97f03383421 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 09:04:51 -0700 Subject: [PATCH 07/33] temp remove --- app_test.go | 22 ---------------------- 1 file changed, 22 deletions(-) diff --git a/app_test.go b/app_test.go index 2006bb8..bccede4 100644 --- a/app_test.go +++ b/app_test.go @@ -1187,28 +1187,6 @@ func TestRequiredFlagAppRunBehavior(t *testing.T) { } } -func TestRegression(t *testing.T) { - // setup - app := NewApp() - app.Commands = []Command{{ - Name: "command", - Flags: []Flag{ - StringFlag{ - Name: "flagone", - }, - }, - Action: func(c *Context) error { return nil }, - }} - - // logic under test - err := app.Run([]string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}) - - // assertions - if err != nil { - t.Errorf("did not expected an error, but there was one: %s", err) - } -} - func TestAppHelpPrinter(t *testing.T) { oldPrinter := HelpPrinter defer func() { From b0ed044b01bed3c0e940840aeb9d314ffa117330 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 09:16:41 -0700 Subject: [PATCH 08/33] add regression test to its own file --- app_regression_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 app_regression_test.go diff --git a/app_regression_test.go b/app_regression_test.go new file mode 100644 index 0000000..91e65aa --- /dev/null +++ b/app_regression_test.go @@ -0,0 +1,29 @@ +package cli + +import ( + "testing" +) + +// TestRegression tests a regression that was merged between versions 1.20.0 and 1.21.0 +// The included app.Run line worked in 1.20.0, and then was broken in 1.21.0. +func TestRegression(t *testing.T) { + // setup + app := NewApp() + app.Commands = []Command{{ + Name: "command", + Flags: []Flag{ + StringFlag{ + Name: "flagone", + }, + }, + Action: func(c *Context) error { return nil }, + }} + + // logic under test + err := app.Run([]string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}) + + // assertions + if err != nil { + t.Errorf("did not expected an error, but there was one: %s", err) + } +} From 192ce003d9a36d0002b487f8d725e520f1db51cc Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 09:17:16 -0700 Subject: [PATCH 09/33] expand name --- app_regression_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app_regression_test.go b/app_regression_test.go index 91e65aa..1dff458 100644 --- a/app_regression_test.go +++ b/app_regression_test.go @@ -6,7 +6,7 @@ import ( // TestRegression tests a regression that was merged between versions 1.20.0 and 1.21.0 // The included app.Run line worked in 1.20.0, and then was broken in 1.21.0. -func TestRegression(t *testing.T) { +func TestVersionOneTwoOneRegression(t *testing.T) { // setup app := NewApp() app.Commands = []Command{{ From 2172d382b6adcde4289a1ffcad9797c2f880da35 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 10:18:00 -0700 Subject: [PATCH 10/33] update tests --- app_regression_test.go | 49 +++++++++++++++++++++++++++++------------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/app_regression_test.go b/app_regression_test.go index 1dff458..dde2a1a 100644 --- a/app_regression_test.go +++ b/app_regression_test.go @@ -6,24 +6,43 @@ import ( // TestRegression tests a regression that was merged between versions 1.20.0 and 1.21.0 // The included app.Run line worked in 1.20.0, and then was broken in 1.21.0. +// Relevant PR: https://github.com/urfave/cli/pull/872 func TestVersionOneTwoOneRegression(t *testing.T) { - // setup - app := NewApp() - app.Commands = []Command{{ - Name: "command", - Flags: []Flag{ - StringFlag{ - Name: "flagone", - }, + testData := []struct { + testCase string + appRunInput []string + }{ + // assertion: empty input, when a required flag is present, errors + { + testCase: "with_dash_dash", + appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "--", "docker", "image", "ls", "--no-trunc"}, }, - Action: func(c *Context) error { return nil }, - }} + { + testCase: "without_dash_dash", + appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}, + }, + } + for _, test := range testData { + t.Run(test.testCase, func(t *testing.T) { + // setup + app := NewApp() + app.Commands = []Command{{ + Name: "command", + Flags: []Flag{ + StringFlag{ + Name: "flagone", + }, + }, + Action: func(c *Context) error { return nil }, + }} - // logic under test - err := app.Run([]string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}) + // logic under test + err := app.Run(test.appRunInput) - // assertions - if err != nil { - t.Errorf("did not expected an error, but there was one: %s", err) + // assertions + if err != nil { + t.Errorf("did not expected an error, but there was one: %s", err) + } + }) } } From 810b9714d3eb5b8cd41a62527c39fb2730c6d151 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 10:34:15 -0700 Subject: [PATCH 11/33] cleanup --- app_regression_test.go | 1 - 1 file changed, 1 deletion(-) diff --git a/app_regression_test.go b/app_regression_test.go index dde2a1a..2299df5 100644 --- a/app_regression_test.go +++ b/app_regression_test.go @@ -12,7 +12,6 @@ func TestVersionOneTwoOneRegression(t *testing.T) { testCase string appRunInput []string }{ - // assertion: empty input, when a required flag is present, errors { testCase: "with_dash_dash", appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "--", "docker", "image", "ls", "--no-trunc"}, From 0f9d8a9abdbb755069f45e89d90b3e06499302c2 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 11:14:20 -0700 Subject: [PATCH 12/33] expand test cases --- app_regression_test.go | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/app_regression_test.go b/app_regression_test.go index 2299df5..3c8681b 100644 --- a/app_regression_test.go +++ b/app_regression_test.go @@ -9,24 +9,36 @@ import ( // Relevant PR: https://github.com/urfave/cli/pull/872 func TestVersionOneTwoOneRegression(t *testing.T) { testData := []struct { - testCase string - appRunInput []string + testCase string + appRunInput []string + skipArgReorder bool }{ { testCase: "with_dash_dash", appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "--", "docker", "image", "ls", "--no-trunc"}, }, + { + testCase: "with_dash_dash_and_skip_reorder", + appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "--", "docker", "image", "ls", "--no-trunc"}, + skipArgReorder: true, + }, { testCase: "without_dash_dash", appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}, }, + { + testCase: "without_dash_dash_and_skip_reorder", + appRunInput: []string{"cli", "command", "--flagone", "flagvalue", "docker", "image", "ls", "--no-trunc"}, + skipArgReorder: true, + }, } for _, test := range testData { t.Run(test.testCase, func(t *testing.T) { // setup app := NewApp() app.Commands = []Command{{ - Name: "command", + Name: "command", + SkipArgReorder: test.skipArgReorder, Flags: []Flag{ StringFlag{ Name: "flagone", From 406a1c31b08754a4553634c1fbb0caaed22865b1 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Sun, 25 Aug 2019 14:12:13 -0700 Subject: [PATCH 13/33] update vars and args --- command.go | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/command.go b/command.go index 44a90de..37387d1 100644 --- a/command.go +++ b/command.go @@ -190,7 +190,7 @@ func (c *Command) parseFlags(args Args) (*flag.FlagSet, error) { } if !c.SkipArgReorder { - args = reorderArgs(args) + args = reorderArgs(c.Flags, args) } set, err := parseIter(c, args) @@ -214,34 +214,36 @@ func (c *Command) useShortOptionHandling() bool { return c.UseShortOptionHandling } -// reorderArgs moves all flags before arguments as this is what flag expects -func reorderArgs(args []string) []string { - var nonflags, flags []string +// reorderArgs moves all flags (reorderedFlags) before arguments (remainingArgs) +// as this is what flag expects. +func reorderArgs(commandFlags []Flag, args []string) []string { + var remainingArgs []string + var reorderedFlags []string readFlagValue := false for i, arg := range args { if arg == "--" { - nonflags = append(nonflags, args[i:]...) + remainingArgs = append(remainingArgs, args[i:]...) break } if readFlagValue && !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") { readFlagValue = false - flags = append(flags, arg) + reorderedFlags = append(reorderedFlags, arg) continue } readFlagValue = false if arg != "-" && strings.HasPrefix(arg, "-") { - flags = append(flags, arg) + reorderedFlags = append(reorderedFlags, arg) readFlagValue = !strings.Contains(arg, "=") } else { - nonflags = append(nonflags, arg) + remainingArgs = append(remainingArgs, arg) } } - return append(flags, nonflags...) + return append(reorderedFlags, remainingArgs...) } // Names returns the names including short names and aliases. From 2071bcfb8328cb13e5575a586f06a33563b04054 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Mon, 26 Aug 2019 00:18:36 -0700 Subject: [PATCH 14/33] checkpoint --- command.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/command.go b/command.go index 37387d1..d33435e 100644 --- a/command.go +++ b/command.go @@ -214,14 +214,18 @@ func (c *Command) useShortOptionHandling() bool { return c.UseShortOptionHandling } -// reorderArgs moves all flags (reorderedFlags) before arguments (remainingArgs) -// as this is what flag expects. +// reorderArgs moves all flags (via reorderedArgs) before the rest of +// the arguments (remainingArgs) as this is what flag expects. func reorderArgs(commandFlags []Flag, args []string) []string { var remainingArgs []string - var reorderedFlags []string + var reorderedArgs []string readFlagValue := false for i, arg := range args { + + // dont reorder any args after a -- + // read about -- here: + // https://unix.stackexchange.com/questions/11376/what-does-double-dash-mean-also-known-as-bare-double-dash if arg == "--" { remainingArgs = append(remainingArgs, args[i:]...) break @@ -229,21 +233,22 @@ func reorderArgs(commandFlags []Flag, args []string) []string { if readFlagValue && !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") { readFlagValue = false - reorderedFlags = append(reorderedFlags, arg) + reorderedArgs = append(reorderedArgs, arg) continue } readFlagValue = false if arg != "-" && strings.HasPrefix(arg, "-") { - reorderedFlags = append(reorderedFlags, arg) + reorderedArgs = append(reorderedArgs, arg) readFlagValue = !strings.Contains(arg, "=") - } else { - remainingArgs = append(remainingArgs, arg) + continue } + + remainingArgs = append(remainingArgs, arg) } - return append(reorderedFlags, remainingArgs...) + return append(reorderedArgs, remainingArgs...) } // Names returns the names including short names and aliases. From 7d0751fa1309a01b405cde327cc43275d13016a2 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 19:25:51 -0700 Subject: [PATCH 15/33] add argIsFlag check --- command.go | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/command.go b/command.go index d33435e..6471f10 100644 --- a/command.go +++ b/command.go @@ -238,7 +238,7 @@ func reorderArgs(commandFlags []Flag, args []string) []string { } readFlagValue = false - if arg != "-" && strings.HasPrefix(arg, "-") { + if arg != "-" && strings.HasPrefix(arg, "-") && argIsFlag(commandFlags, arg) { reorderedArgs = append(reorderedArgs, arg) readFlagValue = !strings.Contains(arg, "=") @@ -251,6 +251,18 @@ func reorderArgs(commandFlags []Flag, args []string) []string { return append(reorderedArgs, remainingArgs...) } +func argIsFlag(commandFlags []Flag, arg string) bool { + strippedArg := strings.ReplaceAll(arg, "-", "") + for _, flag := range commandFlags { + for _, key := range strings.Split(flag.GetName(), ",") { + if key == strippedArg { + return true + } + } + } + return false +} + // Names returns the names including short names and aliases. func (c Command) Names() []string { names := []string{c.Name} From 49dbeed6877737d97ec6e121763a41c617d7b272 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 20:24:42 -0700 Subject: [PATCH 16/33] handle `=` input --- command.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/command.go b/command.go index 6471f10..d52080e 100644 --- a/command.go +++ b/command.go @@ -252,10 +252,11 @@ func reorderArgs(commandFlags []Flag, args []string) []string { } func argIsFlag(commandFlags []Flag, arg string) bool { - strippedArg := strings.ReplaceAll(arg, "-", "") + arg = strings.ReplaceAll(arg, "-", "") + arg = strings.Split(arg, "=")[0] for _, flag := range commandFlags { for _, key := range strings.Split(flag.GetName(), ",") { - if key == strippedArg { + if key == arg { return true } } From 10682fbde6f00b1008d6748d7ffb090bce421da7 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 20:28:48 -0700 Subject: [PATCH 17/33] docs --- command.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/command.go b/command.go index d52080e..af91dbc 100644 --- a/command.go +++ b/command.go @@ -252,8 +252,11 @@ func reorderArgs(commandFlags []Flag, args []string) []string { } func argIsFlag(commandFlags []Flag, arg string) bool { + // this line turns `--flag` into `flag` arg = strings.ReplaceAll(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 commandFlags { for _, key := range strings.Split(flag.GetName(), ",") { if key == arg { From 09cdbbfe281fd79e92616a28f1fdf701e9007c22 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 20:29:31 -0700 Subject: [PATCH 18/33] more docs --- command.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command.go b/command.go index af91dbc..6e09a32 100644 --- a/command.go +++ b/command.go @@ -264,6 +264,7 @@ func argIsFlag(commandFlags []Flag, arg string) bool { } } } + // return false if this arg was not one of our flags return false } From 223e21796c35c3f4a456db83c0d450ef5fe4db01 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 21:08:52 -0700 Subject: [PATCH 19/33] swap a test case --- command_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command_test.go b/command_test.go index 56698fe..47792e1 100644 --- a/command_test.go +++ b/command_test.go @@ -18,7 +18,7 @@ func TestCommandFlagParsing(t *testing.T) { UseShortOptionHandling bool }{ // Test normal "not ignoring flags" flow - {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, errors.New("flag provided but not defined: -break"), false}, + {[]string{"test-cmd", "blah", "blah", "-break"}, false, false, nil, false}, // Test no arg reorder {[]string{"test-cmd", "blah", "blah", "-break"}, false, true, nil, false}, From 6da413ad825dea1f74563bdfee1f8e5c23c1c650 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 22:45:51 -0700 Subject: [PATCH 20/33] fix go version support? --- command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command.go b/command.go index 6e09a32..0a6b6ed 100644 --- a/command.go +++ b/command.go @@ -253,7 +253,7 @@ func reorderArgs(commandFlags []Flag, args []string) []string { func argIsFlag(commandFlags []Flag, arg string) bool { // this line turns `--flag` into `flag` - arg = strings.ReplaceAll(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 From bfdd794eb3d0f8d99af8939a3a8c24547ef262ca Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Wed, 11 Sep 2019 23:05:26 -0700 Subject: [PATCH 21/33] docs --- command.go | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/command.go b/command.go index 0a6b6ed..3993e0f 100644 --- a/command.go +++ b/command.go @@ -220,7 +220,7 @@ func reorderArgs(commandFlags []Flag, args []string) []string { var remainingArgs []string var reorderedArgs []string - readFlagValue := false + nextIndexMayContainValue := false for i, arg := range args { // dont reorder any args after a -- @@ -229,28 +229,30 @@ func reorderArgs(commandFlags []Flag, args []string) []string { if arg == "--" { remainingArgs = append(remainingArgs, args[i:]...) break - } - if readFlagValue && !strings.HasPrefix(arg, "-") && !strings.HasPrefix(arg, "--") { - readFlagValue = false + // checks if this arg is a value that should be re-ordered next to its associated flag + } else if nextIndexMayContainValue && !strings.HasPrefix(arg, "-") { + nextIndexMayContainValue = false reorderedArgs = append(reorderedArgs, arg) - continue - } - readFlagValue = false - if arg != "-" && strings.HasPrefix(arg, "-") && argIsFlag(commandFlags, arg) { + // checks if this is an arg that should be re-ordered + } else if arg != "-" && strings.HasPrefix(arg, "-") && argIsFlag(commandFlags, arg) { + + // 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, "=") - readFlagValue = !strings.Contains(arg, "=") - continue + // simply append any remaining args + } else { + remainingArgs = append(remainingArgs, arg) } - - remainingArgs = append(remainingArgs, arg) } return append(reorderedArgs, remainingArgs...) } +// argIsFlag checks if an arg is one of our command flags func argIsFlag(commandFlags []Flag, arg string) bool { // this line turns `--flag` into `flag` arg = strings.Replace(arg, "-", "", -1) From 3f6f97754aa8af1b6bd7b9324bf1e4c72023a269 Mon Sep 17 00:00:00 2001 From: "Lynn Cyrin (they/them)" Date: Fri, 13 Sep 2019 07:27:16 -0700 Subject: [PATCH 22/33] Update command.go Co-Authored-By: Sascha Grunert --- command.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/command.go b/command.go index 3993e0f..037abe9 100644 --- a/command.go +++ b/command.go @@ -217,7 +217,7 @@ func (c *Command) useShortOptionHandling() bool { // reorderArgs moves all flags (via reorderedArgs) before the rest of // the arguments (remainingArgs) as this is what flag expects. func reorderArgs(commandFlags []Flag, args []string) []string { - var remainingArgs []string + var remainingArgs, reorderedArgs []string var reorderedArgs []string nextIndexMayContainValue := false From 72c249389c590a6817f21cf21c68bc0a076823bf Mon Sep 17 00:00:00 2001 From: "Lynn Cyrin (they/them)" Date: Fri, 13 Sep 2019 07:35:16 -0700 Subject: [PATCH 23/33] Update command.go --- command.go | 1 - 1 file changed, 1 deletion(-) diff --git a/command.go b/command.go index 037abe9..9724a3b 100644 --- a/command.go +++ b/command.go @@ -218,7 +218,6 @@ func (c *Command) useShortOptionHandling() bool { // the arguments (remainingArgs) as this is what flag expects. func reorderArgs(commandFlags []Flag, args []string) []string { var remainingArgs, reorderedArgs []string - var reorderedArgs []string nextIndexMayContainValue := false for i, arg := range args { From 51259808fff1f7384b556f6e127a090fe0362305 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Tue, 1 Oct 2019 20:13:04 -0700 Subject: [PATCH 24/33] better leading dash handling --- command.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/command.go b/command.go index dcaf2b4..70502e8 100644 --- a/command.go +++ b/command.go @@ -240,8 +240,7 @@ func reorderArgs(commandFlags []Flag, args []string) []string { reorderedArgs = append(reorderedArgs, arg) // checks if this is an arg that should be re-ordered - } else if arg != "-" && strings.HasPrefix(arg, "-") && argIsFlag(commandFlags, arg) { - + } else if argIsFlag(commandFlags, arg) { // 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 @@ -258,8 +257,18 @@ func reorderArgs(commandFlags []Flag, args []string) []string { // argIsFlag checks if an arg is one of our command flags func argIsFlag(commandFlags []Flag, arg string) bool { + // checks if this is just a `-`, and so definitely not a flag + if arg == "-" { + return false + } // this line turns `--flag` into `flag` - arg = strings.Replace(arg, "-", "", -1) + 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 From be0cc4b8715d2a57f03b395ae928db61048bb679 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Tue, 1 Oct 2019 20:18:53 -0700 Subject: [PATCH 25/33] add a check --- command.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/command.go b/command.go index 70502e8..d41a1bf 100644 --- a/command.go +++ b/command.go @@ -261,6 +261,10 @@ func argIsFlag(commandFlags []Flag, arg string) bool { if arg == "-" { return false } + // flags always start with a - + if !strings.HasPrefix(arg, "-") { + return false + } // this line turns `--flag` into `flag` if strings.HasPrefix(arg, "--") { arg = strings.Replace(arg, "-", "", 2) From 64d3555482218336121cd318c27454041cc479a1 Mon Sep 17 00:00:00 2001 From: Lynn Cyrin Date: Tue, 1 Oct 2019 20:21:12 -0700 Subject: [PATCH 26/33] trim whitespace --- command.go | 1 + 1 file changed, 1 insertion(+) diff --git a/command.go b/command.go index d41a1bf..e7cb97a 100644 --- a/command.go +++ b/command.go @@ -278,6 +278,7 @@ func argIsFlag(commandFlags []Flag, arg string) bool { // look through all the flags, to see if the `arg` is one of our flags for _, flag := range commandFlags { for _, key := range strings.Split(flag.GetName(), ",") { + key := strings.TrimSpace(key) if key == arg { return true } From df740f656281131043563a9a3b92f83a2e495fa7 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Fri, 11 Oct 2019 22:32:21 -0400 Subject: [PATCH 27/33] Fix infinite loop flag parsing with short options --- command_test.go | 5 +++++ parse.go | 17 ++++++++--------- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/command_test.go b/command_test.go index 6235b41..66859c0 100644 --- a/command_test.go +++ b/command_test.go @@ -70,8 +70,13 @@ func TestParseAndRunShortOpts(t *testing.T) { {[]string{"foo", "test", "-af"}, nil, []string{}}, {[]string{"foo", "test", "-cf"}, nil, []string{}}, {[]string{"foo", "test", "-acf"}, nil, []string{}}, + {[]string{"foo", "test", "--acf"}, errors.New("flag provided but not defined: -acf"), nil}, {[]string{"foo", "test", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, + {[]string{"foo", "test", "-acf", "-invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, + {[]string{"foo", "test", "--invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, + {[]string{"foo", "test", "-acf", "--invalid"}, errors.New("flag provided but not defined: -invalid"), nil}, {[]string{"foo", "test", "-acf", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}}, + {[]string{"foo", "test", "-acf", "arg1", "--invalid"}, nil, []string{"arg1", "--invalid"}}, {[]string{"foo", "test", "-acfi", "not-arg", "arg1", "-invalid"}, nil, []string{"arg1", "-invalid"}}, {[]string{"foo", "test", "-i", "ivalue"}, nil, []string{}}, {[]string{"foo", "test", "-i", "ivalue", "arg1"}, nil, []string{"arg1"}}, diff --git a/parse.go b/parse.go index 2c2005c..cab99e5 100644 --- a/parse.go +++ b/parse.go @@ -22,28 +22,27 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { } errStr := err.Error() - trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: ") + trimmed := strings.TrimPrefix(errStr, "flag provided but not defined: -") if errStr == trimmed { return err } // regenerate the initial args with the split short opts - newArgs := []string{} for i, arg := range args { - if arg != trimmed { - newArgs = append(newArgs, arg) + // skip args that are not part of the error message + if name := strings.TrimLeft(arg, "-"); name != trimmed { continue } - shortOpts := splitShortOptions(set, trimmed) + // if we can't split, the error was accurate + shortOpts := splitShortOptions(set, arg) if len(shortOpts) == 1 { return err } - // add each short option and all remaining arguments - newArgs = append(newArgs, shortOpts...) - newArgs = append(newArgs, args[i+1:]...) - args = newArgs + // swap current argument with the split version + args = append(args[:i], append(shortOpts, args[i+1:]...)...) + break } // Since custom parsing failed, replace the flag set before retrying From f5306b622f216cafde2a670b9a9e620cb6b0fad1 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Fri, 11 Oct 2019 22:57:55 -0400 Subject: [PATCH 28/33] Ensure infinite loop cannot occur in parsing --- parse.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/parse.go b/parse.go index cab99e5..660f538 100644 --- a/parse.go +++ b/parse.go @@ -28,6 +28,7 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { } // regenerate the initial args with the split short opts + argsWereSplit := false for i, arg := range args { // skip args that are not part of the error message if name := strings.TrimLeft(arg, "-"); name != trimmed { @@ -42,9 +43,16 @@ func parseIter(set *flag.FlagSet, ip iterativeParser, args []string) error { // swap current argument with the split version args = append(args[:i], append(shortOpts, args[i+1:]...)...) + argsWereSplit = true break } + // This should be an impossible to reach code path, but in case the arg + // splitting failed to happen, this will prevent infinite loops + if !argsWereSplit { + return err + } + // Since custom parsing failed, replace the flag set before retrying newSet, err := ip.newFlagSet() if err != nil { From e2a8dfe314fa429e04a63152ee3c97806214095c Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Sun, 27 Jan 2019 10:38:23 -0500 Subject: [PATCH 29/33] Decouple help printer for custom templates --- help.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/help.go b/help.go index 7a4ef69..06c526f 100644 --- a/help.go +++ b/help.go @@ -187,7 +187,7 @@ func ShowCommandHelp(ctx *Context, command string) error { for _, c := range ctx.App.Commands { if c.HasName(command) { if c.CustomHelpTemplate != "" { - HelpPrinterCustom(ctx.App.Writer, c.CustomHelpTemplate, c, nil) + HelpPrinter(ctx.App.Writer, c.CustomHelpTemplate, c) } else { HelpPrinter(ctx.App.Writer, CommandHelpTemplate, c) } @@ -261,7 +261,7 @@ func printHelpCustom(out io.Writer, templ string, data interface{}, customFunc m } func printHelp(out io.Writer, templ string, data interface{}) { - printHelpCustom(out, templ, data, nil) + HelpPrinterCustom(out, templ, data, nil) } func checkVersion(c *Context) bool { From b52cca036aadab37c86db59b0e04b1a79d7ac380 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Wed, 16 Oct 2019 07:24:21 -0400 Subject: [PATCH 30/33] Simplify duplicated function call --- help.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/help.go b/help.go index 06c526f..01c19aa 100644 --- a/help.go +++ b/help.go @@ -186,11 +186,13 @@ func ShowCommandHelp(ctx *Context, command string) error { for _, c := range ctx.App.Commands { if c.HasName(command) { - if c.CustomHelpTemplate != "" { - HelpPrinter(ctx.App.Writer, c.CustomHelpTemplate, c) - } else { - HelpPrinter(ctx.App.Writer, CommandHelpTemplate, c) + templ := c.CustomHelpTemplate + if templ == "" { + templ = CommandHelpTemplate } + + HelpPrinter(ctx.App.Writer, templ, c) + return nil } } From a14194aa33517e16dd5884625da8723c5d77acc8 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Wed, 16 Oct 2019 07:57:33 -0400 Subject: [PATCH 31/33] Improve documentation for HelpPrinter/HelpPrinterCustom --- help.go | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/help.go b/help.go index 01c19aa..d5f1c87 100644 --- a/help.go +++ b/help.go @@ -47,13 +47,15 @@ type helpPrinter func(w io.Writer, templ string, data interface{}) // Prints help for the App or Command with custom template function. type helpPrinterCustom func(w io.Writer, templ string, data interface{}, customFunc map[string]interface{}) -// HelpPrinter is a function that writes the help output. If not set a default -// is used. The function signature is: -// func(w io.Writer, templ string, data interface{}) +// HelpPrinter is a function that writes the help output. If not set explicitly, +// this calls HelpPrinterCustom using only the default template functions. var HelpPrinter helpPrinter = printHelp -// HelpPrinterCustom is same as HelpPrinter but -// takes a custom function for template function map. +// HelpPrinterCustom is a function that writes the help output. If not set +// explicitly, a default is used. +// +// The customFuncs map will be combined with a default template.FuncMap to +// allow using arbitrary functions in template rendering. var HelpPrinterCustom helpPrinterCustom = printHelpCustom // VersionPrinter prints the version for the App @@ -240,11 +242,11 @@ func ShowCommandCompletions(ctx *Context, command string) { } -func printHelpCustom(out io.Writer, templ string, data interface{}, customFunc map[string]interface{}) { +func printHelpCustom(out io.Writer, templ string, data interface{}, customFuncs map[string]interface{}) { funcMap := template.FuncMap{ "join": strings.Join, } - for key, value := range customFunc { + for key, value := range customFuncs { funcMap[key] = value } From bab428af09e0373e1deda9dec24d30cc4c5c7026 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Thu, 17 Oct 2019 05:12:17 -0400 Subject: [PATCH 32/33] Ensure command help always prints with overridden print functions --- help_test.go | 174 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/help_test.go b/help_test.go index b13eef4..870b32e 100644 --- a/help_test.go +++ b/help_test.go @@ -4,6 +4,7 @@ import ( "bytes" "flag" "fmt" + "io" "runtime" "strings" "testing" @@ -210,6 +211,179 @@ func TestShowAppHelp_CommandAliases(t *testing.T) { } } +func TestShowCommandHelp_HelpPrinter(t *testing.T) { + doublecho := func(text string) string { + return text + " " + text + } + + tests := []struct { + name string + template string + printer helpPrinter + command string + wantTemplate string + wantOutput string + }{ + { + name: "no-command", + template: "", + printer: func(w io.Writer, templ string, data interface{}) { + fmt.Fprint(w, "yo") + }, + command: "", + wantTemplate: SubcommandHelpTemplate, + wantOutput: "yo", + }, + { + name: "standard-command", + template: "", + printer: func(w io.Writer, templ string, data interface{}) { + fmt.Fprint(w, "yo") + }, + command: "my-command", + wantTemplate: CommandHelpTemplate, + wantOutput: "yo", + }, + { + name: "custom-template-command", + template: "{{doublecho .Name}}", + printer: func(w io.Writer, templ string, data interface{}) { + // Pass a custom function to ensure it gets used + fm := map[string]interface{}{"doublecho": doublecho} + HelpPrinterCustom(w, templ, data, fm) + }, + command: "my-command", + wantTemplate: "{{doublecho .Name}}", + wantOutput: "my-command my-command", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func(old helpPrinter) { + HelpPrinter = old + }(HelpPrinter) + HelpPrinter = func(w io.Writer, templ string, data interface{}) { + if templ != tt.wantTemplate { + t.Errorf("want template:\n%s\ngot template:\n%s", tt.wantTemplate, templ) + } + + tt.printer(w, templ, data) + } + + var buf bytes.Buffer + app := &App{ + Name: "my-app", + Writer: &buf, + Commands: []Command{ + { + Name: "my-command", + CustomHelpTemplate: tt.template, + }, + }, + } + + err := app.Run([]string{"my-app", "help", tt.command}) + if err != nil { + t.Fatal(err) + } + + got := buf.String() + if got != tt.wantOutput { + t.Errorf("want output %q, got %q", tt.wantOutput, got) + } + }) + } +} +func TestShowCommandHelp_HelpPrinterCustom(t *testing.T) { + doublecho := func(text string) string { + return text + " " + text + } + + tests := []struct { + name string + template string + printer helpPrinterCustom + command string + wantTemplate string + wantOutput string + }{ + { + name: "no-command", + template: "", + printer: func(w io.Writer, templ string, data interface{}, fm map[string]interface{}) { + fmt.Fprint(w, "yo") + }, + command: "", + wantTemplate: SubcommandHelpTemplate, + wantOutput: "yo", + }, + { + name: "standard-command", + template: "", + printer: func(w io.Writer, templ string, data interface{}, fm map[string]interface{}) { + fmt.Fprint(w, "yo") + }, + command: "my-command", + wantTemplate: CommandHelpTemplate, + wantOutput: "yo", + }, + { + name: "custom-template-command", + template: "{{doublecho .Name}}", + printer: func(w io.Writer, templ string, data interface{}, _ map[string]interface{}) { + // Pass a custom function to ensure it gets used + fm := map[string]interface{}{"doublecho": doublecho} + printHelpCustom(w, templ, data, fm) + }, + command: "my-command", + wantTemplate: "{{doublecho .Name}}", + wantOutput: "my-command my-command", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func(old helpPrinterCustom) { + HelpPrinterCustom = old + }(HelpPrinterCustom) + HelpPrinterCustom = func(w io.Writer, templ string, data interface{}, fm map[string]interface{}) { + if fm != nil { + t.Error("unexpected function map passed") + } + + if templ != tt.wantTemplate { + t.Errorf("want template:\n%s\ngot template:\n%s", tt.wantTemplate, templ) + } + + tt.printer(w, templ, data, fm) + } + + var buf bytes.Buffer + app := &App{ + Name: "my-app", + Writer: &buf, + Commands: []Command{ + { + Name: "my-command", + CustomHelpTemplate: tt.template, + }, + }, + } + + err := app.Run([]string{"my-app", "help", tt.command}) + if err != nil { + t.Fatal(err) + } + + got := buf.String() + if got != tt.wantOutput { + t.Errorf("want output %q, got %q", tt.wantOutput, got) + } + }) + } +} + func TestShowCommandHelp_CommandAliases(t *testing.T) { app := &App{ Commands: []Command{ From 9ab178d17445f6737817c5265a72a630391fab68 Mon Sep 17 00:00:00 2001 From: Robert Liebowitz Date: Thu, 17 Oct 2019 05:50:38 -0400 Subject: [PATCH 33/33] Make app help behavior consistent with commands --- help.go | 37 +++++++++----- help_test.go | 139 +++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 13 deletions(-) diff --git a/help.go b/help.go index d5f1c87..2280e33 100644 --- a/help.go +++ b/help.go @@ -49,13 +49,16 @@ type helpPrinterCustom func(w io.Writer, templ string, data interface{}, customF // HelpPrinter is a function that writes the help output. If not set explicitly, // this calls HelpPrinterCustom using only the default template functions. +// +// If custom logic for printing help is required, this function can be +// overridden. If the ExtraInfo field is defined on an App, this function +// should not be modified, as HelpPrinterCustom will be used directly in order +// to capture the extra information. var HelpPrinter helpPrinter = printHelp -// HelpPrinterCustom is a function that writes the help output. If not set -// explicitly, a default is used. -// -// The customFuncs map will be combined with a default template.FuncMap to -// allow using arbitrary functions in template rendering. +// HelpPrinterCustom is a function that writes the help output. It is used as +// the default implementation of HelpPrinter, and may be called directly if +// the ExtraInfo field is set on an App. var HelpPrinterCustom helpPrinterCustom = printHelpCustom // VersionPrinter prints the version for the App @@ -68,20 +71,24 @@ func ShowAppHelpAndExit(c *Context, exitCode int) { } // ShowAppHelp is an action that displays the help. -func ShowAppHelp(c *Context) (err error) { - if c.App.CustomAppHelpTemplate == "" { - HelpPrinter(c.App.Writer, AppHelpTemplate, c.App) - return +func ShowAppHelp(c *Context) error { + template := c.App.CustomAppHelpTemplate + if template == "" { + template = AppHelpTemplate + } + + if c.App.ExtraInfo == nil { + HelpPrinter(c.App.Writer, template, c.App) + return nil } + customAppData := func() map[string]interface{} { - if c.App.ExtraInfo == nil { - return nil - } return map[string]interface{}{ "ExtraInfo": c.App.ExtraInfo, } } - HelpPrinterCustom(c.App.Writer, c.App.CustomAppHelpTemplate, c.App, customAppData()) + HelpPrinterCustom(c.App.Writer, template, c.App, customAppData()) + return nil } @@ -242,6 +249,10 @@ func ShowCommandCompletions(ctx *Context, command string) { } +// printHelpCustom is the default implementation of HelpPrinterCustom. +// +// The customFuncs map will be combined with a default template.FuncMap to +// allow using arbitrary functions in template rendering. func printHelpCustom(out io.Writer, templ string, data interface{}, customFuncs map[string]interface{}) { funcMap := template.FuncMap{ "join": strings.Join, diff --git a/help_test.go b/help_test.go index 870b32e..e903494 100644 --- a/help_test.go +++ b/help_test.go @@ -295,6 +295,7 @@ func TestShowCommandHelp_HelpPrinter(t *testing.T) { }) } } + func TestShowCommandHelp_HelpPrinterCustom(t *testing.T) { doublecho := func(text string) string { return text + " " + text @@ -550,6 +551,144 @@ func TestShowAppHelp_HiddenCommand(t *testing.T) { } } +func TestShowAppHelp_HelpPrinter(t *testing.T) { + doublecho := func(text string) string { + return text + " " + text + } + + tests := []struct { + name string + template string + printer helpPrinter + wantTemplate string + wantOutput string + }{ + { + name: "standard-command", + template: "", + printer: func(w io.Writer, templ string, data interface{}) { + fmt.Fprint(w, "yo") + }, + wantTemplate: AppHelpTemplate, + wantOutput: "yo", + }, + { + name: "custom-template-command", + template: "{{doublecho .Name}}", + printer: func(w io.Writer, templ string, data interface{}) { + // Pass a custom function to ensure it gets used + fm := map[string]interface{}{"doublecho": doublecho} + printHelpCustom(w, templ, data, fm) + }, + wantTemplate: "{{doublecho .Name}}", + wantOutput: "my-app my-app", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func(old helpPrinter) { + HelpPrinter = old + }(HelpPrinter) + HelpPrinter = func(w io.Writer, templ string, data interface{}) { + if templ != tt.wantTemplate { + t.Errorf("want template:\n%s\ngot template:\n%s", tt.wantTemplate, templ) + } + + tt.printer(w, templ, data) + } + + var buf bytes.Buffer + app := &App{ + Name: "my-app", + Writer: &buf, + CustomAppHelpTemplate: tt.template, + } + + err := app.Run([]string{"my-app", "help"}) + if err != nil { + t.Fatal(err) + } + + got := buf.String() + if got != tt.wantOutput { + t.Errorf("want output %q, got %q", tt.wantOutput, got) + } + }) + } +} + +func TestShowAppHelp_HelpPrinterCustom(t *testing.T) { + doublecho := func(text string) string { + return text + " " + text + } + + tests := []struct { + name string + template string + printer helpPrinterCustom + wantTemplate string + wantOutput string + }{ + { + name: "standard-command", + template: "", + printer: func(w io.Writer, templ string, data interface{}, fm map[string]interface{}) { + fmt.Fprint(w, "yo") + }, + wantTemplate: AppHelpTemplate, + wantOutput: "yo", + }, + { + name: "custom-template-command", + template: "{{doublecho .Name}}", + printer: func(w io.Writer, templ string, data interface{}, _ map[string]interface{}) { + // Pass a custom function to ensure it gets used + fm := map[string]interface{}{"doublecho": doublecho} + printHelpCustom(w, templ, data, fm) + }, + wantTemplate: "{{doublecho .Name}}", + wantOutput: "my-app my-app", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + defer func(old helpPrinterCustom) { + HelpPrinterCustom = old + }(HelpPrinterCustom) + HelpPrinterCustom = func(w io.Writer, templ string, data interface{}, fm map[string]interface{}) { + if fm != nil { + t.Error("unexpected function map passed") + } + + if templ != tt.wantTemplate { + t.Errorf("want template:\n%s\ngot template:\n%s", tt.wantTemplate, templ) + } + + tt.printer(w, templ, data, fm) + } + + var buf bytes.Buffer + app := &App{ + Name: "my-app", + Writer: &buf, + CustomAppHelpTemplate: tt.template, + } + + err := app.Run([]string{"my-app", "help"}) + if err != nil { + t.Fatal(err) + } + + got := buf.String() + if got != tt.wantOutput { + t.Errorf("want output %q, got %q", tt.wantOutput, got) + } + }) + } +} + func TestShowAppHelp_CustomAppTemplate(t *testing.T) { app := &App{ Commands: []Command{