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.is_bool_flag is set to True even if the option is not a flag #1925

Closed
janluke opened this issue May 21, 2021 · 2 comments · Fixed by #1934
Closed

Option.is_bool_flag is set to True even if the option is not a flag #1925

janluke opened this issue May 21, 2021 · 2 comments · Fixed by #1934
Milestone

Comments

@janluke
Copy link
Contributor

janluke commented May 21, 2021

This is relevant only for Click 8.0.1, since the following line was introduced in PR #1913:

self.is_bool_flag = isinstance(self.type, types.BoolParamType)

The solution is as easy as:

self.is_bool_flag = self.is_flag and isinstance(self.type, types.BoolParamType)

Available to open a PR.

Demonstration code

import click

@click.command()
@click.option('--switch', type=bool)
def f(switch):
    print(switch)


if __name__ == '__main__':
    assert not f.params[0].is_flag
    assert f.params[0].is_bool_flag
    f('--switch'.split())
Error: Option '--switch' requires an argument.
@frenzymadness
Copy link

Not sure if this is related but an option defined like this:

@click.option("--code", nargs=1, default=False, help="Foo")

Had a type TEXT in click 7

# python3 test.py --help
Usage: test.py [OPTIONS]

Options:
  --code TEXT  Foo
  --help       Show this message and exit.

and now has a type BOOLEAN in click 8.

# python3 test.py --help
Usage: test.py [OPTIONS]

Options:
  --code BOOLEAN  Foo
  --help          Show this message and exit.

I'm trying to fix it in pipenv because I think the type should be explicitly defined: pypa/pipenv#4726

@janluke
Copy link
Contributor Author

janluke commented Jun 25, 2021

@frenzymadness Your thing is not related to this issue specifically, but it's related to the same PR. The flag_value attribute is always set to not default by Click, even if the options is not a flag. That, combined with the PR, seems to be the source of the change (but I could be wrong).

I'd say the new behavior makes much more sense. What's the point of setting a string options to False? You can just omit default (*) or set it to "".

(*) this should be the preferred solution, especially if you need to handle the "not provided" case separately and you are using static typing.

davidism pushed a commit to janluke/click that referenced this issue Jul 3, 2021
davidism pushed a commit to janluke/click that referenced this issue Jul 3, 2021
@davidism davidism closed this as completed Jul 3, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 18, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants