fix: complete persistent flags in shell completion

[sb123sb123]

Summary

Fixes #2173. DefaultCompleteWithFlags now suggests visible persistent flags from ancestor commands after a subcommand, while preserving command-local flags first. It avoids duplicate suggestions for shared flags, retains custom flags that implement Flag without VisibleFlag, and omits inherited flags when SkipFlagParsing is enabled.

Regression cases cover persistent flags after positional arguments, local-before-parent ordering, duplicate flags, custom flags, and skipped parsing.

Validation

• make passed, including generation, go vet, race-enabled test suites (99.4% coverage for the main package), binary-size checks, and README gfmrun examples.
• Focused shell-completion regression tests passed.
• git diff --check passed.

AI assistance disclosure

This pull request was developed with assistance from OpenAI Codex.

Release Notes

Fix shell completion suggestions for persistent flags on subcommands.

[Juneezee]

Thanks for working on this! The direction is right, but this change has a few issues:

cli/help.go

Lines 275 to 277 in 109de8b

	flags := cmd.VisiblePersistentFlags()
	flags = append(flags, cmd.VisibleFlags()...)
	printFlagSuggestions(lastArg, flags, cmd.Root().Writer)

1. Duplicate suggestions. A flag shared between a parent and the subcommand is now suggested twice:

verbose := &cli.BoolFlag{Name: "verbose"}
cmd := &cli.Command{
	Name:                  "app",
	EnableShellCompletion: true,
	Flags:                 []cli.Flag{verbose},
	Commands:              []*cli.Command{{Name: "add", Flags: []cli.Flag{verbose}}},
}

$ app add --ver --generate-shell-completion
--verbose
--verbose

2. Custom flags are dropped. cmd.VisibleFlags() only keeps flags that implement VisibleFlag, so a custom flag that only implements Flag is no longer suggested (it was before):

cli/flag.go

Lines 218 to 226 in d1d8108

	func visibleFlags(fl []Flag) []Flag {
	var visible []Flag
	for _, f := range fl {
	if vf, ok := f.(VisibleFlag); ok && vf.IsVisible() {
	visible = append(visible, f)
	}
	}
	return visible
	}

printFlagSuggestions already filters hidden flags, so cmd.allFlags() is enough here:

cli/help.go

Lines 215 to 221 in d1d8108

	func printFlagSuggestions(lastArg string, flags []Flag, writer io.Writer) {
	// Trim to handle both "-short" and "--long" flags.
	cur := strings.TrimLeft(lastArg, "-")
	for _, flag := range flags {
	if vf, ok := flag.(VisibleFlag); ok && !vf.IsVisible() {
	continue
	}

3. SkipFlagParsing. Parent flags are now suggested on a subcommand with SkipFlagParsing: true, e.g. with a root flag --rootf, app sub --r suggests --rootf. But flags are not parsed there, so app sub --rootf does not set --rootf, and it ends up in cmd.Args() instead:

cli/command_run.go

Lines 162 to 167 in d1d8108

	if cmd.SkipFlagParsing {
	tracef("skipping flag parsing (cmd=%[1]q)", cmd.Name)
	cmd.parsedArgs = args
	} else {
	cmd.parsedArgs, err = cmd.parseFlags(args)
	}

4. Order. Please list the command's own flags before the parent flags, to match the help output (OPTIONS, then GLOBAL OPTIONS):

cli/template.go

Lines 84 to 88 in d1d8108

	OPTIONS:{{template "visibleFlagCategoryTemplate" .}}{{else if .VisibleFlags}}
	OPTIONS:{{template "visibleFlagTemplate" .}}{{end}}{{if .VisiblePersistentFlags}}
	GLOBAL OPTIONS:{{template "visiblePersistentFlagTemplate" .}}{{end}}

Something like this fixes 1-4, using the existing hasFlag helper:

flags := cmd.allFlags()
if !cmd.SkipFlagParsing {
	for _, fl := range cmd.VisiblePersistentFlags() {
		if !hasFlag(flags, fl) {
			flags = append(flags, fl)
		}
	}
}
printFlagSuggestions(lastArg, flags, cmd.Root().Writer)

5. Tests. Adding Local: true to the existing fixtures keeps their old expected output, which hides the behavior change:

cli/help_test.go

Lines 1439 to 1444 in 109de8b

	parent: &Command{
	Name: "cmd",
	Flags: []Flag{
	&BoolFlag{Name: "happiness", Local: true},
	&Int64Flag{Name: "everybody-jump-on", Local: true},
	},

Please revert these and update the expected output instead. Test cases for 1-4 would be great too.

Minor: could the new test go into TestDefaultCompleteWithFlags instead of a new file?

[Juneezee]

Thanks for the update! All the review comments are addressed. I pushed a small commit that adds a test for MutuallyExclusiveFlags suggestions and simplifies the expected output in TestPartialFlagCompletionAfterPositionalArgument. LGTM.