fix/help: Generate help command list from registered commands

CHANGELOG.md

@@ -17,6 +17,7 @@ All notable changes to `src-cli` are documented in this file.
 
 - HTTP requests now fail instead of hanging forever if the server does not start responding within 1 minute. Set the `SRC_RESPONSE_HEADER_TIMEOUT` environment variable to change this timeout, or to `0` to disable it. Responses that stream data for a long time (for example, large search job results) are not affected.
 - `src search-jobs logs` and `src search-jobs results` now use the standard API client, gaining proxy support, `-insecure-skip-verify`, and cross-host redirect protection, and now report an error on non-200 responses instead of writing the error page into the output.
+- The command list in `src help` is now generated from the registered commands instead of being maintained by hand. `src debug`, `src snapshot`, and `src lsp` now appear in it; aliases are shown after each description.
 
 ### Fixed
 

cmd/src/batch.go

@@ -37,7 +37,8 @@ Use "src batch [command] -h" for more information about a command.
 
 	// Register the command.
 	commands = append(commands, &command{
-		flagSet: flagSet,
+		flagSet:     flagSet,
+		description: "manages batch changes",
 		aliases: []string{
 			"batchchange",
 			"batch-change",

cmd/src/cmd.go

@@ -22,6 +22,14 @@ type command struct {
 	// flagSet.Usage function to invoke on e.g. -h flag. If nil, a default one is
 	// used.
 	usageFunc func()
+
+	// description is the one-line summary shown next to the command in
+	// 'src help'. Required for top-level commands unless hidden is set.
+	description string
+
+	// hidden excludes the command from 'src help' and from the reference
+	// documentation generated by 'src doc'. It can still be run.
+	hidden bool
 }
 
 // matches tells if the given name matches this command or one of its aliases.

cmd/src/code_intel.go

@@ -28,9 +28,10 @@ Use "src code-intel [command] -h" for more information about a command.
 
 	// Register the command.
 	commands = append(commands, &command{
-		flagSet: flagSet,
-		aliases: []string{"code-intel"},
-		handler: handler,
+		flagSet:     flagSet,
+		description: "manages code intelligence data",
+		aliases:     []string{"code-intel"},
+		handler:     handler,
 		usageFunc: func() {
 			fmt.Println(usage)
 		},

cmd/src/config.go

@@ -45,8 +45,9 @@ Use "src config [command] -h" for more information about a command.
 
 	// Register the command.
 	commands = append(commands, &command{
-		flagSet: flagSet,
-		handler: handler,
+		flagSet:     flagSet,
+		description: "manages global, org, and user settings",
+		handler:     handler,
 		usageFunc: func() {
 			fmt.Println(usage)
 		},

cmd/src/debug.go

@@ -34,9 +34,9 @@ src debug has access to flags on src -- Ex: src -v kube -o foo.zip
 
 	// Register the command.
 	commands = append(commands, &command{
-		flagSet:   flagSet,
-		aliases:   []string{},
-		handler:   handler,
-		usageFunc: func() { fmt.Println(usage) },
+		flagSet:     flagSet,
+		description: "gathers and bundles debug data from a Sourcegraph deployment for troubleshooting",
+		handler:     handler,
+		usageFunc:   func() { fmt.Println(usage) },
 	})
 }

cmd/src/doc.go

@@ -82,7 +82,7 @@ Examples:
 					name,
 				}, " "))
 
-				if fqcn == "doc" || fqcn == "publish" {
+				if cmd.hidden {
 					continue
 				}
 
@@ -176,6 +176,7 @@ Examples:
 
 	commands = append(commands, &command{
 		flagSet: flagSet,
+		hidden:  true,
 		handler: handler,
 		usageFunc: func() {
 			fmt.Fprintln(flag.CommandLine.Output(), usage)

cmd/src/doc_test.go

@@ -187,28 +187,34 @@ func TestDocLegacyGroupsHaveSubcommandPages(t *testing.T) {
 	}
 }
 
-// The root index must link every top-level command, both legacy (commander)
-// and migrated (urfave/cli) ones.
-func TestDocRootIndexListsAllCommands(t *testing.T) {
+// The root index.md written by 'src doc' must link exactly the commands that
+// 'src help' lists, which in turn must be exactly the registered commands.
+// A command that is registered but missing from either is a bug.
+func TestDocRootIndexMatchesHelp(t *testing.T) {
 	dir, _ := runDocCommand(t)
 
 	index, err := os.ReadFile(filepath.Join(dir, "index.md"))
 	if err != nil {
 		t.Fatal(err)
 	}
 
-	var missing []string
-	for _, cmd := range commands {
-		name := cmd.flagSet.Name()
-		if name == "doc" || name == "publish" {
+	var indexed []string
+	for _, line := range strings.Split(string(index), "\n") {
+		// Lines look like: * [`name`](name.md) or * [`name`](name/index.md)
+		rest, ok := strings.CutPrefix(strings.TrimSpace(line), "* [`")
+		if !ok {
 			continue
 		}
-		if !strings.Contains(string(index), "[`"+name+"`](") {
-			missing = append(missing, name)
-		}
+		name, _, _ := strings.Cut(rest, "`")
+		indexed = append(indexed, name)
+	}
+	sort.Strings(indexed)
+
+	registered := registeredRootCommandNames()
+	if diff := cmp.Diff(registered, indexed); diff != "" {
+		t.Errorf("'src doc' root index does not match the registered commands (-registered +index):\n%s", diff)
 	}
-	if len(missing) > 0 {
-		sort.Strings(missing)
-		t.Errorf("root index.md is missing legacy commands: %v", missing)
+	if diff := cmp.Diff(helpCommandNames(t, usageText()), indexed); diff != "" {
+		t.Errorf("'src doc' root index does not match 'src help' (-help +index):\n%s", diff)
 	}
 }

cmd/src/extsvc.go

@@ -36,9 +36,10 @@ Use "src extsvc [command] -h" for more information about a command.
 
 	// Register the command.
 	commands = append(commands, &command{
-		flagSet: flagSet,
-		aliases: []string{"extsvc", "external-service"},
-		handler: handler,
+		flagSet:     flagSet,
+		description: "manages external services",
+		aliases:     []string{"extsvc", "external-service"},
+		handler:     handler,
 		usageFunc: func() {
 			fmt.Println(usage)
 		},

cmd/src/help.go

@@ -0,0 +1,121 @@
+package main
+
+import (
+	"cmp"
+	"fmt"
+	"slices"
+	"strings"
+
+	"github.com/sourcegraph/sourcegraph/lib/docgen"
+)
+
+// rootCommand is a top-level 'src' command as shown in 'src help'. It is the
+// single source for the command list in the help text and for the tests that
+// keep 'src help' and the 'src doc' root index in sync.
+type rootCommand struct {
+	name        string
+	aliases     []string
+	description string
+}
+
+// rootCommands returns every visible top-level command, whether it is
+// registered with the legacy commander (commands) or with urfave/cli
+// (migratedCommands), sorted by name.
+func rootCommands() []rootCommand {
+	var root []rootCommand
+
+	for _, cmd := range commands {
+		if cmd.hidden {
+			continue
+		}
+		name := cmd.flagSet.Name()
+		var aliases []string
+		for _, alias := range cmd.aliases {
+			// Some legacy commands register their own name as an alias.
+			if alias != name {
+				aliases = append(aliases, alias)
+			}
+		}
+		root = append(root, rootCommand{
+			name:        name,
+			aliases:     aliases,
+			description: cmd.description,
+		})
+	}
+
+	for _, cmd := range docgen.VisibleCommands(migratedRootCommand().Commands) {
+		root = append(root, rootCommand{
+			name:        cmd.Name,
+			aliases:     slices.Clone(cmd.Aliases),
+			description: cmd.Usage,
+		})
+	}
+
+	slices.SortFunc(root, func(a, b rootCommand) int {
+		return cmp.Compare(a.name, b.name)
+	})
+	return root
+}
+
+// formatCommandList renders the "The commands are:" block of 'src help':
+// one tab-indented line per command with the name padded to a common width,
+// the description, and any aliases in parentheses.
+func formatCommandList(cmds []rootCommand) string {
+	width := 0
+	for _, cmd := range cmds {
+		width = max(width, len(cmd.name))
+	}
+
+	var b strings.Builder
+	for _, cmd := range cmds {
+		fmt.Fprintf(&b, "\t%-*s  %s", width, cmd.name, cmd.description)
+		if len(cmd.aliases) > 0 {
+			fmt.Fprintf(&b, " (alias: %s)", strings.Join(cmd.aliases, ", "))
+		}
+		b.WriteString("\n")
+	}
+	return b.String()
+}
+
+// usageText renders the top-level 'src help' output.
+func usageText() string {
+	return usageHeader + formatCommandList(rootCommands()) + usageFooter
+}
+
+const usageHeader = `src is a tool that provides access to Sourcegraph instances.
+For more information, see https://github.com/sourcegraph/src-cli
+
+Usage:
+
+	src [options] command [command options]
+
+Environment variables
+	SRC_ACCESS_TOKEN  Sourcegraph access token
+	SRC_ENDPOINT      endpoint to use, if unset will default to "https://sourcegraph.com"
+	SRC_PROXY         A proxy to use for proxying requests to the Sourcegraph endpoint.
+	                  Supports HTTP(S), SOCKS5/5h, and UNIX Domain Socket proxies.
+					  If a UNIX Domain Socket, the path can be either an absolute path,
+					  or can start with ~/ or %USERPROFILE%\ for a path in the user's home directory.
+					  Examples:
+						- https://localhost:3080
+						- https://<user>:<password>localhost:8080
+						- socks5h://localhost:1080
+						- socks5://<username>:<password>@localhost:1080
+						- unix://~/src-proxy.sock
+						- unix://%USERPROFILE%\src-proxy.sock
+						- ~/src-proxy.sock
+						- %USERPROFILE%\src-proxy.sock
+						- C:\some\path\src-proxy.sock
+
+The options are:
+
+	-v                               print verbose output
+
+The commands are:
+
+`
+
+const usageFooter = `
+Use "src [command] -h" for more information about a command.
+
+`

cmd/src/help_test.go

@@ -0,0 +1,113 @@
+package main
+
+import (
+	"sort"
+	"strings"
+	"testing"
+
+	"github.com/google/go-cmp/cmp"
+)
+
+// registeredRootCommandNames computes the set of visible top-level command
+// names straight from the two registries, independently of rootCommands(), so
+// the tests below catch a command that is registered but left out of the help
+// text or the docs.
+func registeredRootCommandNames() []string {
+	seen := map[string]bool{}
+	for _, cmd := range commands {
+		if !cmd.hidden {
+			seen[cmd.flagSet.Name()] = true
+		}
+	}
+	for _, cmd := range migratedCommands {
+		if !cmd.Hidden {
+			seen[cmd.Name] = true
+		}
+	}
+	names := make([]string, 0, len(seen))
+	for name := range seen {
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	return names
+}
+
+// helpCommandNames parses the "The commands are:" block of the given 'src
+// help' output and returns the command names (without aliases), sorted.
+func helpCommandNames(t *testing.T, help string) []string {
+	t.Helper()
+
+	_, block, ok := strings.Cut(help, "The commands are:\n")
+	if !ok {
+		t.Fatalf("help text has no \"The commands are:\" block:\n%s", help)
+	}
+	block, _, _ = strings.Cut(block, "\nUse \"src [command] -h\"")
+
+	var names []string
+	for _, line := range strings.Split(block, "\n") {
+		line = strings.TrimSpace(line)
+		if line == "" {
+			continue
+		}
+		name, _, _ := strings.Cut(line, " ")
+		names = append(names, name)
+	}
+	sort.Strings(names)
+	return names
+}
+
+func TestHelpListsAllRegisteredCommands(t *testing.T) {
+	got := helpCommandNames(t, usageText())
+	if diff := cmp.Diff(registeredRootCommandNames(), got); diff != "" {
+		t.Errorf("'src help' command list does not match the registered commands (-registered +help):\n%s", diff)
+	}
+}
+
+func TestHelpHidesHiddenCommands(t *testing.T) {
+	help := usageText()
+	for _, cmd := range commands {
+		if cmd.hidden && strings.Contains(help, "\t"+cmd.flagSet.Name()+" ") {
+			t.Errorf("hidden command %q is listed in 'src help'", cmd.flagSet.Name())
+		}
+	}
+	if !strings.Contains(help, "\tabc ") || !strings.Contains(help, "\tbatch") {
+		t.Errorf("expected both a urfave/cli command (abc) and a legacy command (batch) in help:\n%s", help)
+	}
+}
+
+func TestRootCommandsAreWellFormed(t *testing.T) {
+	names := map[string]bool{}
+	for _, cmd := range rootCommands() {
+		if cmd.description == "" {
+			t.Errorf("command %q has no description: set description on the legacy command or Usage on the urfave/cli command", cmd.name)
+		}
+		if names[cmd.name] {
+			t.Errorf("command %q is registered more than once", cmd.name)
+		}
+		names[cmd.name] = true
+		for _, alias := range cmd.aliases {
+			if alias == cmd.name {
+				t.Errorf("command %q lists its own name as an alias", cmd.name)
+			}
+		}
+	}
+	for _, cmd := range rootCommands() {
+		for _, alias := range cmd.aliases {
+			if names[alias] {
+				t.Errorf("alias %q of command %q collides with another command's name", alias, cmd.name)
+			}
+		}
+	}
+}
+
+func TestFormatCommandList(t *testing.T) {
+	got := formatCommandList([]rootCommand{
+		{name: "a", description: "first"},
+		{name: "longer", aliases: []string{"l", "lg"}, description: "second"},
+	})
+	want := "\ta       first\n" +
+		"\tlonger  second (alias: l, lg)\n"
+	if diff := cmp.Diff(want, got); diff != "" {
+		t.Errorf("formatCommandList mismatch (-want +got):\n%s", diff)
+	}
+}