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

Detect default true values for boolean flags and display the disable syntax #243

Open
wrouesnel opened this issue Jul 26, 2018 · 2 comments

Comments

@wrouesnel
Copy link

If I have a flag like

app.Flag("store-token", "Store the acquired token in the system keyring").
    Default("true").BoolVar(&authConfig.StoreToken)

Then currently it renders as

--store-token                  Store the acquired token in the system keyring

Even though the default value is always enabled.

It would be nice to have this situation detected, the default help show the parameter the user should actually set - i.e.

--no-store-token                  Store the acquired token in the system keyring

I guess I don't know how the description should be handled? Possibly an extra paramter on bool's for "negative description" so something like

app.Flag("store-token", "Store the acquired token in the system keyring").
    Default("true").BoolVar(&authConfig.StoreToken).NegationDescription("Do not store the acquired token in the system keyring")
negz pushed a commit to planetlabs/draino that referenced this issue Aug 29, 2018
This changes the default behaviour in that Draino would previously evict
unreplicated pods and pods with local storage, and now will not. This is
not ideal, but alecthomas/kingpin#243 would
result in a confusing user experience if we used a mix of default true and
default false boolean flags. Given the project is about 10 minutes old it
seems acceptable to make this change.
@supra08
Copy link

supra08 commented Jan 4, 2020

Hi @wrouesnel! I would like to work on this issue.

@autarch
Copy link
Contributor

autarch commented Feb 16, 2020

@supra08 If you can produce a PR I'd be happy to review it (I don't have merge power, but I think having an additional reviewer can help the maintainer).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants