Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

command.MarkFlagRequired Does not look at inherited flags #921

Open
dallbee opened this issue Jul 29, 2019 · 4 comments
Open

command.MarkFlagRequired Does not look at inherited flags #921

dallbee opened this issue Jul 29, 2019 · 4 comments
Labels
area/flags-args Changes to functionality around command line flags and args

Comments

@dallbee
Copy link

dallbee commented Jul 29, 2019

I believe this is broken behavior, but the documentation isn't entirely clear. It is also not the same as #723.

The functions command.MarkFlagRequired and command.MarkPersistentFlagRequired call return MarkFlagRequired(flags, name) with Flags() and PersistentFlags(), respectively. That causes them to not be aware of any inherited flags on the command.

The documentation for MarkFlagRequired:

// MarkFlagRequired adds the BashCompOneRequiredFlag annotation to the named flag if it exists,
// and causes your command to report an error if invoked without the flag.

Which reads as though it shouldn't distinguish between inherited flags and local flags. Otherwise, you have to re-declare flags at the subcommand that you want to mark them as required.

As a workaround, we're using our own utility function:

// MarkFlagsRequired looks at all flags on the current command and marks any
// matching name(s) as required. Panics if no flag is found.
func MarkFlagsRequired(cmd *cobra.Command, names ...string) {
	// cobra does not merge its local flagset with the persistent flagset 
        // when Flags() is called. By calling InheretedFlags(), we force a merge.
	cmd.InheritedFlags()
	for _, name := range names {
		if err := cobra.MarkFlagRequired(cmd.Flags(), name); err != nil {
			panic(err)
		}
	}
}

It seems like the library methods should be amended to call c.mergePersistentFlags().

@dallbee dallbee changed the title command.Mark(Persistent)FlagRequired Does not look at inherited flags command.Mark(Persistent)FlagRequired Does not look at inherited flags Jul 29, 2019
@dallbee dallbee changed the title command.Mark(Persistent)FlagRequired Does not look at inherited flags command.MarkFlagRequired Does not look at inherited flags Jul 29, 2019
@github-actions
Copy link

github-actions bot commented Apr 5, 2020

This issue is being marked as stale due to a long period of inactivity

@johnSchnake
Copy link
Collaborator

Sounds like the root issue is #961 . Not closing this as duplicate since this expands that issue slightly by pointing out the effect this has on auto-completion.

@johnSchnake johnSchnake added the area/flags-args Changes to functionality around command line flags and args label Mar 5, 2022
@johnSchnake
Copy link
Collaborator

Agree that its frustrating and confusing to have to differentiate the two the whole time; to the local function it shouldn't really matter where the flag came from, just that it is required for the command.

@shaunco
Copy link

shaunco commented Feb 7, 2023

This problem is made a bit worse by the cobra-cli's default way of using init() functions across various commands and root.go, since init() functions are processed within a package in alphabetical order of the filename ... which means root.go is almost always last and other init() calls that might try to make rootCmd flags required will do so before the int() in root.go has even executed.

Would be really nice to see cobra-cli move to a centralized set of AddCommand calls in root.go rather than having each cmd register itself.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/flags-args Changes to functionality around command line flags and args
Projects
None yet
Development

No branches or pull requests

3 participants