[vidigoat]

What type of PR is this?

• bug

What this PR does / why we need it:

In v3 a flag is persistent unless it is Local, so a subcommand accepts its ancestors' flags, and its help lists them under GLOBAL OPTIONS. ToFishCompletion only offered each command's own flags, and the root's flags only before any subcommand, so after a subcommand fish never suggested them:

cmd := &cli.Command{
	Name: "app",
	Flags: []cli.Flag{
		&cli.StringFlag{Name: "config", Aliases: []string{"c"}, TakesFile: true},
		&cli.BoolFlag{Name: "verbose"},
		&cli.BoolFlag{Name: "local-only", Local: true},
	},
	Commands: []*cli.Command{
		{Name: "deploy", Flags: []cli.Flag{&cli.BoolFlag{Name: "dry-run"}},
			Commands: []*cli.Command{{Name: "prod"}}},
		{Name: "exec", SkipFlagParsing: true, Action: func(ctx context.Context, cmd *cli.Command) error {
			fmt.Printf("args=%q verbose=%t\n", cmd.Args().Slice(), cmd.Bool("verbose"))
			return nil
		}},
	},
}

In fish 4.9.3 with the generated script, complete -C 'app deploy --' listed --dry-run --help, while app deploy --verbose prod runs and app deploy prod --help shows --config and --verbose under GLOBAL OPTIONS. The dynamic completion (app completion fish) already offers them, since DefaultCompleteWithFlags adds VisiblePersistentFlags.

A command with SkipFlagParsing is the exception. It takes ancestor flags as plain arguments: complete -C 'app exec --' listed --config --help --verbose, but app exec --verbose x prints args=["--verbose" "x"] verbose=false. The dynamic script lists only --help there, because it adds persistent flags only when the command does not skip flag parsing.

• fish.go: prepareFishFlags now adds the command's VisiblePersistentFlags() that it doesn't define itself, the same way DefaultCompleteWithFlags does, and only when SkipFlagParsing is false. VisiblePersistentFlags already leaves out Local flags and flags a subcommand redefines.
• testdata/expected-fish-full.fish and testdata/expected-help-output.txt: regenerated. Both only gain lines, the persistent flags under each subcommand, and no existing line changes. The SkipFlagParsing case does not change them.

With this change, complete -C gives the same flags with the static script as with the dynamic one at every subcommand level: app deploy -- and app deploy prod -- both list --config --dry-run --help --verbose, --local-only stays at the root, and app deploy -c completes file names. app exec -- lists only --help.

Which issue(s) this PR fixes:

None, found while comparing the static and dynamic fish scripts.

Testing

• TestFishCompletionPersistentFlags: a root flag is offered after a subcommand and a nested subcommand (fails on main), while a Local root flag and a root flag the subcommand redefines are not. A subcommand with SkipFlagParsing does not offer a root flag. That assertion fails without the check.
• Checked the generated script in fish 4.9.3, as above.
• go test ./... passes.

The change and the test were written with Claude Code (AI).

Release Notes

The fish completion script from ToFishCompletion now offers the persistent flags of a subcommand's ancestors, such as the root's flags.

[Juneezee]

Thanks! This fixes it: on main, complete -C 'app deploy --' lists --dry-run --help, and with this PR it lists --config --dry-run --help --verbose, the same as the dynamic script.

One case is missing. The dynamic completion leaves out the persistent flags when the command has SkipFlagParsing, because that command takes them as plain arguments:

cli/help.go

Lines 320 to 327 in d854bb4

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

The fish script now offers them there:

cli/fish.go

Lines 120 to 128 in 7b83486

	func prepareFishFlags(binary string, owner *Command) []string {
	flags := owner.VisibleFlags()
	// a subcommand also accepts the persistent flags of its ancestors, which
	// its help lists as global options
	for _, fl := range owner.VisiblePersistentFlags() {
	if !hasFlag(flags, fl) {
	flags = append(flags, fl)
	}
	}

To reproduce, add this subcommand to the example in the description:

{Name: "exec", SkipFlagParsing: true, Action: func(ctx context.Context, cmd *cli.Command) error {
	fmt.Printf("args=%q verbose=%t\n", cmd.Args().Slice(), cmd.Bool("verbose"))
	return nil
}},

complete -C 'app exec --' lists --config --help --verbose, but app exec --verbose x prints args=["--verbose" "x"] verbose=false. The dynamic script lists only --help.

Could you add the same check, with a test? For example:

	// a subcommand also accepts the persistent flags of its ancestors, which
	// its help lists as global options, unless it skips flag parsing
	if !owner.SkipFlagParsing {
		for _, fl := range owner.VisiblePersistentFlags() {
			if !hasFlag(flags, fl) {
				flags = append(flags, fl)
			}
		}
	}

and in TestFishCompletionPersistentFlags, add {Name: "exec", SkipFlagParsing: true} to Commands and:

	// nor a root flag after a subcommand that skips flag parsing
	assert.NotContains(t, res, "__fish_seen_subcommand_from exec' -f -l verbose")

With both on top of current main, the full test suite passes and the new assertion fails without the check. The testdata files don't change.

[vidigoat]

@Juneezee Thank you. This is done.

prepareFishFlags adds ancestor flags only when the command does not skip flag parsing, the same check as DefaultCompleteWithFlags. TestFishCompletionPersistentFlags now has an exec command with SkipFlagParsing, and asserts the script does not offer --verbose after it. That assertion fails without the check. The description includes the same example. The testdata files are unchanged.

[Juneezee]

Thanks, LGTM.