diff --git a/internal/commandsgen/docs.go b/internal/commandsgen/docs.go index d699c23f6..2cfa68e91 100644 --- a/internal/commandsgen/docs.go +++ b/internal/commandsgen/docs.go @@ -129,22 +129,38 @@ func (w *docWriter) writeSubcommand(c *Command) { w.fileMap[fileName].WriteString(prefix + " " + c.leafName() + "\n\n") w.fileMap[fileName].WriteString(escapeMDXDescription(c.Description) + "\n\n") - if w.isLeafCommand(c) { - w.writeLeafOptions(fileName) + if w.hasOptionsSection(c) { + w.writeOptionsSection(c, fileName) } } -// writeLeafOptions writes the option table (or the global-flags fallback message) -// for a leaf command, and records its inherited options for the file's Global -// Flags section. The command's own options are the top frame of the options -// stack; everything below it is inherited from parent commands. +// hasOptionsSection reports whether a command gets an option table. Leaf +// commands always do. A command with subcommands does only when it declares +// options of its own (e.g. "workflow reset" or "task-queue versioning"). Those +// options are generated as persistent flags, so they apply to the command and +// to every subcommand under it, and are documented nowhere else. Parents that +// only group subcommands, or only pull in option sets, get no table. +func (w *docWriter) hasOptionsSection(c *Command) bool { + return w.isLeafCommand(c) || len(c.Options) > 0 +} + +// writeOptionsSection writes the option table (or the global-flags fallback +// message) for a command that has an options section (see hasOptionsSection), +// and records its inherited options for the file's Global Flags section. The +// command's own options are the top frame of the options stack; everything +// below it is inherited from parent commands. // // The reference to the "#global-flags" anchor is only emitted when there are // inherited options, since that is what causes a Global Flags section (and thus // the anchor) to be generated for the file. A command with no inherited options // (e.g. a top-level command in a split subdirectory) would otherwise link to an // anchor that never exists. -func (w *docWriter) writeLeafOptions(fileName string) { +func (w *docWriter) writeOptionsSection(c *Command, fileName string) { + scope := "this command" + if !w.isLeafCommand(c) { + scope = "this command and all of its subcommands" + } + var options, globalOptions []Option for i, o := range w.optionsStack { if i == len(w.optionsStack)-1 { @@ -162,14 +178,14 @@ func (w *docWriter) writeLeafOptions(fileName string) { hasGlobal := len(globalOptions) > 0 switch { case len(options) > 0 && hasGlobal: - buf.WriteString("Use the following options to change the behavior of this command. ") + buf.WriteString("Use the following options to change the behavior of " + scope + ". ") buf.WriteString("You can also use any of the [global flags](#global-flags) that apply to all subcommands.\n\n") w.writeOptionsTable(options, fileName) case len(options) > 0: - buf.WriteString("Use the following options to change the behavior of this command.\n\n") + buf.WriteString("Use the following options to change the behavior of " + scope + ".\n\n") w.writeOptionsTable(options, fileName) case hasGlobal: - buf.WriteString("Use [global flags](#global-flags) to customize the connection to the Temporal Service for this command.\n\n") + buf.WriteString("Use [global flags](#global-flags) to customize the connection to the Temporal Service for " + scope + ".\n\n") } w.collectGlobalFlags(fileName, globalOptions) @@ -232,13 +248,14 @@ func (w *docWriter) writeSplitCommand(c *Command, splitRoot string) { buf.WriteString(escapeMDXDescription(c.Description) + "\n\n") - if w.isLeafCommand(c) { - w.writeLeafOptions(fileName) - } else { + if !w.isLeafCommand(c) { buf.WriteString(fmt.Sprintf("This page provides a reference for the `temporal %s` commands. ", c.FullName)) buf.WriteString("The flags applicable to each subcommand are presented in a table within the heading for the subcommand. ") buf.WriteString("Refer to [Global Flags](#global-flags) for flags that you can use with every subcommand.\n\n") } + if w.hasOptionsSection(c) { + w.writeOptionsSection(c, fileName) + } } func (w *docWriter) writeSplitSubcommand(c *Command, splitRoot string) { @@ -257,8 +274,8 @@ func (w *docWriter) writeSplitSubcommand(c *Command, splitRoot string) { buf.WriteString(prefix + " " + leafName + "\n\n") buf.WriteString(escapeMDXDescription(c.Description) + "\n\n") - if w.isLeafCommand(c) { - w.writeLeafOptions(fileName) + if w.hasOptionsSection(c) { + w.writeOptionsSection(c, fileName) } } diff --git a/internal/commandsgen/docs_test.go b/internal/commandsgen/docs_test.go index 8e9391b79..264edaf11 100644 --- a/internal/commandsgen/docs_test.go +++ b/internal/commandsgen/docs_test.go @@ -182,3 +182,118 @@ func keys(m map[string][]byte) []string { } return out } + +// parentOptionsFixture has a parent command that defines its own options and +// can be run directly ("thing run", like "workflow reset"), alongside a parent +// that only groups subcommands ("thing group"). +const parentOptionsFixture = ` +commands: + - name: app + summary: App + description: App. + - name: app thing + summary: Thing + description: Manage things. + docs: + keywords: + - thing + description-header: Manage things + tags: + - Things + - name: app thing group + summary: Group + description: Group related subcommands. + - name: app thing group leaf + summary: Leaf + description: A leaf under a grouping parent. + options: + - name: leaf-flag + type: string + description: A leaf flag. + - name: app thing run + summary: Run + description: Run a thing. + options: + - name: target + type: string + description: The thing to run. + - name: app thing run extra + summary: Extra + description: Run a thing, then do something extra. + options: + - name: extra-flag + type: string + description: An extra flag. +` + +func TestGenerateDocsFilesParentOptions(t *testing.T) { + cmds, err := ParseCommands([]byte(parentOptionsFixture)) + if err != nil { + t.Fatalf("ParseCommands: %v", err) + } + + cases := []struct { + name string + subdirs []string + file string + run string + extra string + group string + groupLeaf string + }{ + {name: "single file", file: "thing", run: "## run", extra: "### extra", group: "## group", groupLeaf: "### leaf"}, + {name: "split subdirectory", subdirs: []string{"app"}, file: "app/thing", run: "## run", extra: "### run extra", group: "## group", groupLeaf: "### group leaf"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + docs, err := GenerateDocsFiles(cmds, tc.subdirs) + if err != nil { + t.Fatalf("GenerateDocsFiles: %v", err) + } + doc, ok := docs[tc.file] + if !ok { + t.Fatalf("expected file %q, got keys: %v", tc.file, keys(docs)) + } + + // A parent with its own options gets an option table under its heading. + run := section(t, string(doc), tc.run, tc.extra) + if !strings.Contains(run, "`--target`") { + t.Errorf("expected --target in the run section, got:\n%s", run) + } + // Its options are persistent flags, so the section says they reach subcommands. + if !strings.Contains(run, "this command and all of its subcommands") { + t.Errorf("expected the run section to say its options apply to subcommands, got:\n%s", run) + } + extra := section(t, string(doc), tc.extra, "") + if !strings.Contains(extra, "`--extra-flag`") { + t.Errorf("expected --extra-flag in the extra section, got:\n%s", extra) + } + + // A parent that only groups subcommands still gets no table. + group := section(t, string(doc), tc.group, tc.groupLeaf) + if strings.Contains(group, "| Flag |") { + t.Errorf("expected no option table in the group section, got:\n%s", group) + } + }) + } +} + +// section returns the text from the start heading up to the end heading, or to +// the end of the document when end is empty. +func section(t *testing.T, doc, start, end string) string { + t.Helper() + i := strings.Index(doc, start+"\n") + if i < 0 { + t.Fatalf("heading %q not found in:\n%s", start, doc) + } + rest := doc[i:] + if end == "" { + return rest + } + j := strings.Index(rest, end+"\n") + if j < 0 { + t.Fatalf("heading %q not found after %q in:\n%s", end, start, doc) + } + return rest[:j] +}