From ef7074a8d16536ce37e3ccb04cc2f49f28d84206 Mon Sep 17 00:00:00 2001 From: Hennik Hunsaker Date: Fri, 24 Mar 2023 18:42:34 -0600 Subject: [PATCH 1/7] v2 - Fix `incorrectTypeForFlagError` for unknowns The `reflect` package doesn't always know what to call a type in `(reflect.Type).Name()`, but `fmt.Errorf("%T")` always gets it right. Use that so we get actionable error messages. --- altsrc/map_input_source.go | 9 +-------- altsrc/map_input_source_test.go | 6 ++++++ 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/altsrc/map_input_source.go b/altsrc/map_input_source.go index abd989f0d4..1a76051e84 100644 --- a/altsrc/map_input_source.go +++ b/altsrc/map_input_source.go @@ -3,7 +3,6 @@ package altsrc import ( "fmt" "math" - "reflect" "strings" "time" @@ -471,11 +470,5 @@ func (fsm *MapInputSource) isSet(name string) bool { } func incorrectTypeForFlagError(name, expectedTypeName string, value interface{}) error { - valueType := reflect.TypeOf(value) - valueTypeName := "" - if valueType != nil { - valueTypeName = valueType.Name() - } - - return fmt.Errorf("Mismatched type for flag '%s'. Expected '%s' but actual is '%s'", name, expectedTypeName, valueTypeName) + return fmt.Errorf("Mismatched type for flag '%s'. Expected '%s' but actual is '%T'", name, expectedTypeName, value) } diff --git a/altsrc/map_input_source_test.go b/altsrc/map_input_source_test.go index cf399b5354..4d3514e962 100644 --- a/altsrc/map_input_source_test.go +++ b/altsrc/map_input_source_test.go @@ -1,6 +1,7 @@ package altsrc import ( + "fmt" "testing" "time" ) @@ -33,3 +34,8 @@ func TestMapInputSource_Int64Slice(t *testing.T) { expect(t, []int64{1, 2, 3}, d) expect(t, nil, err) } + +func TestMapInputSource_IncorrectFlagTypeError(t *testing.T) { + var testVal *bool + expect(t, incorrectTypeForFlagError("test", "bool", testVal), fmt.Errorf("Mismatched type for flag 'test'. Expected 'bool' but actual is '*bool'")) +} From 57cbb2dd6606dad21991b1315900e4e281f40874 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 17 Apr 2023 10:01:01 -0400 Subject: [PATCH 2/7] Fix:(issue_1689) Let markdown output behave similar to cli help output --- docs.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/docs.go b/docs.go index 8b1c9c8a2c..6cd0624aea 100644 --- a/docs.go +++ b/docs.go @@ -153,9 +153,14 @@ func prepareFlags( // flagDetails returns a string containing the flags metadata func flagDetails(flag DocGenerationFlag) string { description := flag.GetUsage() - value := flag.GetValue() - if value != "" { - description += " (default: " + value + ")" + if flag.TakesValue() { + defaultText := flag.GetDefaultText() + if defaultText == "" { + defaultText = flag.GetValue() + } + if defaultText != "" { + description += " (default: " + defaultText + ")" + } } return ": " + description } From a78aa8cfa680c576c326c17c28b15dd9636b8df3 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Mon, 17 Apr 2023 10:06:31 -0400 Subject: [PATCH 3/7] Update tests --- fish_test.go | 11 ++++++----- testdata/expected-doc-full.man | 2 +- testdata/expected-doc-full.md | 2 +- testdata/expected-doc-no-authors.md | 2 +- testdata/expected-doc-no-commands.md | 2 +- testdata/expected-doc-no-usagetext.md | 2 +- 6 files changed, 11 insertions(+), 10 deletions(-) diff --git a/fish_test.go b/fish_test.go index d28b0d4202..da68164174 100644 --- a/fish_test.go +++ b/fish_test.go @@ -27,11 +27,12 @@ func testApp() *App { app.Name = "greet" app.Flags = []Flag{ &StringFlag{ - Name: "socket", - Aliases: []string{"s"}, - Usage: "some 'usage' text", - Value: "value", - TakesFile: true, + Name: "socket", + Aliases: []string{"s"}, + Usage: "some 'usage' text", + Value: "value", + DefaultText: "svalue", + TakesFile: true, }, &StringFlag{Name: "flag", Aliases: []string{"fl", "f"}}, &BoolFlag{ diff --git a/testdata/expected-doc-full.man b/testdata/expected-doc-full.man index a2131f0f55..bfae3a175c 100644 --- a/testdata/expected-doc-full.man +++ b/testdata/expected-doc-full.man @@ -47,7 +47,7 @@ app [first_arg] [second_arg] \fB--flag, --fl, -f\fP="": .PP -\fB--socket, -s\fP="": some 'usage' text (default: value) +\fB--socket, -s\fP="": some 'usage' text (default: svalue) .SH COMMANDS diff --git a/testdata/expected-doc-full.md b/testdata/expected-doc-full.md index 80ff6a7a68..8f79818ba8 100644 --- a/testdata/expected-doc-full.md +++ b/testdata/expected-doc-full.md @@ -28,7 +28,7 @@ app [first_arg] [second_arg] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: value) +**--socket, -s**="": some 'usage' text (default: svalue) # COMMANDS diff --git a/testdata/expected-doc-no-authors.md b/testdata/expected-doc-no-authors.md index 80ff6a7a68..8f79818ba8 100644 --- a/testdata/expected-doc-no-authors.md +++ b/testdata/expected-doc-no-authors.md @@ -28,7 +28,7 @@ app [first_arg] [second_arg] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: value) +**--socket, -s**="": some 'usage' text (default: svalue) # COMMANDS diff --git a/testdata/expected-doc-no-commands.md b/testdata/expected-doc-no-commands.md index 4db2a375ec..3e285f2d42 100644 --- a/testdata/expected-doc-no-commands.md +++ b/testdata/expected-doc-no-commands.md @@ -28,5 +28,5 @@ app [first_arg] [second_arg] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: value) +**--socket, -s**="": some 'usage' text (default: svalue) diff --git a/testdata/expected-doc-no-usagetext.md b/testdata/expected-doc-no-usagetext.md index d09b69fb6b..8354f4c629 100644 --- a/testdata/expected-doc-no-usagetext.md +++ b/testdata/expected-doc-no-usagetext.md @@ -28,7 +28,7 @@ greet [GLOBAL OPTIONS] command [COMMAND OPTIONS] [ARGUMENTS...] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: value) +**--socket, -s**="": some 'usage' text (default: svalue) # COMMANDS From d31582cdebb7e91a76223dc8357d8be8277726d0 Mon Sep 17 00:00:00 2001 From: dearchap Date: Wed, 26 Apr 2023 15:16:20 -0400 Subject: [PATCH 4/7] Update fish_test.go Co-authored-by: Anatoli Babenia --- fish_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fish_test.go b/fish_test.go index da68164174..f74b55aceb 100644 --- a/fish_test.go +++ b/fish_test.go @@ -31,7 +31,7 @@ func testApp() *App { Aliases: []string{"s"}, Usage: "some 'usage' text", Value: "value", - DefaultText: "svalue", + DefaultText: "/some/path", TakesFile: true, }, &StringFlag{Name: "flag", Aliases: []string{"fl", "f"}}, From 5b9c03e5cd963a5c4abc6db5286e09569a1db365 Mon Sep 17 00:00:00 2001 From: Naveen Gogineni Date: Wed, 26 Apr 2023 15:20:38 -0400 Subject: [PATCH 5/7] Change from code review --- testdata/expected-doc-full.man | 2 +- testdata/expected-doc-full.md | 2 +- testdata/expected-doc-no-authors.md | 2 +- testdata/expected-doc-no-commands.md | 2 +- testdata/expected-doc-no-usagetext.md | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/testdata/expected-doc-full.man b/testdata/expected-doc-full.man index bfae3a175c..615f8d6af4 100644 --- a/testdata/expected-doc-full.man +++ b/testdata/expected-doc-full.man @@ -47,7 +47,7 @@ app [first_arg] [second_arg] \fB--flag, --fl, -f\fP="": .PP -\fB--socket, -s\fP="": some 'usage' text (default: svalue) +\fB--socket, -s\fP="": some 'usage' text (default: /some/path) .SH COMMANDS diff --git a/testdata/expected-doc-full.md b/testdata/expected-doc-full.md index 8f79818ba8..65c3fda162 100644 --- a/testdata/expected-doc-full.md +++ b/testdata/expected-doc-full.md @@ -28,7 +28,7 @@ app [first_arg] [second_arg] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: svalue) +**--socket, -s**="": some 'usage' text (default: /some/path) # COMMANDS diff --git a/testdata/expected-doc-no-authors.md b/testdata/expected-doc-no-authors.md index 8f79818ba8..65c3fda162 100644 --- a/testdata/expected-doc-no-authors.md +++ b/testdata/expected-doc-no-authors.md @@ -28,7 +28,7 @@ app [first_arg] [second_arg] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: svalue) +**--socket, -s**="": some 'usage' text (default: /some/path) # COMMANDS diff --git a/testdata/expected-doc-no-commands.md b/testdata/expected-doc-no-commands.md index 3e285f2d42..9f918fe14f 100644 --- a/testdata/expected-doc-no-commands.md +++ b/testdata/expected-doc-no-commands.md @@ -28,5 +28,5 @@ app [first_arg] [second_arg] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: svalue) +**--socket, -s**="": some 'usage' text (default: /some/path) diff --git a/testdata/expected-doc-no-usagetext.md b/testdata/expected-doc-no-usagetext.md index 8354f4c629..50a87d5540 100644 --- a/testdata/expected-doc-no-usagetext.md +++ b/testdata/expected-doc-no-usagetext.md @@ -28,7 +28,7 @@ greet [GLOBAL OPTIONS] command [COMMAND OPTIONS] [ARGUMENTS...] **--flag, --fl, -f**="": -**--socket, -s**="": some 'usage' text (default: svalue) +**--socket, -s**="": some 'usage' text (default: /some/path) # COMMANDS From b9cb2a4111985258391ff5d99bac905447200bf8 Mon Sep 17 00:00:00 2001 From: Jonas Tingeborn Date: Sun, 30 Apr 2023 18:24:32 +0200 Subject: [PATCH 6/7] Fix #1703, honor custom help template --- help.go | 24 +++++++++++++++++++++-- help_test.go | 55 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 2 deletions(-) diff --git a/help.go b/help.go index c7b8f55a58..61961c37e9 100644 --- a/help.go +++ b/help.go @@ -42,6 +42,7 @@ var helpCommand = &Command{ // 3 $ app foo // 4 $ app help foo // 5 $ app foo help + // 6 $ app foo -h (with no other sub-commands nor flags defined) // Case 4. when executing a help command set the context to parent // to allow resolution of subsequent args. This will transform @@ -77,10 +78,25 @@ var helpCommand = &Command{ HelpPrinter(cCtx.App.Writer, templ, cCtx.Command) return nil } + + // Case 6, handling incorporated in the callee itself return ShowSubcommandHelp(cCtx) }, } +func anyFlag(candidates []string, flags []Flag) bool { + for _, flag := range flags { + for _, alias := range flag.Names() { + for _, c := range candidates { + if c == alias { + return true + } + } + } + } + return false +} + // Prints help for the App or Command type helpPrinter func(w io.Writer, templ string, data interface{}) @@ -292,8 +308,12 @@ func ShowSubcommandHelp(cCtx *Context) error { if cCtx == nil { return nil } - - HelpPrinter(cCtx.App.Writer, SubcommandHelpTemplate, cCtx.Command) + // use custom template when provided (fixes #1703) + templ := SubcommandHelpTemplate + if cCtx.Command != nil && cCtx.Command.CustomHelpTemplate != "" { + templ = cCtx.Command.CustomHelpTemplate + } + HelpPrinter(cCtx.App.Writer, templ, cCtx.Command) return nil } diff --git a/help_test.go b/help_test.go index c680e3c65f..ca1323afab 100644 --- a/help_test.go +++ b/help_test.go @@ -1126,6 +1126,61 @@ func TestHideHelpCommand_WithSubcommands(t *testing.T) { } } +func TestHideHelpCommand_RunAsSubcommand_False_CustomTemplate(t *testing.T) { + var buf bytes.Buffer + + app := &App{ + Writer: &buf, + Commands: []*Command{ + { + Name: "dummy", + CustomHelpTemplate: "TEMPLATE", + HideHelpCommand: false, + }, + }, + } + + err := app.RunAsSubcommand(newContextFromStringSlice([]string{"", "dummy", "help"})) + if err != nil { + t.Errorf("Run returned unexpected error: %v", err) + } + if !strings.Contains(buf.String(), "TEMPLATE") { + t.Errorf("Custom Help template ignored") + } + buf.Reset() + + err = app.RunAsSubcommand(newContextFromStringSlice([]string{"", "dummy", "-h"})) + if err != nil { + t.Errorf("Run returned unexpected error: %v", err) + } + if !strings.Contains(buf.String(), "TEMPLATE") { + t.Errorf("Custom Help template ignored") + } +} + +func TestHideHelpCommand_RunAsSubcommand_True_CustomTemplate(t *testing.T) { + var buf bytes.Buffer + + app := &App{ + Writer: &buf, + Commands: []*Command{ + { + Name: "dummy", + CustomHelpTemplate: "TEMPLATE", + HideHelpCommand: true, + }, + }, + } + + err := app.RunAsSubcommand(newContextFromStringSlice([]string{"", "dummy", "-h"})) + if err != nil { + t.Errorf("Run returned unexpected error: %v", err) + } + if !strings.Contains(buf.String(), "TEMPLATE") { + t.Errorf("Custom Help template ignored") + } +} + func TestDefaultCompleteWithFlags(t *testing.T) { origEnv := os.Environ() origArgv := os.Args From 3541e283dbf14c86eae88fabbe56567babd4ac8d Mon Sep 17 00:00:00 2001 From: Jonas Tingeborn Date: Mon, 1 May 2023 16:18:33 +0200 Subject: [PATCH 7/7] Address review comments (reduntant code) --- help.go | 13 ------------- help_test.go | 32 -------------------------------- 2 files changed, 45 deletions(-) diff --git a/help.go b/help.go index 61961c37e9..a71fceb709 100644 --- a/help.go +++ b/help.go @@ -84,19 +84,6 @@ var helpCommand = &Command{ }, } -func anyFlag(candidates []string, flags []Flag) bool { - for _, flag := range flags { - for _, alias := range flag.Names() { - for _, c := range candidates { - if c == alias { - return true - } - } - } - } - return false -} - // Prints help for the App or Command type helpPrinter func(w io.Writer, templ string, data interface{}) diff --git a/help_test.go b/help_test.go index ca1323afab..fd9983bb64 100644 --- a/help_test.go +++ b/help_test.go @@ -1126,38 +1126,6 @@ func TestHideHelpCommand_WithSubcommands(t *testing.T) { } } -func TestHideHelpCommand_RunAsSubcommand_False_CustomTemplate(t *testing.T) { - var buf bytes.Buffer - - app := &App{ - Writer: &buf, - Commands: []*Command{ - { - Name: "dummy", - CustomHelpTemplate: "TEMPLATE", - HideHelpCommand: false, - }, - }, - } - - err := app.RunAsSubcommand(newContextFromStringSlice([]string{"", "dummy", "help"})) - if err != nil { - t.Errorf("Run returned unexpected error: %v", err) - } - if !strings.Contains(buf.String(), "TEMPLATE") { - t.Errorf("Custom Help template ignored") - } - buf.Reset() - - err = app.RunAsSubcommand(newContextFromStringSlice([]string{"", "dummy", "-h"})) - if err != nil { - t.Errorf("Run returned unexpected error: %v", err) - } - if !strings.Contains(buf.String(), "TEMPLATE") { - t.Errorf("Custom Help template ignored") - } -} - func TestHideHelpCommand_RunAsSubcommand_True_CustomTemplate(t *testing.T) { var buf bytes.Buffer