Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 32 additions & 15 deletions internal/commandsgen/docs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep parent-only flags out of the global table

When an option-bearing parent is followed directly by its child, this new call emits the parent's options under its own heading, but processing the child subsequently treats that parent's optionsStack frame as globalOptions and collects it into the same file's Global Flags section. For example, a minimal app thing run / app thing run extra tree lists --target both under run and under Global Flags, where the generated text incorrectly claims it is valid for sibling commands. Intermediate persistent flags need to be excluded from the file-wide globals or otherwise prevented from being emitted a second time.

Useful? React with 👍 / 👎.

}
}

// 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 {
Expand All @@ -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)
Expand Down Expand Up @@ -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) {
Expand All @@ -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)
}
}

Expand Down
115 changes: 115 additions & 0 deletions internal/commandsgen/docs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
}
Loading