Introduce helpTopics type and reduce duplication across commands (#7414)

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")})

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 {

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: <https://golang.org/pkg/text/template/>.
 		`, "`"),
-		"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")

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)
 		})
 	}
 }

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
 }