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

option with bool type #1287

Closed
Cologler opened this issue May 2, 2019 · 7 comments
Closed

option with bool type #1287

Cologler opened this issue May 2, 2019 · 7 comments
Labels
Milestone

Comments

@Cologler
Copy link

@Cologler Cologler commented May 2, 2019

@click.command()
@click.option('--shout/--no-shout', default=False)
def info(shout):
    pass

info()

is ok, but

@click.command()
@click.option('--shout/--no-shout', default=False, type=bool)
def info(shout):
    pass

info()

will raise a TypeError: Got secondary option for non boolean flag.

@jab

This comment has been minimized.

Copy link
Member

@jab jab commented May 3, 2019

I've tracked this down to

and type is None:

in this case type is not None, it's bool, so the condition is failing and self.is_bool_flag incorrectly gets set to False.

I can't immediately see why this is there (it just looks wrong at first glance). But blame says that line of code is 5 years old, which suggests there's more to it...

@Cologler

This comment has been minimized.

Copy link
Author

@Cologler Cologler commented May 3, 2019

I am working on a click helper library (https://github.com/Cologler/click-anno-python), so I was did a lot of tests for click.

@davidism

This comment has been minimized.

Copy link
Member

@davidism davidism commented May 3, 2019

I think this is there because the --no- option makes this a special type of boolean flag automatically, it wouldn't make sense to assign a type to it. I guess we could check if type is none or bool.

@jab

This comment has been minimized.

Copy link
Member

@jab jab commented May 4, 2019

Huh, I’d have thought using the /--no- spelling should imply type=bool, so you wouldn’t have to add that explicitly, but if you did, it would mean the same thing.

@jcrotts jcrotts added the enhancement label May 6, 2019
@jcrotts jcrotts added this to the 8.0 milestone May 6, 2019
@Ketzalkotal

This comment has been minimized.

Copy link
Contributor

@Ketzalkotal Ketzalkotal commented May 6, 2019

Taking a look at this

@Ketzalkotal

This comment has been minimized.

Copy link
Contributor

@Ketzalkotal Ketzalkotal commented May 6, 2019

Checked the Discord, it looks like KP has got it

kporangehat added a commit to kporangehat/click that referenced this issue May 6, 2019
kporangehat added a commit to kporangehat/click that referenced this issue May 6, 2019
kporangehat added a commit to kporangehat/click that referenced this issue May 6, 2019
@xingheng

This comment has been minimized.

Copy link

@xingheng xingheng commented Feb 12, 2020

Same here. type=click.BOOL repros this bug, too. Temporarily remove the type and resolved.

@davidism davidism modified the milestones: 8.0, 7.1 Feb 17, 2020
davidism added a commit that referenced this issue Feb 17, 2020
for #1287: allow bool type with bool flags
@davidism davidism closed this Feb 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.