Skip to content

Commit 3da63b4

Browse files
committed
Reconcile default value passing and default activation
Fix #3111 Reintroduce a commit pushed by mistake in 6c4a77b and reverted in bb7be1f Refine tests introduced in 06847da for #3030 Document the interaction between `default` and `flag_value`
1 parent 878de46 commit 3da63b4

File tree

5 files changed

+144
-15
lines changed

5 files changed

+144
-15
lines changed

CHANGES.rst

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,11 @@ Unreleased
2121
``pytest-xdist`` to detect test pollution and race conditions. :pr:`3151`
2222
- Add contributor documentation for running stress tests, randomized
2323
parallel tests, and Flask smoke tests. :pr:`3151` :pr:`3177`
24+
- Fix ``default=True`` with boolean ``flag_value`` always returning the
25+
``flag_value`` instead of ``True``. The ``default=True`` to ``flag_value``
26+
substitution now only applies to non-boolean flags, where ``True`` acts as a
27+
sentinel meaning "activate this flag by default". For boolean flags,
28+
``default=True`` is returned as a literal value. :issue:`3111` :pr:`3239`
2429

2530
Version 8.3.2
2631
-------------

docs/options.md

Lines changed: 70 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -341,12 +341,13 @@ To have a flag pass a value to the decorated function set `flag_value`. This aut
341341
invoke(info)
342342
```
343343

344-
````{note}
345-
The `default` value is given to the decorated function as-is. So if you set `default=None`, the value passed to the function is the `None` Python value. Same for any other type.
344+
### How `default` and `flag_value` interact
346345

347-
But there is a special case for flags. If a flag has a `flag_value`, then setting `default=True` is interpreted as *the flag should be activated by default*. So instead of the decorated function receiving the `True` Python value, it will receive the `flag_value`.
346+
The `default` value is given to the underlying function as-is. So if you set `default=None`, the function receives `None`. Same for any other type.
348347

349-
Which means, in example above, this option:
348+
But there is a special case for **non-boolean** flags: if a flag has a non-boolean `flag_value` (like a string or a class), then `default=True` is interpreted as *the flag should be activated by default*. The function receives the `flag_value`, not the Python `True`.
349+
350+
Which means, in the example above, this option:
350351

351352
```python
352353
@click.option('--upper', 'transformation', flag_value='upper', default=True)
@@ -358,9 +359,73 @@ is equivalent to:
358359
@click.option('--upper', 'transformation', flag_value='upper', default='upper')
359360
```
360361

361-
Because the two are equivalent, it is recommended to always set `default` to the actual value you want to pass and avoid using the special `True` case. This makes the code more explicit and predictable.
362+
Because the two are equivalent, it is recommended to always use the second form and set `default` to the actual value you want. This makes code more explicit and predictable.
363+
364+
This special case does **not** apply to boolean flags (where `flag_value` is `True` or `False`). For boolean flags, `default=True` is the literal Python value `True`.
365+
366+
The tables below show the value received by the function for each combination of `default`, `flag_value`, and whether the flag was passed on the command line.
367+
368+
#### Boolean flags (`is_flag=True`, boolean `flag_value`)
369+
370+
These are flags where `flag_value` is `True` or `False`. The `default` value is always passed through literally without any special substitution.
371+
372+
| `default` | `flag_value` | Not passed | `--flag` passed |
373+
|-----------|--------------|------------|-----------------|
374+
| *(unset)* | *(unset)* | `False` | `True` |
375+
| `True` | *(unset)* | `True` | `True` |
376+
| `False` | *(unset)* | `False` | `True` |
377+
| `None` | *(unset)* | `None` | `True` |
378+
| `True` | `True` | `True` | `True` |
379+
| `True` | `False` | `True` | `False` |
380+
| `False` | `True` | `False` | `True` |
381+
| `False` | `False` | `False` | `False` |
382+
| `None` | `True` | `None` | `True` |
383+
| `None` | `False` | `None` | `False` |
384+
385+
````{tip}
386+
For a negative flag that defaults to off, prefer the explicit pair form `--with-xyz/--without-xyz` over the single-flag `flag_value=False, default=True`:
387+
388+
```python
389+
@click.option('--with-xyz/--without-xyz', 'enable_xyz', default=True)
390+
```
362391
````
363392

393+
#### Boolean flag pairs (`--flag/--no-flag`)
394+
395+
These use secondary option names to provide both an on and off switch. The `default` value is always literal.
396+
397+
| `default` | Not passed | `--flag` | `--no-flag` |
398+
|-----------|------------|----------|-------------|
399+
| *(unset)* | `False` | `True` | `False` |
400+
| `True` | `True` | `True` | `False` |
401+
| `False` | `False` | `True` | `False` |
402+
| `None` | `None` | `True` | `False` |
403+
404+
#### Non-boolean feature switches (`flag_value` is a string, class, etc.)
405+
406+
For these flags, `default=True` is a **special case**: it means "activate this flag by default" and resolves to the `flag_value`. All other `default` values are passed through literally.
407+
408+
| `default` | `flag_value` | Not passed | `--flag` passed |
409+
|------------|--------------|-------------|-----------------|
410+
| *(unset)* | `"upper"` | `None` | `"upper"` |
411+
| `True` | `"upper"` | `"upper"`¹ | `"upper"` |
412+
| `"lower"` | `"upper"` | `"lower"` | `"upper"` |
413+
| `None` | `"upper"` | `None` | `"upper"` |
414+
415+
```{hint}
416+
¹: `default=True` is substituted with `flag_value`.
417+
```
418+
419+
#### Feature switch groups (multiple flags sharing one variable)
420+
421+
When multiple `flag_value` options target the same parameter name, `default=True` on one of them marks it as the default choice.
422+
423+
| Definition | Not passed | `--upper` | `--lower` |
424+
|--------------------------------------------------------|------------|-----------|-----------|
425+
| `--upper` with `flag_value='upper'`, `default=True` | `"upper"` | `"upper"` | `"lower"` |
426+
| `--upper` with `flag_value='upper'`, `default='upper'` | `"upper"` | `"upper"` | `"lower"` |
427+
| Both without `default` | `None` | `"upper"` | `"lower"` |
428+
364429
## Values from Environment Variables
365430

366431
To pass in a value from a specific environment variable use `envvar`.

src/click/core.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2891,13 +2891,33 @@ def to_info_dict(self) -> dict[str, t.Any]:
28912891
def get_default(
28922892
self, ctx: Context, call: bool = True
28932893
) -> t.Any | t.Callable[[], t.Any] | None:
2894+
"""For non-boolean flag options, ``default=True`` is treated as a
2895+
sentinel meaning "activate this flag by default" and is resolved to
2896+
:attr:`flag_value`. This resolution is performed lazily here (rather
2897+
than eagerly in :meth:`__init__`) to prevent callable ``flag_value``
2898+
values (like classes) from being instantiated prematurely
2899+
(:issue:`3121`).
2900+
2901+
For example, with ``--upper/--lower`` feature switches where
2902+
``flag_value="upper"`` and ``default=True``, the default resolves
2903+
to ``"upper"``.
2904+
2905+
.. caution::
2906+
This substitution only applies to **non-boolean** flags
2907+
(:attr:`is_bool_flag` is ``False``). For boolean flags, ``True`` is
2908+
not a sentinel but a legitimate Python value, so ``default=True`` is
2909+
returned as-is. Without this distinction, ``flag_value=False,
2910+
default=True`` would silently always return ``False``, regardless of
2911+
whether the flag was passed or not.
2912+
2913+
.. versionchanged:: 8.3.3
2914+
``default=True`` is no longer substituted with ``flag_value`` for
2915+
boolean flags, fixing negative boolean flags like ``flag_value=False,
2916+
default=True``. :issue:`3111`
2917+
"""
28942918
value = super().get_default(ctx, call=False)
28952919

2896-
# Lazily resolve default=True to flag_value. Doing this here
2897-
# (instead of eagerly in __init__) prevents callable flag_values
2898-
# (like classes) from being instantiated by the callable check below.
2899-
# https://github.com/pallets/click/issues/3121
2900-
if value is True and self.is_flag:
2920+
if value is True and self.is_flag and not self.is_bool_flag:
29012921
value = self.flag_value
29022922
elif call and callable(value):
29032923
value = value()

tests/test_options.py

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1457,9 +1457,12 @@ def test_type_from_flag_value():
14571457
({"type": str, "flag_value": None}, [], None),
14581458
({"type": str, "flag_value": None}, ["--foo"], None),
14591459
# Not passing --foo returns the default value as-is, in its Python type, then
1460-
# converted by the option type.
1460+
# converted by the option type. For boolean flags, default=True is a literal
1461+
# value, not a sentinel meaning "activate flag". So it is NOT substituted with
1462+
# flag_value. See: https://github.com/pallets/click/issues/3111
1463+
# https://github.com/pallets/click/pull/3239
14611464
({"type": bool, "default": True, "flag_value": True}, [], True),
1462-
({"type": bool, "default": True, "flag_value": False}, [], False),
1465+
({"type": bool, "default": True, "flag_value": False}, [], True),
14631466
({"type": bool, "default": False, "flag_value": True}, [], False),
14641467
({"type": bool, "default": False, "flag_value": False}, [], False),
14651468
({"type": bool, "default": None, "flag_value": True}, [], None),
@@ -2484,6 +2487,38 @@ def rcli(scm_ignore_files):
24842487
assert result.exit_code == 0
24852488

24862489

2490+
@pytest.mark.parametrize(
2491+
("default", "args", "expected"),
2492+
[
2493+
# default=None: 3-state pattern (e.g. Flask --reload/--no-reload).
2494+
# https://github.com/pallets/click/issues/3024
2495+
(None, [], None),
2496+
(None, ["--flag"], True),
2497+
(None, ["--no-flag"], False),
2498+
# default=True: literal value, not substituted with flag_value.
2499+
# https://github.com/pallets/click/issues/3111
2500+
(True, [], True),
2501+
(True, ["--flag"], True),
2502+
(True, ["--no-flag"], False),
2503+
],
2504+
)
2505+
def test_bool_flag_pair_default(runner, default, args, expected):
2506+
"""Boolean flag pairs pass ``default`` through literally.
2507+
2508+
Ensures ``default=True`` is not replaced by ``flag_value`` for boolean
2509+
flags, and that ``default=None`` enables 3-state logic.
2510+
"""
2511+
2512+
@click.command()
2513+
@click.option("--flag/--no-flag", default=default)
2514+
def cli(flag):
2515+
click.echo(repr(flag), nl=False)
2516+
2517+
result = runner.invoke(cli, args)
2518+
assert result.exit_code == 0
2519+
assert result.output == repr(expected)
2520+
2521+
24872522
@pytest.mark.parametrize(
24882523
("flag_type", "args", "expect_output"),
24892524
[

tests/test_termui.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -825,9 +825,13 @@ def cmd(arg1):
825825
({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "", True),
826826
({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "y", True),
827827
({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "n", False),
828-
({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "", False),
829-
({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "y", True),
830-
({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "n", False),
828+
# For boolean flags, default=True is a literal value, not a sentinel meaning
829+
# "activate flag", so the prompt shows [Y/n] with default=True. See:
830+
# https://github.com/pallets/click/issues/3111
831+
# https://github.com/pallets/click/pull/3239
832+
({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "", True),
833+
({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "y", True),
834+
({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "n", False),
831835
# default=False
832836
({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "", False),
833837
({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "y", True),

0 commit comments

Comments
 (0)