From 07ed1e4e8a2adc4ee46b0108cd2d464498689842 Mon Sep 17 00:00:00 2001 From: William Martin Date: Thu, 11 May 2023 14:59:12 +0200 Subject: [PATCH] Introduce helpTopics type and reduce duplication across commands (#7414) --- pkg/cmd/root/help.go | 4 +- pkg/cmd/root/help_reference.go | 6 +- pkg/cmd/root/help_topic.go | 65 ++++++++++++-------- pkg/cmd/root/help_topic_test.go | 103 +++++++++++++++++--------------- pkg/cmd/root/root.go | 26 +++++--- 5 files changed, 118 insertions(+), 86 deletions(-) diff --git a/pkg/cmd/root/help.go b/pkg/cmd/root/help.go index e35486f48..9da57d6e9 100644 --- a/pkg/cmd/root/help.go +++ b/pkg/cmd/root/help.go @@ -139,8 +139,8 @@ func rootHelpFunc(f *cmdutil.Factory, command *cobra.Command, args []string) { if c := findCommand(command, "actions"); c != nil { helpTopics = append(helpTopics, rpad(c.Name()+":", namePadding)+c.Short) } - for topic, params := range HelpTopics { - helpTopics = append(helpTopics, rpad(topic+":", namePadding)+params["short"]) + for _, helpTopic := range HelpTopics { + helpTopics = append(helpTopics, rpad(helpTopic.name+":", namePadding)+helpTopic.short) } sort.Strings(helpTopics) helpEntries = append(helpEntries, helpEntry{"HELP TOPICS", strings.Join(helpTopics, "\n")}) diff --git a/pkg/cmd/root/help_reference.go b/pkg/cmd/root/help_reference.go index 86132c985..76e9db24e 100644 --- a/pkg/cmd/root/help_reference.go +++ b/pkg/cmd/root/help_reference.go @@ -11,7 +11,9 @@ import ( "github.com/spf13/cobra" ) -func referenceHelpFn(io *iostreams.IOStreams) func(*cobra.Command, []string) { +// longPager provides a pager over a commands Long message. +// It is currently only used for the reference command +func longPager(io *iostreams.IOStreams) func(*cobra.Command, []string) { return func(cmd *cobra.Command, args []string) { wrapWidth := 0 if io.IsStdoutTTY() { @@ -38,7 +40,7 @@ func referenceHelpFn(io *iostreams.IOStreams) func(*cobra.Command, []string) { } } -func referenceLong(cmd *cobra.Command) string { +func stringifyReference(cmd *cobra.Command) string { buf := bytes.NewBufferString("# gh reference\n\n") for _, c := range cmd.Commands() { if c.Hidden { diff --git a/pkg/cmd/root/help_topic.go b/pkg/cmd/root/help_topic.go index 4df7d9e1a..18a7f4801 100644 --- a/pkg/cmd/root/help_topic.go +++ b/pkg/cmd/root/help_topic.go @@ -10,10 +10,18 @@ import ( "github.com/spf13/cobra" ) -var HelpTopics = map[string]map[string]string{ - "mintty": { - "short": "Information about using gh with MinTTY", - "long": heredoc.Doc(` +type helpTopic struct { + name string + short string + long string + example string +} + +var HelpTopics = []helpTopic{ + { + name: "mintty", + short: "Information about using gh with MinTTY", + long: heredoc.Doc(` MinTTY is the terminal emulator that comes by default with Git for Windows. It has known issues with gh's ability to prompt a user for input. @@ -30,9 +38,10 @@ var HelpTopics = map[string]map[string]string{ NOTE: this can lead to some UI bugs. `), }, - "environment": { - "short": "Environment variables that can be used with gh", - "long": heredoc.Doc(` + { + name: "environment", + short: "Environment variables that can be used with gh", + long: heredoc.Doc(` GH_TOKEN, GITHUB_TOKEN (in order of precedence): an authentication token for github.com API requests. Setting this avoids being prompted to authenticate and takes precedence over previously stored credentials. @@ -91,12 +100,14 @@ var HelpTopics = map[string]map[string]string{ its own path such as in the cygwin terminal. `), }, - "reference": { - "short": "A comprehensive reference of all gh commands", + { + name: "reference", + short: "A comprehensive reference of all gh commands", }, - "formatting": { - "short": "Formatting options for JSON data exported from gh", - "long": heredoc.Docf(` + { + name: "formatting", + short: "Formatting options for JSON data exported from gh", + long: heredoc.Docf(` By default, the result of %[1]sgh%[1]s commands are output in line-based plain text format. Some commands support passing the %[1]s--json%[1]s flag, which converts the output to JSON format. Once in JSON, the output can be further formatted according to a required formatting string by @@ -132,7 +143,7 @@ var HelpTopics = map[string]map[string]string{ To learn more about Go templates, see: . `, "`"), - "example": heredoc.Doc(` + example: heredoc.Doc(` # default output format $ gh pr list Showing 23 of 23 open pull requests in cli/cli @@ -242,9 +253,10 @@ var HelpTopics = map[string]map[string]string{ mislav COMMENTED This is going along great! Thanks for working on this ❤️ `), }, - "exit-codes": { - "short": "Exit codes used by gh", - "long": heredoc.Doc(` + { + name: "exit-codes", + short: "Exit codes used by gh", + long: heredoc.Doc(` gh follows normal conventions regarding exit codes. - If a command completes successfully, the exit code will be 0 @@ -262,30 +274,31 @@ var HelpTopics = map[string]map[string]string{ }, } -func NewHelpTopic(ios *iostreams.IOStreams, topic string) *cobra.Command { +func NewCmdHelpTopic(ios *iostreams.IOStreams, ht helpTopic) *cobra.Command { cmd := &cobra.Command{ - Use: topic, - Short: HelpTopics[topic]["short"], - Long: HelpTopics[topic]["long"], - Example: HelpTopics[topic]["example"], + Use: ht.name, + Short: ht.short, + Long: ht.long, + Example: ht.example, Hidden: true, Annotations: map[string]string{ "markdown:generate": "true", - "markdown:basename": "gh_help_" + topic, + "markdown:basename": "gh_help_" + ht.name, }, } - cmd.SetHelpFunc(func(c *cobra.Command, args []string) { - helpTopicHelpFunc(ios.Out, c, args) - }) cmd.SetUsageFunc(func(c *cobra.Command) error { return helpTopicUsageFunc(ios.ErrOut, c) }) + cmd.SetHelpFunc(func(c *cobra.Command, _ []string) { + helpTopicHelpFunc(ios.Out, c) + }) + return cmd } -func helpTopicHelpFunc(w io.Writer, command *cobra.Command, args []string) { +func helpTopicHelpFunc(w io.Writer, command *cobra.Command) { fmt.Fprint(w, command.Long) if command.Example != "" { fmt.Fprintf(w, "\n\nEXAMPLES\n") diff --git a/pkg/cmd/root/help_topic_test.go b/pkg/cmd/root/help_topic_test.go index 5f6825912..baa4f7f56 100644 --- a/pkg/cmd/root/help_topic_test.go +++ b/pkg/cmd/root/help_topic_test.go @@ -4,76 +4,85 @@ import ( "testing" "github.com/cli/cli/v2/pkg/iostreams" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -func TestNewHelpTopic(t *testing.T) { +func TestCmdHelpTopic(t *testing.T) { + t.Parallel() + tests := []struct { - name string - topic string - args []string - flags []string - wantsErr bool + name string + topic helpTopic + args []string + flags []string + errorAssertion require.ErrorAssertionFunc + expectedAnnotations map[string]string }{ { - name: "valid topic", - topic: "environment", - args: []string{}, - flags: []string{}, - wantsErr: false, + name: "when there are no args or flags, prints the long message to stdout", + topic: helpTopic{name: "test-topic", long: "test-topic-long"}, + args: []string{}, + flags: []string{}, + errorAssertion: require.NoError, }, { - name: "invalid topic", - topic: "invalid", - args: []string{}, - flags: []string{}, - wantsErr: false, + name: "when an arg is provided, it is ignored and help is printed", + topic: helpTopic{name: "test-topic"}, + args: []string{"anything"}, + flags: []string{}, + errorAssertion: require.NoError, }, { - name: "more than zero args", - topic: "environment", - args: []string{"invalid"}, - flags: []string{}, - wantsErr: false, + name: "when a flag is provided, returns an error", + topic: helpTopic{name: "test-topic"}, + args: []string{}, + flags: []string{"--anything"}, + errorAssertion: require.Error, }, { - name: "more than zero flags", - topic: "environment", - args: []string{}, - flags: []string{"--invalid"}, - wantsErr: true, + name: "when there is an example, include it in the stdout", + topic: helpTopic{name: "test-topic", example: "test-topic-example"}, + args: []string{}, + flags: []string{}, + errorAssertion: require.NoError, }, { - name: "help arg", - topic: "environment", - args: []string{"help"}, - flags: []string{}, - wantsErr: false, - }, - { - name: "help flag", - topic: "environment", - args: []string{}, - flags: []string{"--help"}, - wantsErr: false, + name: "sets important markdown annotations on the command", + topic: helpTopic{name: "test-topic"}, + args: []string{}, + flags: []string{}, + errorAssertion: require.NoError, + expectedAnnotations: map[string]string{ + "markdown:generate": "true", + "markdown:basename": "gh_help_test-topic", + }, }, } for _, tt := range tests { + tt := tt t.Run(tt.name, func(t *testing.T) { - ios, _, _, stderr := iostreams.Test() + t.Parallel() - cmd := NewHelpTopic(ios, tt.topic) + ios, _, stdout, _ := iostreams.Test() + + cmd := NewCmdHelpTopic(ios, tt.topic) cmd.SetArgs(append(tt.args, tt.flags...)) - cmd.SetOut(stderr) - cmd.SetErr(stderr) _, err := cmd.ExecuteC() - if tt.wantsErr { - assert.Error(t, err) - return + tt.errorAssertion(t, err) + + if tt.topic.long != "" { + require.Contains(t, stdout.String(), tt.topic.long) + } + + if tt.topic.example != "" { + require.Contains(t, stdout.String(), tt.topic.example) + } + + if len(tt.expectedAnnotations) > 0 { + require.Equal(t, cmd.Annotations, tt.expectedAnnotations) } - assert.NoError(t, err) }) } } diff --git a/pkg/cmd/root/root.go b/pkg/cmd/root/root.go index 78b436fe1..8d18a4db1 100644 --- a/pkg/cmd/root/root.go +++ b/pkg/cmd/root/root.go @@ -125,18 +125,26 @@ func NewCmdRoot(f *cmdutil.Factory, version, buildDate string) *cobra.Command { cmd.AddCommand(labelCmd.NewCmdLabel(&repoResolvingCmdFactory)) // Help topics - cmd.AddCommand(NewHelpTopic(f.IOStreams, "environment")) - cmd.AddCommand(NewHelpTopic(f.IOStreams, "formatting")) - cmd.AddCommand(NewHelpTopic(f.IOStreams, "mintty")) - cmd.AddCommand(NewHelpTopic(f.IOStreams, "exit-codes")) - referenceCmd := NewHelpTopic(f.IOStreams, "reference") - referenceCmd.SetHelpFunc(referenceHelpFn(f.IOStreams)) - cmd.AddCommand(referenceCmd) + var referenceCmd *cobra.Command + for _, ht := range HelpTopics { + helpTopicCmd := NewCmdHelpTopic(f.IOStreams, ht) + cmd.AddCommand(helpTopicCmd) + + // See bottom of the function for why we explicitly care about the reference cmd + if ht.name == "reference" { + referenceCmd = helpTopicCmd + } + } cmdutil.DisableAuthCheck(cmd) - // this needs to appear last: - referenceCmd.Long = referenceLong(cmd) + // The reference command produces paged output that displays information on every other command. + // Therefore, we explicitly set the Long text and HelpFunc here after all other commands are registered. + // We experimented with producing the paged output dynamically when the HelpFunc is called but since + // doc generation makes use of the Long text, it is simpler to just be explicit here that this command + // is special. + referenceCmd.Long = stringifyReference(cmd) + referenceCmd.SetHelpFunc(longPager(f.IOStreams)) return cmd }