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

If feature switch flag is a boolean, default value isn't set correctly #2351

Open
mmazur opened this issue Sep 7, 2022 · 3 comments · May be fixed by #2848
Open

If feature switch flag is a boolean, default value isn't set correctly #2351

mmazur opened this issue Sep 7, 2022 · 3 comments · May be fixed by #2848
Assignees
Labels

Comments

@mmazur
Copy link

mmazur commented Sep 7, 2022

Let's have two short code snippets that differ only in whether the flag values being set are bool vs int:

# bool.py
import click

@click.command()
@click.option('--one', 'flag', flag_value=True, default=True)
@click.option('--two', 'flag', flag_value=False)
def test(flag):
   click.echo(flag)

if __name__ == '__main__':
   test()
# int.py
import click

@click.command()
@click.option('--one', 'flag', flag_value=1, default=True)
@click.option('--two', 'flag', flag_value=0)
def test(flag):
    click.echo(flag)

if __name__ == '__main__':
    test()

Running both scripts without any parameters, I would expect to get True and 1, but that's not what happens.

[mmazur@klapek click-odd]$ ./bool.py
False
[mmazur@klapek click-odd]$ ./int.py
1

This seems wrong.

Environment:

  • Python version: Python 3.10.6
  • Click version: 8.1.3
@pocek
Copy link
Contributor

pocek commented Oct 5, 2022

(Not a click developer here, just lurking around.)

This behavior seems to be in click since forever. For some unknown reason click doesn't expect that you would to this with boolean flags:

    def get_default(
        self, ctx: Context, call: bool = True
    ) -> t.Optional[t.Union[t.Any, t.Callable[[], t.Any]]]:
        # If we're a non boolean flag our default is more complex because
        # we need to look at all flags in the same group to figure out
        # if we're the default one in which case we return the flag
        # value as default.
        if self.is_flag and not self.is_bool_flag:
            for param in ctx.command.params:
                if param.name == self.name and param.default:
                    return param.flag_value  # type: ignore

            return None

        return super().get_default(ctx, call=call)

BTW, I think the comment above is misleading. It should be:

-# if we're the default one in which case we return the flag
+# which is the default one in which case we return the flag

So, for booleans we get default value of the last parameter wins behavior; if you reverse the order of --one and --two in bool.py the output will change.

Perhaps it's expected that one would just use the usual:

@click.option('--one/--two', 'flag', default=True)

The only downside is that you get one option instead of two (or more), so YMMV.

@bow
Copy link

bow commented Nov 27, 2022

Can confirm the same behavior from my side (on 8.1.3). This was a little confusing when I discovered it the first time, since I had a different impression from reading the documentation.

Would the project be open to doing either of these two alternatives?

  • Update the documentation (specifically the feature switches section) to say that setting boolean values in feature switches does not use the default keyword argument, but rather uses whatever is set last.

  • Update the code so that boolean values behave the same as any other values.

@Rowlando13
Copy link
Collaborator

I think this is a docs problem. Feature switches docs are unclear. I will investigate.

@Rowlando13 Rowlando13 added the docs label Jan 5, 2025
@Rowlando13 Rowlando13 self-assigned this Jan 5, 2025
@Rowlando13 Rowlando13 linked a pull request Feb 2, 2025 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants