From b64ea07128a6368b5f6f93035c75d5693c7ba572 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Wed, 6 Aug 2025 12:26:04 +0400 Subject: [PATCH 01/37] Differentiate absence of value and value of absence for `default` and `flag_value` Closes: #1992 #2012 #2514 #2610 #3024 --- CHANGES.rst | 18 +- docs/options.md | 11 +- src/click/_utils.py | 25 ++ src/click/core.py | 527 ++++++++++++++---------- src/click/parser.py | 33 +- src/click/termui.py | 2 + tests/test_arguments.py | 171 ++++++-- tests/test_basic.py | 64 ++- tests/test_options.py | 880 ++++++++++++++++++++++++++++++---------- tests/test_termui.py | 195 +++++++++ tests/test_utils.py | 52 +++ 11 files changed, 1486 insertions(+), 492 deletions(-) create mode 100644 src/click/_utils.py diff --git a/CHANGES.rst b/CHANGES.rst index c2e019da34..b6e948e694 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,19 +5,31 @@ Version 8.2.x Unreleased -- show correct auto complete value for nargs option in combination with flag option :issue:`2813` +- Rework flag value passing by disentangling use of ``default``. Formerly defaults + for ``flag_value`` were set by passing ``default=True``, now set by passing + ``default=``. This change breaks all default settings for flag value + type flags. In all other places in code, ``default`` takes on the default value. + Fixing this inconsistency allows the fixing of the linked issues and supports a + more consistent API. :issue:`1992` :issue:`2012`` :issue:`2514` :issue:`2610` + :issue:`3024` :pr:`3030` +- Custom ``callback`` function are now allowed to receive the ``UNSET`` sentinel + value. :pr:`3030` +- Allow ``default`` to be set on ``Argument`` for ``nargs = -1``. :issue:`2164` + :pr:`3030` +- Show correct auto complete value for ``nargs`` option in combination with flag + option :issue:`2813` Version 8.2.2 ------------- Released 2025-07-31 -- Fix reconciliation of `default`, `flag_value` and `type` parameters for +- Fix reconciliation of ``default``, ``flag_value`` and ``type`` parameters for flag options, as well as parsing and normalization of environment variables. :issue:`2952` :pr:`2956` - Fix typing issue in ``BadParameter`` and ``MissingParameter`` exceptions for the parameter ``param_hint`` that did not allow for a sequence of string where the - underlying functino ``_join_param_hints`` allows for it. :issue:`2777` :pr:`2990` + underlying function ``_join_param_hints`` allows for it. :issue:`2777` :pr:`2990` - Use the value of ``Enum`` choices to render their default value in help screen. Refs :issue:`2911` :pr:`3004` - Fix completion for the Z shell (``zsh``) for completion items containing diff --git a/docs/options.md b/docs/options.md index 48c35c801e..4b4a701059 100644 --- a/docs/options.md +++ b/docs/options.md @@ -314,7 +314,7 @@ If you want to define an alias for the second option only, then you will need to ## Flag Value -To have an flag pass a value to the underlying function set `flag_value`. This automatically sets `is_flag=True`. To set a default flag, set `default=True`. Setting flag values can be used to create patterns like this: +To have an flag pass a value to the underlying function set `flag_value`. This automatically sets `is_flag=True`. To influence the value-less behavior, force its value with `default='upper'`. Setting flag values can be used to create patterns like this: ```{eval-rst} .. click:example:: @@ -322,7 +322,7 @@ To have an flag pass a value to the underlying function set `flag_value`. This a import sys @click.command() - @click.option('--upper', 'transformation', flag_value='upper', default=True) + @click.option('--upper', 'transformation', flag_value='upper', default='upper') @click.option('--lower', 'transformation', flag_value='lower') def info(transformation): click.echo(getattr(sys.platform, transformation)()) @@ -386,10 +386,11 @@ Here are the rules used to parse environment variable values for flag options: - If the flag option has a `flag_value` argument, passing that value in the environment variable will activate the flag, in addition to all the cases described above - Any other value is interpreted as deactivating the flag -.. caution:: - For boolean flags with a pair of values, the only recognized environment variable is the one provided to the `envvar` argument. +```{caution} +For boolean flags with a pair of values, the only recognized environment variable is the one provided to the `envvar` argument. - So an option defined as `--flag\--no-flag`, with a `envvar="FLAG"` parameter, there is no magical `NO_FLAG=` variable that is recognized. Only the `FLAG=` environment variable is recognized. +So an option defined as `--flag\--no-flag`, with a `envvar="FLAG"` parameter, there is no magical `NO_FLAG=` variable that is recognized. Only the `FLAG=` environment variable is recognized. +``` Once the status of the flag has been determine to be activated or not, the `flag_value` is used as the value of the flag if it is activated. If the flag is not activated, the value of the flag is set to `None` by default. diff --git a/src/click/_utils.py b/src/click/_utils.py new file mode 100644 index 0000000000..a92dea3d84 --- /dev/null +++ b/src/click/_utils.py @@ -0,0 +1,25 @@ +from __future__ import annotations + +import enum +import typing as t + + +class Sentinel(enum.Enum): + """Enum used to define sentinel values. + + .. seealso:: + + `PEP 661 - Sentinel Values `_. + """ + + UNSET = object() + + def __repr__(self) -> str: + return f"{self.__class__.__name__}.{self.name}" + + +UNSET = Sentinel.UNSET +"""A sentinel object used to indicate that a value is not set.""" + +T_UNSET = t.Literal[UNSET] # type: ignore[valid-type] +"""Type hint for the :data:`UNSET` sentinel value.""" diff --git a/src/click/core.py b/src/click/core.py index 583cc895d9..52c19cde6d 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -19,6 +19,7 @@ from types import TracebackType from . import types +from ._utils import UNSET from .exceptions import Abort from .exceptions import BadParameter from .exceptions import ClickException @@ -686,14 +687,14 @@ def lookup_default(self, name: str, call: bool = True) -> t.Any | None: Added the ``call`` parameter. """ if self.default_map is not None: - value = self.default_map.get(name) + value = self.default_map.get(name, UNSET) if call and callable(value): return value() return value - return None + return UNSET def fail(self, message: str) -> t.NoReturn: """Aborts the execution of the program with a specific error @@ -834,7 +835,7 @@ def get_parameter_source(self, name: str) -> ParameterSource | None: :rtype: ParameterSource .. versionchanged:: 8.0 - Returns ``None`` if the parameter was not provided from any + Returns `None` if the parameter was not provided from any source. """ return self._parameter_source.get(name) @@ -863,7 +864,7 @@ class Command: If enabled this will add ``--help`` as argument if no arguments are passed :param hidden: hide this command from help outputs. - :param deprecated: If ``True`` or non-empty string, issues a message + :param deprecated: If `True` or non-empty string, issues a message indicating that the command is deprecated and highlights its deprecation in --help. The message can be customized by using a string as the value. @@ -1024,7 +1025,7 @@ def get_help_option_names(self, ctx: Context) -> list[str]: def get_help_option(self, ctx: Context) -> Option | None: """Returns the help option object. - Skipped if :attr:`add_help_option` is ``False``. + Skipped if :attr:`add_help_option` is `False`. .. versionchanged:: 8.1.8 The help option is now cached to avoid creating it multiple times. @@ -1194,7 +1195,7 @@ def parse_args(self, ctx: Context, args: list[str]) -> list[str]: opts, args, param_order = parser.parse_args(args=args) for param in iter_params_for_processing(param_order, self.get_params(ctx)): - value, args = param.handle_parse_result(ctx, opts, args) + _, args = param.handle_parse_result(ctx, opts, args) if args and not ctx.allow_extra_args and not ctx.resilient_parsing: ctx.fail( @@ -1734,7 +1735,7 @@ def function(value: t.Any, /, *args: t.Any, **kwargs: t.Any) -> t.Any: def get_command(self, ctx: Context, cmd_name: str) -> Command | None: """Given a context and a command name, this returns a :class:`Command` - object if it exists or returns ``None``. + object if it exists or returns `None`. """ return self.commands.get(cmd_name) @@ -2015,14 +2016,16 @@ class Parameter: :param is_eager: eager values are processed before non eager ones. This should not be set for arguments or it will inverse the order of processing. - :param envvar: a string or list of strings that are environment variables - that should be checked. + :param envvar: environment variable(s) that are used to provide a default value for + this parameter. This can be a string or a sequence of strings. If a + sequence is given, only the first non-empty environment variable is + used for the parameter. :param shell_complete: A function that returns custom shell completions. Used instead of the param's type completion if given. Takes ``ctx, param, incomplete`` and must return a list of :class:`~click.shell_completion.CompletionItem` or a list of strings. - :param deprecated: If ``True`` or non-empty string, issues a message + :param deprecated: If `True` or non-empty string, issues a message indicating that the argument is deprecated and highlights its deprecation in --help. The message can be customized by using a string as the value. A deprecated parameter @@ -2057,7 +2060,7 @@ class Parameter: .. versionchanged:: 8.0 Setting a default is no longer required for ``nargs>1``, it will - default to ``None``. ``multiple=True`` or ``nargs=-1`` will + default to `None`. ``multiple=True`` or ``nargs=-1`` will default to ``()``. .. versionchanged:: 7.1 @@ -2078,7 +2081,7 @@ def __init__( param_decls: cabc.Sequence[str] | None = None, type: types.ParamType | t.Any | None = None, required: bool = False, - default: t.Any | t.Callable[[], t.Any] | None = None, + default: t.Any | t.Callable[[], t.Any] | None = UNSET, callback: t.Callable[[Context, Parameter, t.Any], t.Any] | None = None, nargs: int | None = None, multiple: bool = False, @@ -2127,40 +2130,6 @@ def __init__( f" type {self.type!r}, but it was {nargs}." ) - # Skip no default or callable default. - check_default = default if not callable(default) else None - - if check_default is not None: - if multiple: - try: - # Only check the first value against nargs. - check_default = next(_check_iter(check_default), None) - except TypeError: - raise ValueError( - "'default' must be a list when 'multiple' is true." - ) from None - - # Can be None for multiple with empty default. - if nargs != 1 and check_default is not None: - try: - _check_iter(check_default) - except TypeError: - if multiple: - message = ( - "'default' must be a list of lists when 'multiple' is" - " true and 'nargs' != 1." - ) - else: - message = "'default' must be a list when 'nargs' != 1." - - raise ValueError(message) from None - - if nargs > 1 and len(check_default) != nargs: - subject = "item length" if multiple else "length" - raise ValueError( - f"'default' {subject} must match nargs={nargs}." - ) - if required and deprecated: raise ValueError( f"The {self.param_type_name} '{self.human_readable_name}' " @@ -2175,6 +2144,13 @@ def to_info_dict(self) -> dict[str, t.Any]: Use :meth:`click.Context.to_info_dict` to traverse the entire CLI structure. + .. versionchanged:: 8.3.0 + Returns `None` for the :attr:`default` if it is :attr:`UNSET`. + + We explicitly hide the :attr:`UNSET` value to the user, as we choose to + make it an implementation detail. And because ``to_info_dict`` has been + designed for documentation purposes, we return `None` instead. + .. versionadded:: 8.0 """ return { @@ -2186,7 +2162,7 @@ def to_info_dict(self) -> dict[str, t.Any]: "required": self.required, "nargs": self.nargs, "multiple": self.multiple, - "default": self.default, + "default": self.default if self.default is not UNSET else None, "envvar": self.envvar, } @@ -2254,7 +2230,7 @@ def get_default( """ value = ctx.lookup_default(self.name, call=False) # type: ignore - if value is None: + if value is UNSET: value = self.default if call and callable(value): @@ -2268,20 +2244,39 @@ def add_to_parser(self, parser: _OptionParser, ctx: Context) -> None: def consume_value( self, ctx: Context, opts: cabc.Mapping[str, t.Any] ) -> tuple[t.Any, ParameterSource]: - value = opts.get(self.name) # type: ignore - source = ParameterSource.COMMANDLINE + """Returns the parameter value produced by the parser. - if value is None: - value = self.value_from_envvar(ctx) - source = ParameterSource.ENVIRONMENT + If the parser did not produce a value from user input, the value is either + sourced from the environment variable, the default map, or the parameter's + default value. In that order of precedence. - if value is None: - value = ctx.lookup_default(self.name) # type: ignore - source = ParameterSource.DEFAULT_MAP + If no value is found, the value is returned as :attr:`UNSET`. + """ + # If the value is set, it means the parser produced a value from user input. + value = opts.get(self.name, UNSET) # type: ignore + source = ( + ParameterSource.COMMANDLINE + if value is not UNSET + else ParameterSource.DEFAULT + ) - if value is None: - value = self.get_default(ctx) - source = ParameterSource.DEFAULT + if value is UNSET: + envvar_value = self.value_from_envvar(ctx) + if envvar_value is not None: + value = envvar_value + source = ParameterSource.ENVIRONMENT + + if value is UNSET: + default_map_value = ctx.lookup_default(self.name) # type: ignore + if default_map_value is not UNSET: + value = default_map_value + source = ParameterSource.DEFAULT_MAP + + if value is UNSET: + default_value = self.get_default(ctx) + if default_value is not UNSET: + value = default_value + source = ParameterSource.DEFAULT return value, source @@ -2289,8 +2284,11 @@ def type_cast_value(self, ctx: Context, value: t.Any) -> t.Any: """Convert and validate a value against the option's :attr:`type`, :attr:`multiple`, and :attr:`nargs`. """ - if value is None: - return () if self.multiple or self.nargs == -1 else None + if value in (None, UNSET): + if self.multiple or self.nargs == -1: + return () + else: + return value def check_iter(value: t.Any) -> cabc.Iterator[t.Any]: try: @@ -2303,6 +2301,8 @@ def check_iter(value: t.Any) -> cabc.Iterator[t.Any]: _("Value must be an iterable."), ctx=ctx, param=self ) from None + # Define the conversion function based on nargs and type. + if self.nargs == 1 or self.type.is_composite: def convert(value: t.Any) -> t.Any: @@ -2337,7 +2337,15 @@ def convert(value: t.Any) -> t.Any: # tuple[t.Any, ...] return convert(value) def value_is_missing(self, value: t.Any) -> bool: - if value is None: + """ + A value is considered missing if: + + - it is :attr:`UNSET`, + - or if it is an empty sequence while the parameter is suppose to have + non-single value (i.e. :attr:`nargs` is not ``1`` or :attr:`multiple` is + set). + """ + if value is UNSET: return True if (self.nargs != 1 or self.multiple) and value == (): @@ -2346,6 +2354,22 @@ def value_is_missing(self, value: t.Any) -> bool: return False def process_value(self, ctx: Context, value: t.Any) -> t.Any: + """Process the value of this parameter: + + 1. Type cast the value using :meth:`type_cast_value`. + 2. Check if the value is missing (see: :meth:`value_is_missing`), and raise + :exc:`MissingParameter` if it is required. + 3. If a :attr:`callback` is set, call it to have the value replaced buy the + result of the callback. + + .. versionchanged:: 8.3 + + The :attr:`callback` is now receiving the :attr:`UNSET` sentinel if the + parameter is not given by the CLI user. + + The callback is receiving the value as-is, which let the developer decide + how to handle the different cases of `None`, ``UNSET``, or any other value. + """ value = self.type_cast_value(ctx, value) if self.required and self.value_is_missing(value): @@ -2357,71 +2381,108 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any: return value def resolve_envvar_value(self, ctx: Context) -> str | None: - if self.envvar is None: - return None + """Returns the value found in the environment variable(s) attached to this + parameter. - if isinstance(self.envvar, str): - rv = os.environ.get(self.envvar) + Environment variables values are `always returned as strings + `_. - if rv: - return rv - else: - for envvar in self.envvar: - rv = os.environ.get(envvar) + This method returns `None` if: + + - the :attr:`envvar` property is not set on the :class:`Parameter`, + - the environment variable is not found in the environment, + - the variable is found in the environment but its value is empty (i.e. the + environment variable is present but has an empty string). - if rv: - return rv + If :attr:`envvar` is setup with multiple environment variables, + then only the first non-empty value is returned. + + .. caution:: + + The raw value extracted from the environment is not normalized and is + returned as-is. Any normalization or reconciliation is performed later by + the :class:`Parameter`'s :attr:`type`. + """ + if not self.envvar: + return None + + envvars = [self.envvar] if isinstance(self.envvar, str) else self.envvar + for envvar in envvars: + raw_value = os.environ.get(envvar) + + # Return the first non-empty value of the list of environment variables. + if raw_value: + return raw_value + # Else, absence of value is interpreted as an environment variable that is + # not set, so proceed to the next one. return None - def value_from_envvar(self, ctx: Context) -> t.Any | None: - rv: t.Any | None = self.resolve_envvar_value(ctx) + def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: + """Process the raw environment variable string for this parameter. - if rv is not None and self.nargs != 1: - rv = self.type.split_envvar_value(rv) + Returns the string as-is or splits it into a sequence of strings if the + parameter is expecting multiple values (i.e. its :attr:`nargs` property is set + to a value other than ``1``). + """ + raw_value = self.resolve_envvar_value(ctx) - return rv + if raw_value is not None and self.nargs != 1: + return self.type.split_envvar_value(raw_value) + + return raw_value def handle_parse_result( self, ctx: Context, opts: cabc.Mapping[str, t.Any], args: list[str] ) -> tuple[t.Any, list[str]]: + """Process the value produced by the parser from user input.""" with augment_usage_errors(ctx, param=self): value, source = self.consume_value(ctx, opts) - if ( - self.deprecated - and value is not None - and source - not in ( - ParameterSource.DEFAULT, - ParameterSource.DEFAULT_MAP, - ) - ): - extra_message = ( - f" {self.deprecated}" if isinstance(self.deprecated, str) else "" - ) - message = _( - "DeprecationWarning: The {param_type} {name!r} is deprecated." - "{extra_message}" - ).format( - param_type=self.param_type_name, - name=self.human_readable_name, - extra_message=extra_message, - ) - echo(style(message, fg="red"), err=True) - ctx.set_parameter_source(self.name, source) # type: ignore + # Only try to process the value if it is not coming from the defaults. + # Defaults set by the users are under their control and so are left + # untouched as Python values: these values does not need to be processed. + if source not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP): + # If the parameter is deprecated, we want to warn the user about it. + # If the value is UNSET there is no need to warn the user of a + # parameter that has not been invoked. + if self.deprecated and value is not UNSET: + extra_message = ( + f" {self.deprecated}" + if isinstance(self.deprecated, str) + else "" + ) + message = _( + "DeprecationWarning: The {param_type} {name!r} is deprecated." + "{extra_message}" + ).format( + param_type=self.param_type_name, + name=self.human_readable_name, + extra_message=extra_message, + ) + echo(style(message, fg="red"), err=True) + try: value = self.process_value(ctx, value) except Exception: if not ctx.resilient_parsing: raise - value = None + # Ignore values that we have a hard time processing. + value = UNSET - if self.expose_value: - ctx.params[self.name] = value # type: ignore + if self.expose_value and self.name not in ctx.params: + # Click is logically enforcing that the name is None if the parameter is + # not exposed. We still assert it here to please the type checker. + assert self.name is not None, ( + f"{self!r} parameter's name should not be None when exposing value." + ) + # Normalize unset values to None, as we're about to pass them to the + # command function and move them to the pure-Python realm of user-written + # code. + ctx.params[self.name] = value if value is not UNSET else None return value, args @@ -2470,25 +2531,25 @@ class Option(Parameter): :param show_default: Show the default value for this option in its help text. Values are not shown by default, unless - :attr:`Context.show_default` is ``True``. If this value is a + :attr:`Context.show_default` is `True`. If this value is a string, it shows that string in parentheses instead of the actual value. This is particularly useful for dynamic options. For single option boolean flags, the default remains hidden if - its value is ``False``. + its value is `False`. :param show_envvar: Controls if an environment variable should be shown on the help page and error messages. Normally, environment variables are not shown. - :param prompt: If set to ``True`` or a non empty string then the - user will be prompted for input. If set to ``True`` the prompt + :param prompt: If set to `True` or a non empty string then the + user will be prompted for input. If set to `True` the prompt will be the option name capitalized. A deprecated option cannot be prompted. :param confirmation_prompt: Prompt a second time to confirm the value if it was prompted for. Can be set to a string instead of - ``True`` to customize the message. - :param prompt_required: If set to ``False``, the user will be + `True` to customize the message. + :param prompt_required: If set to `False`, the user will be prompted for input only when the option was specified as a flag without a value. - :param hide_input: If this is ``True`` then the input on the prompt + :param hide_input: If this is `True` then the input on the prompt will be hidden from the user. This is useful for password input. :param is_flag: forces this option to act as a flag. The default is auto detection. @@ -2522,7 +2583,7 @@ class Option(Parameter): .. versionchanged:: 8.1 The default of a single option boolean flag is not shown if the - default value is ``False``. + default value is `False`. .. versionchanged:: 8.0.1 ``type`` is detected from ``flag_value`` if given. @@ -2539,7 +2600,7 @@ def __init__( prompt_required: bool = True, hide_input: bool = False, is_flag: bool | None = None, - flag_value: t.Any | None = None, + flag_value: t.Any = UNSET, multiple: bool = False, count: bool = False, allow_from_autoenv: bool = True, @@ -2582,46 +2643,47 @@ def __init__( self.hide_input = hide_input self.hidden = hidden - # If prompt is enabled but not required, then the option can be - # used as a flag to indicate using prompt or flag_value. + # The _flag_needs_value property tells the parser that this option is a flag + # that cannot be used standalone and needs a value. With this information, the + # parser can determine whether to consider the next user-provided argument in + # the CLI as a value for this flag or as a new option. + # If prompt is enabled but not required, then it opens the possibility for the + # option to gets its value from the user. self._flag_needs_value = self.prompt is not None and not self.prompt_required + # Auto-detect if this is a flag or not. if is_flag is None: # Implicitly a flag because flag_value was set. - if flag_value is not None: + if flag_value is not UNSET: is_flag = True # Not a flag, but when used as a flag it shows a prompt. elif self._flag_needs_value: is_flag = False - # Implicitly a flag because flag options were given. + # Implicitly a flag because secondary options names were given. elif self.secondary_opts: is_flag = True + # The option is explicitly not a flag. But we do not know yet if it needs a + # value or not. So we look at the default value to determine it. elif is_flag is False and not self._flag_needs_value: - # Not a flag, and prompt is not enabled, can be used as a - # flag if flag_value is set. - self._flag_needs_value = flag_value is not None - - self.default: t.Any | t.Callable[[], t.Any] + self._flag_needs_value = self.default is UNSET if is_flag: # Set missing default for flags if not explicitly required or prompted. - if self.default is None and not self.required and not self.prompt: + if self.default is UNSET and not self.required and not self.prompt: if multiple: self.default = () - else: - self.default = False - - if flag_value is None: - # A boolean flag presence in the command line is enough to set - # the value: to the default if it is not blank, or to True - # otherwise. - flag_value = self.default if self.default else True - self.type: types.ParamType - if is_flag and type is None: - # Re-guess the type from the flag value instead of the - # default. - self.type = types.convert_type(None, flag_value) + # Auto-detect the type of the flag based on the flag_value. + if type is None: + # A flag without a flag_value is a boolean flag. + if flag_value is UNSET: + self.type = types.BoolParamType() + # If the flag value is a boolean, use BoolParamType. + elif isinstance(flag_value, bool): + self.type = types.BoolParamType() + # Otherwise, guess the type from the flag value. + else: + self.type = types.convert_type(None, flag_value) self.is_flag: bool = bool(is_flag) self.is_bool_flag: bool = bool( @@ -2629,12 +2691,24 @@ def __init__( ) self.flag_value: t.Any = flag_value - # Counting + # Set boolean flag default to False if unset and not required. + if self.is_bool_flag: + if self.default is UNSET and not self.required: + self.default = False + + # Set the default flag_value if it is not set. + if self.flag_value is UNSET: + if self.is_flag: + self.flag_value = True + else: + self.flag_value = None + + # Counting. self.count = count if count: if type is None: self.type = types.IntRange(min=0) - if self.default is None: + if self.default is UNSET: self.default = 0 self.allow_from_autoenv = allow_from_autoenv @@ -2650,9 +2724,6 @@ def __init__( if self.nargs == -1: raise TypeError("nargs=-1 is not supported for options.") - if self.prompt and self.is_flag and not self.is_bool_flag: - raise TypeError("'prompt' is not valid for non-boolean flag.") - if not self.is_bool_flag and self.secondary_opts: raise TypeError("Secondary flag is not valid for non-boolean flag.") @@ -2669,12 +2740,20 @@ def __init__( raise TypeError("'count' is not valid with 'is_flag'.") def to_info_dict(self) -> dict[str, t.Any]: + """ + .. versionchanged:: 8.3.0 + Returns `None` for the :attr:`flag_value` if it is :attr:`UNSET`. + + We explicitly hide the :attr:`UNSET` value to the user, as we choose to + make it an implementation detail. And because ``to_info_dict`` has been + designed for documentation purposes, we return `None` instead. + """ info_dict = super().to_info_dict() info_dict.update( help=self.help, prompt=self.prompt, is_flag=self.is_flag, - flag_value=self.flag_value, + flag_value=self.flag_value if self.flag_value is not UNSET else None, count=self.count, hidden=self.hidden, ) @@ -2867,7 +2946,9 @@ def get_help_extra(self, ctx: Context) -> types.OptionHelpExtra: elif ctx.show_default is not None: show_default = ctx.show_default - if show_default_is_str or (show_default and (default_value is not None)): + if show_default_is_str or ( + show_default and (default_value not in (None, UNSET)) + ): if show_default_is_str: default_string = f"({self.show_default})" elif isinstance(default_value, (list, tuple)): @@ -2907,32 +2988,6 @@ def get_help_extra(self, ctx: Context) -> types.OptionHelpExtra: return extra - @t.overload - def get_default( - self, ctx: Context, call: t.Literal[True] = True - ) -> t.Any | None: ... - - @t.overload - def get_default( - self, ctx: Context, call: bool = ... - ) -> t.Any | t.Callable[[], t.Any] | None: ... - - def get_default( - self, ctx: Context, call: bool = True - ) -> t.Any | t.Callable[[], t.Any] | None: - # 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 is not None: - return t.cast(Option, param).default - - return None - - return super().get_default(ctx, call=call) - def prompt_for_value(self, ctx: Context) -> t.Any: """This is an alternative flow that can be activated in the full value processing if a value does not exist. It will prompt the @@ -2941,12 +2996,18 @@ def prompt_for_value(self, ctx: Context) -> t.Any: """ assert self.prompt is not None - # Calculate the default before prompting anything to be stable. + # Calculate the default before prompting anything to lock in the value before + # attempting any user interaction. default = self.get_default(ctx) - # If this is a prompt for a flag we need to handle this - # differently. + # A boolean flag can use a simplified [y/n] confirmation prompt. if self.is_bool_flag: + # If we have no boolean default, we force the user to explicitly provide + # one. + if default in (UNSET, None): + default = None + else: + default = bool(default) return confirm(self.prompt, default) # If show_default is set to True/False, provide this to `prompt` as well. For @@ -2957,7 +3018,9 @@ def prompt_for_value(self, ctx: Context) -> t.Any: return prompt( self.prompt, - default=default, + # Use `None` to inform the prompt() function to reiterate until a valid + # value is provided by the user if we have no default. + default=None if default is UNSET else default, type=self.type, hide_input=self.hide_input, show_choices=self.show_choices, @@ -2967,22 +3030,17 @@ def prompt_for_value(self, ctx: Context) -> t.Any: ) def resolve_envvar_value(self, ctx: Context) -> str | None: - """Find which environment variable to read for this option and return - its value. - - Returns the value of the environment variable if it exists, or ``None`` - if it does not. - - .. caution:: - - The raw value extracted from the environment is not normalized and - is returned as-is. Any normalization or reconciation with the - option's type should happen later. + """:class:`Option` resolves its environment variable the same way as + :func:`Parameter.resolve_envvar_value`, but it also supports + :attr:`Context.auto_envvar_prefix`. If we could not find an environment from + the :attr:`envvar` property, we fallback on :attr:`Context.auto_envvar_prefix` + to build dynamiccaly the environment variable name using the + :python:`{ctx.auto_envvar_prefix}_{self.name.upper()}` template. """ - rv = super().resolve_envvar_value(ctx) + raw_value = super().resolve_envvar_value(ctx) - if rv is not None: - return rv + if raw_value is not None: + return raw_value if ( self.allow_from_autoenv @@ -2990,82 +3048,111 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: and self.name is not None ): envvar = f"{ctx.auto_envvar_prefix}_{self.name.upper()}" - rv = os.environ.get(envvar) + raw_value = os.environ.get(envvar) - if rv: - return rv + # Return the first non-empty value of the list of environment variables. + if raw_value: + return raw_value + # Else, absence of value is interpreted as an environment variable that is + # not set, so proceed to the next one. return None - def value_from_envvar(self, ctx: Context) -> t.Any | None: - """Normalize the value from the environment variable, if it exists.""" - rv: str | None = self.resolve_envvar_value(ctx) + def value_from_envvar(self, ctx: Context) -> t.Any: + """ + For :class:`Option`, this method processes the raw environment variable string + the same way as :func:`Parameter.value_from_envvar` does. - if rv is None: + But in the case of non-boolean flags, the value is analyzed to determine if the + flag is activated or not, and returns a boolean of its activation, or the + :attr:`flag_value` if the latter is set. + + This method also takes care of repeated options (i.e. options with + :attr:`multiple` set to `True`). + """ + raw_value = self.resolve_envvar_value(ctx) + + # Absent environment variable or an empty string is interpreted as unset. + if raw_value is None: return None # Non-boolean flags are more liberal in what they accept. But a flag being a - # flag, its envvar value still needs to analyzed to determine if the flag is + # flag, its envvar value still needs to be analyzed to determine if the flag is # activated or not. if self.is_flag and not self.is_bool_flag: # If the flag_value is set and match the envvar value, return it # directly. - if self.flag_value is not None and rv == self.flag_value: + if self.flag_value is not UNSET and raw_value == self.flag_value: return self.flag_value # Analyze the envvar value as a boolean to know if the flag is # activated or not. - return types.BoolParamType.str_to_bool(rv) + return types.BoolParamType.str_to_bool(raw_value) # Split the envvar value if it is allowed to be repeated. value_depth = (self.nargs != 1) + bool(self.multiple) if value_depth > 0: - multi_rv = self.type.split_envvar_value(rv) + multi_rv = self.type.split_envvar_value(raw_value) if self.multiple and self.nargs != 1: multi_rv = batch(multi_rv, self.nargs) # type: ignore[assignment] return multi_rv - return rv + return raw_value def consume_value( self, ctx: Context, opts: cabc.Mapping[str, Parameter] ) -> tuple[t.Any, ParameterSource]: + """ + For :class:`Option`, the value can be collected from an interactive prompt if + the option is a flag that needs a value (and the :attr:`prompt` property is + set). + + Additionally, this method handles flag option that are activated without a + value, in which case the :attr:`flag_value` is returned. + """ value, source = super().consume_value(ctx, opts) - # The parser will emit a sentinel value if the option can be - # given as a flag without a value. This is different from None - # to distinguish from the flag not being given at all. + # The parser will emit a sentinel value if the option is allowed to as a flag + # without a value. if value is _flag_needs_value: + # If the option allows for a prompt, we start an interaction with the user. if self.prompt is not None and not ctx.resilient_parsing: value = self.prompt_for_value(ctx) source = ParameterSource.PROMPT + # Else the flag takes its flag_value as value. else: value = self.flag_value source = ParameterSource.COMMANDLINE - # A flag which is activated and has a flag_value set, should returns - # the latter, unless the value comes from the explicitly sets default. + # A flag which is activated always returns the flag value, unless the value + # comes from the explicitly sets default. elif ( self.is_flag and value is True and not self.is_bool_flag - and self.flag_value is not None - and source is not ParameterSource.DEFAULT + and source not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP) ): value = self.flag_value + # Re-interpret a multiple option which has been sent as-is by the parser. + # Here we replace each occurrence of value-less flags (marked by the + # _flag_needs_value sentinel) with the flag_value. elif ( self.multiple - and value is not None + and value is not UNSET + and source not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP) and any(v is _flag_needs_value for v in value) ): value = [self.flag_value if v is _flag_needs_value else v for v in value] source = ParameterSource.COMMANDLINE - # The value wasn't set, or used the param's default, prompt if - # prompting is enabled. + # The value wasn't set, or used the param's default, prompt for one to the user + # if prompting is enabled. elif ( - source in {None, ParameterSource.DEFAULT} + ( + value is UNSET + or source in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP) + ) and self.prompt is not None and (self.required or self.prompt_required) and not ctx.resilient_parsing @@ -3075,6 +3162,14 @@ def consume_value( return value, source + def type_cast_value(self, ctx: Context, value: t.Any) -> t.Any: + if self.is_flag and not self.required: + if value is UNSET: + if self.is_bool_flag: + # If the flag is a boolean flag, we return False if it is not set. + value = False + return super().type_cast_value(ctx, value) + class Argument(Parameter): """Arguments are positional parameters to a command. They generally @@ -3092,21 +3187,21 @@ def __init__( required: bool | None = None, **attrs: t.Any, ) -> None: + # Auto-detect the requirement status of the argument if not explicitly set. if required is None: - if attrs.get("default") is not None: - required = False - else: + # The argument gets automatically required if it has no explicit default + # value set and is setup to match at least one value. + if attrs.get("default", UNSET) is UNSET: required = attrs.get("nargs", 1) > 0 + # If the argument has a default value, it is not required. + else: + required = False if "multiple" in attrs: raise TypeError("__init__() got an unexpected keyword argument 'multiple'.") super().__init__(param_decls, required=required, **attrs) - if __debug__: - if self.default is not None and self.nargs == -1: - raise TypeError("'default' is not supported for nargs=-1.") - @property def human_readable_name(self) -> str: if self.metavar is not None: diff --git a/src/click/parser.py b/src/click/parser.py index a8b7d2634a..2c2159cc01 100644 --- a/src/click/parser.py +++ b/src/click/parser.py @@ -30,12 +30,14 @@ from gettext import gettext as _ from gettext import ngettext +from ._utils import UNSET from .exceptions import BadArgumentUsage from .exceptions import BadOptionUsage from .exceptions import NoSuchOption from .exceptions import UsageError if t.TYPE_CHECKING: + from ._utils import T_UNSET from .core import Argument as CoreArgument from .core import Context from .core import Option as CoreOption @@ -59,21 +61,21 @@ def _unpack_args( The nargs specification is the number of arguments that should be consumed or `-1` to indicate that this position should eat up all the remainders. - Missing items are filled with `None`. + Missing items are filled with ``UNSET``. """ args = deque(args) nargs_spec = deque(nargs_spec) - rv: list[str | tuple[str | None, ...] | None] = [] + rv: list[str | tuple[str | T_UNSET, ...] | T_UNSET] = [] spos: int | None = None - def _fetch(c: deque[V]) -> V | None: + def _fetch(c: deque[V]) -> V | T_UNSET: try: if spos is None: return c.popleft() else: return c.pop() except IndexError: - return None + return UNSET while nargs_spec: nargs = _fetch(nargs_spec) @@ -82,7 +84,7 @@ def _fetch(c: deque[V]) -> V | None: continue if nargs == 1: - rv.append(_fetch(args)) + rv.append(_fetch(args)) # type: ignore[arg-type] elif nargs > 1: x = [_fetch(args) for _ in range(nargs)] @@ -97,7 +99,7 @@ def _fetch(c: deque[V]) -> V | None: raise TypeError("Cannot have two nargs < 0") spos = len(rv) - rv.append(None) + rv.append(UNSET) # spos is the position of the wildcard (star). If it's not `None`, # we fill it with the remainder. @@ -187,14 +189,14 @@ def __init__(self, obj: CoreArgument, dest: str | None, nargs: int = 1): def process( self, - value: str | cabc.Sequence[str | None] | None, + value: str | cabc.Sequence[str | None] | None | T_UNSET, state: _ParsingState, ) -> None: if self.nargs > 1: - assert value is not None - holes = sum(1 for x in value if x is None) + assert isinstance(value, cabc.Sequence) + holes = sum(1 for x in value if x is UNSET) if holes == len(value): - value = None + value = UNSET elif holes != 0: raise BadArgumentUsage( _("Argument {name!r} takes {nargs} values.").format( @@ -202,10 +204,9 @@ def process( ) ) - if self.nargs == -1 and self.obj.envvar is not None and value == (): - # Replace empty tuple with None so that a value from the - # environment may be tried. - value = None + # We failed to collect any argument value so we consider the argument as unset. + if value == (): + value = UNSET state.opts[self.dest] = value # type: ignore state.order.append(self.obj) @@ -384,7 +385,7 @@ def _match_long_opt( ) else: - value = None + value = UNSET option.process(value, state) @@ -414,7 +415,7 @@ def _match_short_opt(self, arg: str, state: _ParsingState) -> None: value = self._get_value_from_state(opt, option, state) else: - value = None + value = UNSET option.process(value, state) diff --git a/src/click/termui.py b/src/click/termui.py index dcbb22216c..b0c7bfa6bf 100644 --- a/src/click/termui.py +++ b/src/click/termui.py @@ -220,6 +220,8 @@ def confirm( .. versionadded:: 4.0 Added the ``err`` parameter. """ + assert default in (True, False, None), "default must be True, False, or None" + prompt = _build_prompt( text, prompt_suffix, diff --git a/tests/test_arguments.py b/tests/test_arguments.py index d5789bc6a4..9663258bd9 100644 --- a/tests/test_arguments.py +++ b/tests/test_arguments.py @@ -4,6 +4,7 @@ import pytest import click +from click._utils import UNSET def test_nargs_star(runner): @@ -19,15 +20,6 @@ def copy(src, dst): assert result.output.splitlines() == ["src=foo.txt|bar.txt", "dst=dir"] -def test_argument_unbounded_nargs_cant_have_default(runner): - with pytest.raises(TypeError, match="nargs=-1"): - - @click.command() - @click.argument("src", nargs=-1, default=["42"]) - def copy(src): - pass - - def test_nargs_tup(runner): @click.command() @click.argument("name", nargs=1) @@ -174,6 +166,8 @@ def inout(output): result = runner.invoke(inout, []) assert not result.exception assert result.output == "Foo bar baz\n" + assert result.stdout == "Foo bar baz\n" + assert not result.stderr @pytest.mark.parametrize( @@ -251,13 +245,43 @@ def cmd(arg): assert "Missing argument 'ARG'." in result.output -def test_missing_argument_string_cast(): +@pytest.mark.parametrize( + ("value", "expect_missing", "processed_value"), + [ + # Unspecified type of the argument fallback to string, so everything is + # processed the click.STRING type. + ("", False, ""), + (" ", False, " "), + ("foo", False, "foo"), + ("12", False, "12"), + (12, False, "12"), + (12.1, False, "12.1"), + (list(), False, "[]"), + (tuple(), False, "()"), + (set(), False, "set()"), + (frozenset(), False, "frozenset()"), + (dict(), False, "{}"), + # None is a value that is allowed to be processed by a required argument + # because at this stage, the process_value method happens after the default is + # applied. + (None, False, None), + # An UNSET required argument will raise MissingParameter. + (UNSET, True, None), + ], +) +def test_required_argument(value, expect_missing, processed_value): + """Test how a required argument is processing the provided values.""" ctx = click.Context(click.Command("")) + argument = click.Argument(["a"], required=True) - with pytest.raises(click.MissingParameter) as excinfo: - click.Argument(["a"], required=True).process_value(ctx, None) + if expect_missing: + with pytest.raises(click.MissingParameter) as excinfo: + argument.process_value(ctx, value) + assert str(excinfo.value) == "Missing parameter: a" - assert str(excinfo.value) == "Missing parameter: a" + else: + value = argument.process_value(ctx, value) + assert value == processed_value def test_implicit_non_required(runner): @@ -351,22 +375,115 @@ def cmd(a, b, c): assert result.output.splitlines() == ["('a', 'b', 'c')", "d", "('e', 'f')"] -def test_defaults_for_nargs(runner): +@pytest.mark.parametrize( + ("argument_params", "args", "expected"), + [ + # Any iterable with the same number of arguments as nargs is valid. + [{"nargs": 2, "default": (1, 2)}, [], (1, 2)], + [{"nargs": 2, "default": (1.1, 2.2)}, [], (1, 2)], + [{"nargs": 2, "default": ("1", "2")}, [], (1, 2)], + [{"nargs": 2, "default": (None, None)}, [], (None, None)], + [{"nargs": 2, "default": [1, 2]}, [], (1, 2)], + [{"nargs": 2, "default": {1, 2}}, [], (1, 2)], + [{"nargs": 2, "default": frozenset([1, 2])}, [], (1, 2)], + [{"nargs": 2, "default": {1: "a", 2: "b"}}, [], (1, 2)], + # Empty iterable is valid if default is None. + [{"nargs": 2, "default": None}, [], None], + # Arguments overrides the default. + [{"nargs": 2, "default": (1, 2)}, ["3", "4"], (3, 4)], + # Unbounded arguments are allowed to have a default. + # See: https://github.com/pallets/click/issues/2164 + [{"nargs": -1, "default": [42]}, [], (42,)], + [{"nargs": -1, "default": None}, [], ()], + [{"nargs": -1, "default": {1, 2, 3, 4, 5}}, [], (1, 2, 3, 4, 5)], + ], +) +def test_good_defaults_for_nargs(runner, argument_params, args, expected): @click.command() - @click.argument("a", nargs=2, type=int, default=(1, 2)) + @click.argument("a", type=int, **argument_params) def cmd(a): - x, y = a - click.echo(x + y) + click.echo(repr(a), nl=False) - result = runner.invoke(cmd, []) - assert result.output.strip() == "3" + result = runner.invoke(cmd, args) + assert result.output == repr(expected) - result = runner.invoke(cmd, ["3", "4"]) - assert result.output.strip() == "7" - result = runner.invoke(cmd, ["3"]) - assert result.exception is not None - assert "Argument 'a' takes 2 values." in result.output +@pytest.mark.parametrize( + ("default", "message"), + [ + # Non-iterables defaults. + ["Yo", "Error: Invalid value for '[A]...': Value must be an iterable."], + ["", "Error: Invalid value for '[A]...': Value must be an iterable."], + [True, "Error: Invalid value for '[A]...': Value must be an iterable."], + [False, "Error: Invalid value for '[A]...': Value must be an iterable."], + [12, "Error: Invalid value for '[A]...': Value must be an iterable."], + [7.9, "Error: Invalid value for '[A]...': Value must be an iterable."], + # Generator default. + [(), "Error: Invalid value for '[A]...': Takes 2 values but 0 were given."], + # Unset default. + [UNSET, "Error: Missing argument 'A...'."], + # Tuples defaults with wrong length. + [ + tuple(), + "Error: Invalid value for '[A]...': Takes 2 values but 0 were given.", + ], + [(1,), "Error: Invalid value for '[A]...': Takes 2 values but 1 was given."], + [ + (1, 2, 3), + "Error: Invalid value for '[A]...': Takes 2 values but 3 were given.", + ], + # Lists defaults with wrong length. + [list(), "Error: Invalid value for '[A]...': Takes 2 values but 0 were given."], + [[1], "Error: Invalid value for '[A]...': Takes 2 values but 1 was given."], + [ + [1, 2, 3], + "Error: Invalid value for '[A]...': Takes 2 values but 3 were given.", + ], + # Sets defaults with wrong length. + [set(), "Error: Invalid value for '[A]...': Takes 2 values but 0 were given."], + [ + set([1]), + "Error: Invalid value for '[A]...': Takes 2 values but 1 was given.", + ], + [ + set([1, 2, 3]), + "Error: Invalid value for '[A]...': Takes 2 values but 3 were given.", + ], + # Frozensets defaults with wrong length. + [ + frozenset(), + "Error: Invalid value for '[A]...': Takes 2 values but 0 were given.", + ], + [ + frozenset([1]), + "Error: Invalid value for '[A]...': Takes 2 values but 1 was given.", + ], + [ + frozenset([1, 2, 3]), + "Error: Invalid value for '[A]...': Takes 2 values but 3 were given.", + ], + # Dictionaries defaults with wrong length. + [dict(), "Error: Invalid value for '[A]...': Takes 2 values but 0 were given."], + [ + {1: "a"}, + "Error: Invalid value for '[A]...': Takes 2 values but 1 was given.", + ], + [ + {1: "a", 2: "b", 3: "c"}, + "Error: Invalid value for '[A]...': Takes 2 values but 3 were given.", + ], + ], +) +def test_bad_defaults_for_nargs(runner, default, message): + """Some defaults are not valid when nargs is set.""" + + @click.command() + @click.argument("a", nargs=2, type=int, default=default) + def cmd(a): + click.echo(repr(a)) + + result = runner.invoke(cmd, []) + assert message in result.stderr def test_multiple_param_decls_not_allowed(runner): @@ -383,12 +500,6 @@ def test_multiple_not_allowed(): click.Argument(["a"], multiple=True) -@pytest.mark.parametrize("value", [(), ("a",), ("a", "b", "c")]) -def test_nargs_bad_default(runner, value): - with pytest.raises(ValueError, match="nargs=2"): - click.Argument(["a"], nargs=2, default=value) - - def test_subcommand_help(runner): @click.group() @click.argument("name") diff --git a/tests/test_basic.py b/tests/test_basic.py index d773c3714f..5e490d9d9b 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -7,6 +7,7 @@ import pytest import click +from click._utils import UNSET def test_basic_functionality(runner): @@ -205,19 +206,29 @@ def cli(f): assert result.exception is None -@pytest.mark.parametrize("default", [True, False]) @pytest.mark.parametrize( - ("args", "expect"), [(["--on"], True), (["--off"], False), ([], None)] + ("args", "default", "expect"), + [ + (["--on"], True, True), + (["--on"], False, True), + (["--on"], None, True), + (["--on"], UNSET, True), + (["--off"], True, False), + (["--off"], False, False), + (["--off"], None, False), + (["--off"], UNSET, False), + ([], True, True), + ([], False, False), + ([], None, None), + ([], UNSET, False), + ], ) -def test_boolean_switch(runner, default, args, expect): +def test_boolean_switch(runner, args, default, expect): @click.command() @click.option("--on/--off", default=default) def cli(on): return on - if expect is None: - expect = default - result = runner.invoke(cli, args, standalone_mode=False) assert result.return_value is expect @@ -229,6 +240,10 @@ def cli(on): (True, [], True), (False, ["--f"], True), (False, [], False), + # Boolean flags have a 3-states logic. + # See: https://github.com/pallets/click/issues/3024#issue-3285556668 + (None, ["--f"], True), + (None, [], None), ), ) def test_boolean_flag(runner, default, args, expect): @@ -261,6 +276,27 @@ def cli(flag): assert result.output == expect +@pytest.mark.parametrize( + ("default", "args", "expect"), + ( + # See: https://github.com/pallets/click/issues/3024#issuecomment-3146199461 + (True, ["--upper"], "upper"), + (True, ["--lower"], "lower"), + (True, [], "True"), + ("upper", [], "upper"), + ), +) +def test_flag_value(runner, default, args, expect): + @click.command() + @click.option("--upper", "transformation", flag_value="upper", default=default) + @click.option("--lower", "transformation", flag_value="lower") + def cli(transformation): + click.echo(repr(transformation), nl=False) + + result = runner.invoke(cli, args) + assert result.output == repr(expect) + + def test_file_option(runner): @click.command() @click.option("--file", type=click.File("w")) @@ -475,15 +511,25 @@ def test_choice_argument_none(runner): ) def cli(method: str | None): assert isinstance(method, str) or method is None - click.echo(method) + click.echo(repr(method), nl=False) result = runner.invoke(cli, ["not-none"]) assert not result.exception - assert result.output == "not-none\n" + assert result.output == repr("not-none") - # None is not yet supported. result = runner.invoke(cli, ["none"]) + assert not result.exception + assert result.output == repr(None) + + result = runner.invoke(cli, []) assert result.exception + assert ( + "Error: Missing argument '{not-none|none}'. " + "Choose from:\n\tnot-none,\n\tnone\n" in result.stderr + ) + + result = runner.invoke(cli, ["--help"]) + assert result.output.startswith("Usage: cli [OPTIONS] {not-none|none}\n") def test_datetime_option_default(runner): diff --git a/tests/test_options.py b/tests/test_options.py index ad2be1cb21..8747699f2f 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -10,6 +10,8 @@ import click from click import Option +from click import UNPROCESSED +from click._utils import UNSET def test_prefixes(runner): @@ -85,7 +87,7 @@ def test_deprecated_prompt(runner): def test_invalid_nargs(runner): - with pytest.raises(TypeError, match="nargs=-1"): + with pytest.raises(TypeError, match="nargs=-1 is not supported for options."): @click.command() @click.option("--foo", nargs=-1) @@ -170,38 +172,120 @@ def cli(message): @pytest.mark.parametrize( - ("multiple", "nargs", "default"), + ("args", "multiple", "nargs", "default", "expected"), [ - (True, 1, []), - (True, 1, [1]), - # (False, -1, []), - # (False, -1, [1]), - (False, 2, [1, 2]), - # (True, -1, [[]]), - # (True, -1, []), - # (True, -1, [[1]]), - (True, 2, []), - (True, 2, [[1, 2]]), + # If multiple values are allowed, defaults should be iterable. + ([], True, 1, [], ()), + ([], True, 1, (), ()), + ([], True, 1, tuple(), ()), + ([], True, 1, set(), ()), + ([], True, 1, frozenset(), ()), + ([], True, 1, {}, ()), + # Special values. + ([], True, 1, None, ()), + ([], True, 1, UNSET, ()), + # Number of values are kept as-is in the default. + ([], True, 1, [1], (1,)), + ([], True, 1, [1, 2], (1, 2)), + ([], True, 1, [1, 2, 3], (1, 2, 3)), + ([], True, 1, [1.1, 2.2], (1.1, 2.2)), + ([], True, 1, ["1", "2"], ("1", "2")), + ([], True, 1, [None, None], (None, None)), + # XXX Why items of the defaults are converted to strings and not left as + # integers? + ([], True, 1, {1, 2}, ("1", "2")), + ([], True, 1, frozenset([1, 2]), ("1", "2")), + ([], True, 1, {1: "a", 2: "b"}, ("1", "2")), + # Multiple values with nargs > 1. + ([], True, 2, [], ()), + ([], True, 2, (), ()), + ([], True, 2, tuple(), ()), + ([], True, 2, set(), ()), + ([], True, 2, frozenset(), ()), + ([], True, 2, {}, ()), + ([], True, 2, None, ()), + ([], True, 2, UNSET, ()), + ([], True, 2, [[1, 2]], ((1, 2),)), + ([], True, 2, [[1, 2], [3, 4]], ((1, 2), (3, 4))), + ([], True, 2, [[1, 2], [3, 4], [5, 6]], ((1, 2), (3, 4), (5, 6))), + ([], True, 2, [[1.1, 2.2], [3.3, 4.4]], ((1.1, 2.2), (3.3, 4.4))), + ([], True, 2, [["1", "2"], ["3", "4"]], (("1", "2"), ("3", "4"))), + ([], True, 2, [[None, None], [None, None]], ((None, None), (None, None))), + ([], True, 2, [[1, 2.2], ["3", None]], ((1, 2.2), (3, None))), + ([], True, 2, [[1, 2.2], None], ((1, 2.2), None)), + # + ([], False, 2, [1, 2], (1, 2)), + # ], ) -def test_init_good_default_list(runner, multiple, nargs, default): - click.Option(["-a"], multiple=multiple, nargs=nargs, default=default) +def test_good_defaults_for_multiple(runner, args, multiple, nargs, default, expected): + @click.command() + @click.option("-a", multiple=multiple, nargs=nargs, default=default) + def cmd(a): + click.echo(repr(a), nl=False) + + result = runner.invoke(cmd, args) + assert result.output == repr(expected) @pytest.mark.parametrize( - ("multiple", "nargs", "default"), + ("multiple", "nargs", "default", "message"), [ - (True, 1, 1), - # (False, -1, 1), - (False, 2, [1]), - (True, 2, [[1]]), + # Non-iterables defaults. + (True, 1, "Yo", "Error: Invalid value for '-a': Value must be an iterable."), + (True, 1, "", "Error: Invalid value for '-a': Value must be an iterable."), + ( + True, + 1, + True, + "Error: Invalid value for '-a': Value must be an iterable.", + ), + ( + True, + 1, + False, + "Error: Invalid value for '-a': Value must be an iterable.", + ), + (True, 1, 12, "Error: Invalid value for '-a': Value must be an iterable."), + (True, 1, 7.9, "Error: Invalid value for '-a': Value must be an iterable."), + # + (False, 2, 42, "Error: Invalid value for '-a': Value must be an iterable."), + ( + True, + 2, + ["test string which is not a list in the list"], + "Error: Invalid value for '-a': Value must be an iterable.", + ), + # Multiple options, each with 2 args, but with wrong length. + ( + True, + 2, + (1,), + "Error: Invalid value for '-a': Value must be an iterable.", + ), + ( + True, + 2, + (1, 2, 3), + "Error: Invalid value for '-a': Value must be an iterable.", + ), + # A mix of valid and invalid defaults. + ( + True, + 2, + [[1, 2.2], []], + "Error: Invalid value for '-a': 2 values are required, but 0 were given.", + ), ], ) -def test_init_bad_default_list(runner, multiple, nargs, default): - type = (str, str) if nargs == 2 else None +def test_bad_defaults_for_multiple(runner, multiple, nargs, default, message): + @click.command() + @click.option("-a", multiple=multiple, nargs=nargs, default=default) + def cmd(a): + click.echo(repr(a)) - with pytest.raises(ValueError, match="default"): - click.Option(["-a"], type=type, multiple=multiple, nargs=nargs, default=default) + result = runner.invoke(cmd, []) + assert message in result.stderr @pytest.mark.parametrize("env_key", ["MYPATH", "AUTO_MYPATH"]) @@ -253,7 +337,7 @@ def cmd(arg): @pytest.mark.parametrize( - ("name", "value", "expected"), + ("envvar_name", "envvar_value", "expected"), ( # Lower-cased variations of value. ("SHOUT", "true", True), @@ -270,17 +354,19 @@ def cmd(arg): ("SHOUT", "FaLsE", False), ("SHOUT", "falsE", False), # Extra spaces around the value. - ("SHOUT", "true ", True), - ("SHOUT", " true", True), - ("SHOUT", " true ", True), - ("SHOUT", "false ", False), - ("SHOUT", " false", False), - ("SHOUT", " false ", False), + ("SHOUT", "true ", True), + ("SHOUT", " true ", True), + ("SHOUT", " true", True), + ("SHOUT", "false ", False), + ("SHOUT", " false ", False), + ("SHOUT", " false", False), # Integer variations. ("SHOUT", "1", True), ("SHOUT", "0", False), # Alternative states. ("SHOUT", "t", True), + ("SHOUT", "T", True), + ("SHOUT", " T ", True), ("SHOUT", "f", False), ("SHOUT", "y", True), ("SHOUT", "n", False), @@ -295,47 +381,69 @@ def cmd(arg): ("SHOUT", " ", False), # Variable names are not stripped of spaces and so don't match the # flag, which then naturraly takes its default value. - ("SHOUT ", "True", False), - ("SHOUT ", "False", False), + ("SHOUT ", "True", False), + ("SHOUT ", "False", False), (" SHOUT ", "True", False), (" SHOUT ", "False", False), + (" SHOUT", "True", False), + (" SHOUT", "False", False), + ("SH OUT", "True", False), + ("SH OUT", "False", False), # Same for random and reverse environment variable names: they are not # recognized by the option. ("RANDOM", "True", False), ("NO_SHOUT", "True", False), ("NO_SHOUT", "False", False), + ("NOSHOUT", "True", False), + ("NOSHOUT", "False", False), ), ) -def test_boolean_envvar_normalization(runner, name, value, expected): +def test_boolean_flag_envvar(runner, envvar_name, envvar_value, expected): + assert isinstance(envvar_name, str) + assert isinstance(envvar_value, str) or envvar_value is None + @click.command() @click.option("--shout/--no-shout", envvar="SHOUT") def cli(shout): - click.echo(f"shout: {shout!r}") + click.echo(repr(shout), nl=False) - result = runner.invoke(cli, [], env={name: value}) + result = runner.invoke(cli, [], env={envvar_name: envvar_value}) assert result.exit_code == 0 - assert result.output == f"shout: {expected}\n" + assert result.output == repr(expected) @pytest.mark.parametrize( - ("name", "value"), + "value", ( - ("SHOUT", "tr ue"), - ("SHOUT", "10"), - ("SHOUT", "01"), - ("SHOUT", "00"), - ("SHOUT", "11"), - ("SHOUT", "randomstring"), - ("SHOUT", "None"), + # Extra spaces inside the value. + "tr ue", + "fa lse", + # Numbers. + "10", + "01", + "00", + "11", + "0.0", + "1.1", + # Random strings. + "randomstring", + "None", + "'None'", + "A B", + " 1 2 ", + "9.3", + "a;n", + "x:y", + "i/o", ), ) -def test_boolean_envvar_bad_values(runner, name, value): +def test_boolean_envvar_bad_values(runner, value): @click.command() @click.option("--shout/--no-shout", envvar="SHOUT") def cli(shout): - click.echo(f"shout: {shout!r}") + click.echo(shout) - result = runner.invoke(cli, [], env={name: value}) + result = runner.invoke(cli, [], env={"SHOUT": value}) assert result.exit_code == 2 assert ( f"Invalid value for '--shout': {value!r} is not a valid boolean." @@ -629,13 +737,37 @@ def cmd(whatever): assert result.output == "23\n" -def test_missing_option_string_cast(): +@pytest.mark.parametrize( + ("value", "expect_missing", "processed_value"), + [ + (UNSET, True, None), + (None, False, None), + # Default type of the argument is str, so everything is processed as strings. + ("", False, ""), + (" ", False, " "), + ("foo", False, "foo"), + ("12", False, "12"), + (12, False, "12"), + (12.1, False, "12.1"), + (list(), False, "[]"), + (tuple(), False, "()"), + (set(), False, "set()"), + (frozenset(), False, "frozenset()"), + (dict(), False, "{}"), + ], +) +def test_required_option(value, expect_missing, processed_value): ctx = click.Context(click.Command("")) + argument = click.Option(["-a"], required=True) - with pytest.raises(click.MissingParameter) as excinfo: - click.Option(["-a"], required=True).process_value(ctx, None) + if expect_missing: + with pytest.raises(click.MissingParameter) as excinfo: + argument.process_value(ctx, value) + assert str(excinfo.value) == "Missing parameter: a" - assert str(excinfo.value) == "Missing parameter: a" + else: + value = argument.process_value(ctx, value) + assert value == processed_value def test_missing_required_flag(runner): @@ -1100,188 +1232,293 @@ def test_type_from_flag_value(): @pytest.mark.parametrize( - ("opt_params", "pass_flag", "expected"), + ("opt_params", "args", "expected"), [ - # Effect of the presence/absence of flag depending on type - ({"type": bool}, False, False), - ({"type": bool}, True, True), - ({"type": click.BOOL}, False, False), - ({"type": click.BOOL}, True, True), - ({"type": str}, False, "False"), - ({"type": str}, True, "True"), - # Effects of default value - ({"type": bool, "default": True}, False, True), - ({"type": bool, "default": True}, True, True), - ({"type": bool, "default": False}, False, False), - ({"type": bool, "default": False}, True, True), - ({"type": bool, "default": None}, False, False), - ({"type": bool, "default": None}, True, True), - ({"type": click.BOOL, "default": True}, False, True), - ({"type": click.BOOL, "default": True}, True, True), - ({"type": click.BOOL, "default": False}, False, False), - ({"type": click.BOOL, "default": False}, True, True), - ({"type": click.BOOL, "default": None}, False, False), - ({"type": click.BOOL, "default": None}, True, True), - ({"type": str, "default": True}, False, "True"), - ({"type": str, "default": True}, True, "True"), - ({"type": str, "default": False}, False, "False"), - ({"type": str, "default": False}, True, "True"), - ({"type": str, "default": "foo"}, False, "foo"), - ({"type": str, "default": "foo"}, True, "foo"), - ({"type": str, "default": None}, False, "False"), - ({"type": str, "default": None}, True, "True"), - # Effects of flag_value - ({"type": bool, "flag_value": True}, False, False), - ({"type": bool, "flag_value": True}, True, True), - ({"type": bool, "flag_value": False}, False, False), - ({"type": bool, "flag_value": False}, True, False), - ({"type": bool, "flag_value": None}, False, False), - ({"type": bool, "flag_value": None}, True, True), - ({"type": click.BOOL, "flag_value": True}, False, False), - ({"type": click.BOOL, "flag_value": True}, True, True), - ({"type": click.BOOL, "flag_value": False}, False, False), - ({"type": click.BOOL, "flag_value": False}, True, False), - ({"type": click.BOOL, "flag_value": None}, False, False), - ({"type": click.BOOL, "flag_value": None}, True, True), - ({"type": str, "flag_value": True}, False, "False"), - ({"type": str, "flag_value": True}, True, "True"), - ({"type": str, "flag_value": False}, False, "False"), - ({"type": str, "flag_value": False}, True, "False"), - ({"type": str, "flag_value": "foo"}, False, "False"), - ({"type": str, "flag_value": "foo"}, True, "foo"), - ({"type": str, "flag_value": None}, False, "False"), - ({"type": str, "flag_value": None}, True, "True"), - # Not passing --foo returns the default value as-is - ({"type": bool, "default": True, "flag_value": True}, False, True), - ({"type": bool, "default": True, "flag_value": False}, False, True), - ({"type": bool, "default": False, "flag_value": True}, False, False), - ({"type": bool, "default": False, "flag_value": False}, False, False), - ({"type": bool, "default": None, "flag_value": True}, False, False), - ({"type": bool, "default": None, "flag_value": False}, False, False), - ({"type": str, "default": True, "flag_value": True}, False, "True"), - ({"type": str, "default": True, "flag_value": False}, False, "True"), - ({"type": str, "default": False, "flag_value": True}, False, "False"), - ({"type": str, "default": False, "flag_value": False}, False, "False"), - ({"type": str, "default": "foo", "flag_value": True}, False, "foo"), - ({"type": str, "default": "foo", "flag_value": False}, False, "foo"), - ({"type": str, "default": "foo", "flag_value": "bar"}, False, "foo"), - ({"type": str, "default": "foo", "flag_value": None}, False, "foo"), - ({"type": str, "default": None, "flag_value": True}, False, "False"), - ({"type": str, "default": None, "flag_value": False}, False, "False"), - # Passing --foo returns the explicitly set flag_value - ({"type": bool, "default": True, "flag_value": True}, True, True), - ({"type": bool, "default": True, "flag_value": False}, True, False), - ({"type": bool, "default": False, "flag_value": True}, True, True), - ({"type": bool, "default": False, "flag_value": False}, True, False), - ({"type": bool, "default": None, "flag_value": True}, True, True), - ({"type": bool, "default": None, "flag_value": False}, True, False), - ({"type": str, "default": True, "flag_value": True}, True, "True"), - ({"type": str, "default": True, "flag_value": False}, True, "False"), - ({"type": str, "default": False, "flag_value": True}, True, "True"), - ({"type": str, "default": False, "flag_value": False}, True, "False"), - ({"type": str, "default": "foo", "flag_value": True}, True, "True"), - ({"type": str, "default": "foo", "flag_value": False}, True, "False"), - ({"type": str, "default": "foo", "flag_value": "bar"}, True, "bar"), - ({"type": str, "default": "foo", "flag_value": None}, True, "foo"), - ({"type": str, "default": None, "flag_value": True}, True, "True"), - ({"type": str, "default": None, "flag_value": False}, True, "False"), + # The type passed to the option is responsible to converting the value, whether + # we pass the option flag or not. + ({"type": bool}, [], False), + ({"type": bool}, ["--foo"], True), + ({"type": click.BOOL}, [], False), + ({"type": click.BOOL}, ["--foo"], True), + ({"type": str}, [], None), + ({"type": str}, ["--foo"], "True"), + # Default value is given as-is to the --foo option when it is not passed, + # whatever the type. Now if --foo is passed, the value is always True, whatever + # the type. In both case the type of the option is responsible for the + # conversion of the value. + ({"type": bool, "default": True}, [], True), + ({"type": bool, "default": True}, ["--foo"], True), + ({"type": bool, "default": False}, [], False), + ({"type": bool, "default": False}, ["--foo"], True), + # ({"type": bool, "default": "foo"}, [], "foo"), + ({"type": bool, "default": "foo"}, ["--foo"], True), + ({"type": bool, "default": None}, [], None), + ({"type": bool, "default": None}, ["--foo"], True), + ({"type": click.BOOL, "default": True}, [], True), + ({"type": click.BOOL, "default": True}, ["--foo"], True), + ({"type": click.BOOL, "default": False}, [], False), + ({"type": click.BOOL, "default": False}, ["--foo"], True), + # ({"type": click.BOOL, "default": "foo"}, [], "foo"), + ({"type": click.BOOL, "default": "foo"}, ["--foo"], True), + ({"type": click.BOOL, "default": None}, [], None), + ({"type": click.BOOL, "default": None}, ["--foo"], True), + ({"type": str, "default": True}, [], "True"), + ({"type": str, "default": True}, ["--foo"], "True"), + ({"type": str, "default": False}, [], "False"), + ({"type": str, "default": False}, ["--foo"], "True"), + ({"type": str, "default": "foo"}, [], "foo"), + ({"type": str, "default": "foo"}, ["--foo"], "True"), + ({"type": str, "default": None}, [], None), + ({"type": str, "default": None}, ["--foo"], "True"), + # Flag value is given as-is to the --foo option when it is passed, then + # converted by the option type. + ({"type": bool, "flag_value": True}, [], False), + ({"type": bool, "flag_value": True}, ["--foo"], True), + ({"type": bool, "flag_value": False}, [], False), + ({"type": bool, "flag_value": False}, ["--foo"], False), + ({"type": bool, "flag_value": None}, [], False), + ({"type": bool, "flag_value": None}, ["--foo"], None), + ({"type": click.BOOL, "flag_value": True}, [], False), + ({"type": click.BOOL, "flag_value": True}, ["--foo"], True), + ({"type": click.BOOL, "flag_value": False}, [], False), + ({"type": click.BOOL, "flag_value": False}, ["--foo"], False), + ({"type": click.BOOL, "flag_value": None}, [], False), + ({"type": click.BOOL, "flag_value": None}, ["--foo"], None), + ({"type": str, "flag_value": True}, [], None), + ({"type": str, "flag_value": True}, ["--foo"], "True"), + ({"type": str, "flag_value": False}, [], None), + ({"type": str, "flag_value": False}, ["--foo"], "False"), + ({"type": str, "flag_value": "foo"}, [], None), + ({"type": str, "flag_value": "foo"}, ["--foo"], "foo"), + ({"type": str, "flag_value": None}, [], None), + ({"type": str, "flag_value": None}, ["--foo"], None), + # Not passing --foo returns the default value as-is, in its Python type, then + # converted by the option type. + ({"type": bool, "default": True, "flag_value": True}, [], True), + ({"type": bool, "default": True, "flag_value": False}, [], True), + ({"type": bool, "default": False, "flag_value": True}, [], False), + ({"type": bool, "default": False, "flag_value": False}, [], False), + ({"type": bool, "default": None, "flag_value": True}, [], None), + ({"type": bool, "default": None, "flag_value": False}, [], None), + ({"type": str, "default": True, "flag_value": True}, [], "True"), + ({"type": str, "default": True, "flag_value": False}, [], "True"), + ({"type": str, "default": False, "flag_value": True}, [], "False"), + ({"type": str, "default": False, "flag_value": False}, [], "False"), + ({"type": str, "default": "foo", "flag_value": True}, [], "foo"), + ({"type": str, "default": "foo", "flag_value": False}, [], "foo"), + ({"type": str, "default": "foo", "flag_value": "bar"}, [], "foo"), + ({"type": str, "default": "foo", "flag_value": None}, [], "foo"), + ({"type": str, "default": None, "flag_value": True}, [], None), + ({"type": str, "default": None, "flag_value": False}, [], None), + # Passing --foo returns the flag_value that was explicitly set by the user, + # with its Python type, but still converted by the option type. + ({"type": bool, "default": True, "flag_value": True}, ["--foo"], True), + ({"type": bool, "default": True, "flag_value": False}, ["--foo"], False), + ({"type": bool, "default": False, "flag_value": True}, ["--foo"], True), + ({"type": bool, "default": False, "flag_value": False}, ["--foo"], False), + ({"type": bool, "default": None, "flag_value": True}, ["--foo"], True), + ({"type": bool, "default": None, "flag_value": False}, ["--foo"], False), + ({"type": str, "default": True, "flag_value": True}, ["--foo"], "True"), + ({"type": str, "default": True, "flag_value": False}, ["--foo"], "False"), + ({"type": str, "default": False, "flag_value": True}, ["--foo"], "True"), + ({"type": str, "default": False, "flag_value": False}, ["--foo"], "False"), + ({"type": str, "default": "foo", "flag_value": True}, ["--foo"], "True"), + ({"type": str, "default": "foo", "flag_value": False}, ["--foo"], "False"), + ({"type": str, "default": "foo", "flag_value": "bar"}, ["--foo"], "bar"), + ({"type": str, "default": "foo", "flag_value": None}, ["--foo"], None), + ({"type": str, "default": None, "flag_value": True}, ["--foo"], "True"), + ({"type": str, "default": None, "flag_value": False}, ["--foo"], "False"), ], ) -def test_flag_value_and_default(runner, opt_params, pass_flag, expected): +def test_flag_value_and_default(runner, opt_params, args, expected): @click.command() @click.option("--foo", is_flag=True, **opt_params) def cmd(foo): - click.echo(repr(foo)) + click.echo(repr(foo), nl=False) - result = runner.invoke(cmd, ["--foo"] if pass_flag else []) - assert result.output == f"{expected!r}\n" + result = runner.invoke(cmd, args) + assert result.output == repr(expected) @pytest.mark.parametrize( - "opts", + ("args", "opts"), [ - {"type": bool, "default": "foo"}, - {"type": bool, "flag_value": "foo"}, - {"type": click.BOOL, "default": "foo"}, - {"type": click.BOOL, "flag_value": "foo"}, + ([], {"type": bool, "default": "foo"}), + ([], {"type": click.BOOL, "default": "foo"}), + (["--foo"], {"type": bool, "flag_value": "foo"}), + (["--foo"], {"type": click.BOOL, "flag_value": "foo"}), ], ) -def test_invalid_flag_definition(runner, opts): +def test_invalid_flag_definition(runner, args, opts): @click.command() @click.option("--foo", is_flag=True, **opts) def cmd(foo): click.echo(foo) - result = runner.invoke(cmd, ["--foo"]) + result = runner.invoke(cmd, args) assert ( "Error: Invalid value for '--foo': 'foo' is not a valid boolean" in result.output ) +@pytest.mark.parametrize( + ("default", "args", "expected"), + [ + (None, [], "A real None"), + (UNSET, [], "A real UNSET"), + ("random", [], "random"), + (None, ["--js"], "js"), + (None, ["--xml"], "xml"), + ], +) +def test_flag_value_callback(runner, default, args, expected): + """ + Reproduction of the issue reported in + https://github.com/pallets/click/pull/3030#discussion_r2271571819 + """ + + def _my_func(ctx, param, value): + if value is None: + return "A real None" + if value is UNSET: + return "A real UNSET" + return value + + @click.command() + @click.option("--js", "fmt", flag_value="js", callback=_my_func, default=default) + @click.option("--xml", "fmt", flag_value="xml", callback=_my_func) + def main(fmt): + click.secho(repr(fmt), nl=False) + + result = runner.invoke(main, args) + assert result.output == repr(expected) + assert result.exit_code == 0 + + @pytest.mark.parametrize( ("flag_value", "envvar_value", "expected"), [ - # Envvar is set to false, so the flag default value is returned. - ("bar", "False", "False"), - # Same as above with alternative states and blank values. - ("bar", "0", "False"), - ("bar", "f", "False"), - ("bar", "", "False"), - # Envvar is set to true, so the flag_value is returned. - ("bar", "True", "bar"), - ("bar", "1", "bar"), - ("bar", "t", "bar"), - # So far we have the same cases as the test_flag_value_and_default - # test case. Now instead of passing a boolean-like value, let's use - # the flag_value as the envvar value. + # The envvar match exactly the flag value and is case-sensitive. ("bar", "bar", "bar"), - # flag_value is expected to be the exact same in the envvar for the flag to be - # activated. - ("bar", " bar ", "False"), - ("bar", "BAR", "False"), - ("bar", "random", "False"), - ("bar", "bar random", "False"), - ("bar", "random bar", "False"), ("BAR", "BAR", "BAR"), - ("BAR", "bar", "False"), (" bar ", " bar ", " bar "), - (" bar ", "bar", "False"), - (" bar ", "BAR", "False"), + ("42", "42", "42"), + ("None", "None", "None"), + ("True", "True", "True"), + ("False", "False", "False"), + ("true", "true", "true"), + ("false", "false", "false"), + ("A B", "A B", "A B"), + (" 1 2 ", " 1 2 ", " 1 2 "), + ("9.3", "9.3", "9.3"), + ("a;n", "a;n", "a;n"), + ("x:y", "x:y", "x:y"), + ("i/o", "i/o", "i/o"), + # Empty or absent envvar is consider unset, so the flag default value is + # returned, which is None in this case. + ("bar", "", None), + ("bar", None, None), + ("BAR", "", None), + ("BAR", None, None), + (" bar ", "", None), + (" bar ", None, None), + (42, "", None), + (42, None, None), + ("42", "", None), + ("42", None, None), + (None, "", None), + (None, None, None), + ("None", "", None), + ("None", None, None), + (True, "", None), + (True, None, None), + ("True", "", None), + ("True", None, None), + ("true", "", None), + ("true", None, None), + (False, "", None), + (False, None, None), + ("False", "", None), + ("False", None, None), + ("false", "", None), + ("false", None, None), + # Activate the flag with a value recognized as True by the envvar, which + # returns the flag_value. + ("bar", "True", "bar"), + ("bar", "true", "bar"), + ("bar", "trUe", "bar"), + ("bar", " TRUE ", "bar"), + ("bar", "1", "bar"), + ("bar", "yes", "bar"), + ("bar", "on", "bar"), + ("bar", "t", "bar"), + ("bar", "y", "bar"), + # Deactivating the flag with a value recognized as False by the envvar, which + # explicitly return the 'False' as the option is explicitely declared as a + # string. + ("bar", "False", "False"), + ("bar", "false", "False"), + ("bar", "faLse", "False"), + ("bar", " FALSE ", "False"), + ("bar", "0", "False"), + ("bar", "no", "False"), + ("bar", "off", "False"), + ("bar", "f", "False"), + ("bar", "n", "False"), + # Any other value than the flag_value, or a recogned True or False value, fails + # to explicitely activate or deactivate the flag. So the flag default value is + # returned, which is None in this case. + ("bar", " bar ", None), + ("bar", "BAR", None), + ("bar", "random", None), + ("bar", "bar random", None), + ("bar", "random bar", None), + ("BAR", "bar", None), + (" bar ", "bar", None), + (" bar ", "BAR", None), + (42, "42", None), + (42, "foo", None), + (None, "foo", None), + ("None", "foo", None), ], ) -def test_envvar_flag_value(runner, flag_value, envvar_value, expected): +def test_envvar_string_flag_value(runner, flag_value, envvar_value, expected): """Ensure that flag_value is recognized by the envvar.""" @click.command() - @click.option( - "--upper", type=str, is_flag=True, flag_value=flag_value, envvar="UPPER" - ) + @click.option("--upper", type=str, flag_value=flag_value, envvar="UPPER") def cmd(upper): - click.echo(repr(upper)) + click.echo(repr(upper), nl=False) result = runner.invoke(cmd, env={"UPPER": envvar_value}) assert result.exit_code == 0 - assert result.output == f"{expected!r}\n" + assert result.output == repr(expected) @pytest.mark.parametrize( - ("option", "expected"), + ("opt_decls", "opt_params", "expect_is_flag", "expect_is_bool_flag"), [ - # Not boolean flags - pytest.param(Option(["-a"], type=int), False, id="int option"), - pytest.param(Option(["-a"], type=bool), False, id="bool non-flag [None]"), - pytest.param(Option(["-a"], default=True), False, id="bool non-flag [True]"), - pytest.param(Option(["-a"], default=False), False, id="bool non-flag [False]"), - pytest.param(Option(["-a"], flag_value=1), False, id="non-bool flag_value"), - # Boolean flags - pytest.param(Option(["-a"], is_flag=True), True, id="is_flag=True"), - pytest.param(Option(["-a/-A"]), True, id="secondary option [implicit flag]"), - pytest.param(Option(["-a"], flag_value=True), True, id="bool flag_value"), + # Not boolean flags. + ("-a", {"type": int}, False, False), + ("-a", {"type": bool}, False, False), + ("-a", {"default": True}, False, False), + ("-a", {"default": False}, False, False), + ("-a", {"flag_value": 1}, True, False), + # Boolean flags. + ("-a", {"is_flag": True}, True, True), + ("-a/-A", {}, True, True), + ("-a", {"flag_value": True}, True, True), + # Non-flag with flag_value. + ("-a", {"is_flag": False, "flag_value": 1}, False, False), ], ) -def test_bool_flag_auto_detection(option, expected): - assert option.is_bool_flag is expected +def test_flag_auto_detection( + opt_decls, opt_params, expect_is_flag, expect_is_bool_flag +): + option = Option([opt_decls], **opt_params) + assert option.is_flag is expect_is_flag + assert option.is_bool_flag is expect_is_bool_flag @pytest.mark.parametrize( @@ -1365,11 +1602,7 @@ def cli_with_choices(g): pass @click.command() - @click.option( - "-g", - type=click.Choice(choices), - show_choices=False, - ) + @click.option("-g", type=click.Choice(choices), show_choices=False) def cli_without_choices(g): pass @@ -1455,31 +1688,252 @@ def cli(one, two): runner.invoke(cli, []) +def test_custom_type_click_class_flag_value(runner): + """A reproduction of + https://github.com/pallets/click/issues/3024#issuecomment-3146511356 + """ + + NO_CONFIG = object() + + class ConfigParamType(click.ParamType): + name = "config" + + def convert(self, value, param, ctx): + if value is NO_CONFIG: + return value + else: + return click.Path(exists=True, dir_okay=False).convert( + value, param, ctx + ) + + @click.command() + @click.option("-c", "--config", type=ConfigParamType()) + @click.option("--no-config", "config", flag_value=NO_CONFIG, type=ConfigParamType()) + def main(config): + click.echo(repr(config), nl=False) + + result = runner.invoke(main) + assert result.output == repr(None) + + +class EngineType(enum.Enum): + OSS = enum.auto() + PRO = enum.auto() + MAX = enum.auto() + + +class Class1: + pass + + +class Class2: + pass + + @pytest.mark.parametrize( - ("multiple", "nargs", "default", "expected_message"), + ("opt_params", "args", "expected"), [ + # Reproduction of cases from + # https://github.com/pallets/click/issues/3024#issuecomment-3146480714 + ({"type": EngineType, "flag_value": None}, [], None), + ({"type": EngineType, "flag_value": None}, ["--pro"], None), + ({"type": EngineType, "flag_value": EngineType.MAX}, [], None), + ( + {"type": EngineType, "flag_value": EngineType.MAX}, + ["--pro"], + EngineType.MAX, + ), + ({"type": EngineType, "default": EngineType.OSS}, [], EngineType.OSS), + ( + {"type": EngineType, "default": EngineType.OSS}, + ["--pro"], + re.compile(re.escape("Error: Option '--pro' requires an argument.\n")), + ), + ( + {"type": EngineType, "is_flag": True, "default": EngineType.OSS}, + [], + EngineType.OSS, + ), + ( + {"type": EngineType, "is_flag": True, "default": EngineType.OSS}, + ["--pro"], + EngineType.OSS, + ), + # Reproduction of cases from + # https://github.com/pallets/click/issues/2012#issuecomment-892437060 + ( + {"flag_value": EngineType.OSS, "default": True}, + [], + "True", + ), + ( + {"type": EngineType, "flag_value": EngineType.OSS, "default": True}, + [], + EngineType.OSS, + ), + ({"flag_value": 1, "default": True}, [], 1), + ({"flag_value": "1", "default": True}, [], "True"), + ({"flag_value": 1, "type": str, "default": True}, [], "True"), + ({"flag_value": "1", "type": str, "default": True}, [], "True"), + ({"flag_value": 1, "type": int, "default": True}, [], 1), + ({"flag_value": "1", "type": int, "default": True}, [], 1), + ], +) +def test_custom_type_flag_value_standalone_option(runner, opt_params, args, expected): + @click.command() + @click.option("--pro", **opt_params) + def scan(pro): + click.echo(repr(pro), nl=False) + + result = runner.invoke(scan, args) + if isinstance(expected, re.Pattern): + assert re.match(expected, result.output) + else: + assert result.output == repr(expected) + + +@pytest.mark.parametrize( + ("opt1_params", "opt2_params", "args", "expected"), + [ + ### + ### Reproduces https://github.com/pallets/click/issues/3024#issuecomment-3146508536 + ### + ( + {"flag_value": EngineType.OSS, "type": UNPROCESSED}, + {"flag_value": EngineType.PRO, "type": UNPROCESSED}, + [], + None, + ), + ( + {"flag_value": EngineType.OSS, "type": UNPROCESSED}, + {"flag_value": EngineType.PRO, "type": UNPROCESSED}, + ["--opt1"], + EngineType.OSS, + ), + ( + {"flag_value": EngineType.OSS, "type": UNPROCESSED}, + {"flag_value": EngineType.PRO, "type": UNPROCESSED}, + ["--opt2"], + EngineType.PRO, + ), + ### + ### Reproduces https://github.com/pallets/click/issues/2012#issue-946471049 + ### + # Default type fallback to string. + ({"flag_value": Class1, "default": True}, {"flag_value": Class2}, [], "True"), ( - False, # multiple - 2, # nargs - 42, # default (not a list) - "'default' must be a list when 'nargs' != 1.", + {"flag_value": Class1, "default": True}, + {"flag_value": Class2}, + ["--opt1"], + "", ), ( - True, # multiple - 2, # nargs - [ - "test string which is not a list in the list" - ], # default (list of non-lists) - "'default' must be a list of lists when 'multiple' is true" - " and 'nargs' != 1.", + {"flag_value": Class1, "default": True}, + {"flag_value": Class2}, + ["--opt2"], + "", + ), + # Even the default is processed as a string. + ({"flag_value": Class1, "default": "True"}, {"flag_value": Class2}, [], "True"), + ({"flag_value": Class1, "default": None}, {"flag_value": Class2}, [], None), + # To get the classes as-is, we need to specify the type as UNPROCESSED. + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": True}, + {"flag_value": Class2, "type": UNPROCESSED}, + [], + True, + ), + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": True}, + {"flag_value": Class2, "type": UNPROCESSED}, + ["--opt1"], + Class1, + ), + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": True}, + {"flag_value": Class2, "type": UNPROCESSED}, + ["--opt2"], + Class2, + ), + # Setting the default to a class, an instance of the class is returned instead + # of the class itself, because the default is allowed to be callable (and + # consummd). And this what happens the type is. + ( + {"flag_value": Class1, "default": Class1}, + {"flag_value": Class2}, + [], + re.compile(r"''"), + ), + ( + {"flag_value": Class1, "default": Class2}, + {"flag_value": Class2}, + [], + re.compile(r"''"), + ), + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": Class1}, + {"flag_value": Class2, "type": UNPROCESSED}, + [], + re.compile(r""), + ), + ( + {"flag_value": Class1, "type": UNPROCESSED, "default": Class2}, + {"flag_value": Class2, "type": UNPROCESSED}, + [], + re.compile(r""), + ), + ### + ### Reproduces https://github.com/pallets/click/issues/2012#issuecomment-892437060 + ### + ( + {"flag_value": 1, "default": True}, + {"flag_value": "1"}, + [], + 1, + ), + ( + {"flag_value": 1, "type": int, "default": True}, + {"flag_value": "1", "type": int}, + [], + 1, ), ], ) -def test_default_type_error_raises(multiple, nargs, default, expected_message): - with pytest.raises(ValueError, match=expected_message): - click.Option( - ["--test"], - multiple=multiple, - nargs=nargs, - default=default, - ) +def test_custom_type_flag_value_dual_options( + runner, opt1_params, opt2_params, args, expected +): + @click.command() + @click.option("--opt1", "dual_option", **opt1_params) + @click.option("--opt2", "dual_option", **opt2_params) + def cli(dual_option): + click.echo(repr(dual_option), nl=False) + + result = runner.invoke(cli, args) + if isinstance(expected, re.Pattern): + assert re.match(expected, result.output) + else: + assert result.output == repr(expected) + + +def test_custom_type_frozenset_flag_value(runner): + """A reproduction of https://github.com/pallets/click/issues/2610""" + + @click.command() + @click.option( + "--without-scm-ignore-files", + "scm_ignore_files", + is_flag=True, + type=frozenset, + flag_value=frozenset(), + default=frozenset(["git"]), + ) + def rcli(scm_ignore_files): + click.echo(repr(scm_ignore_files), nl=False) + + result = runner.invoke(rcli) + assert result.stdout == "frozenset({'git'})" + assert result.exit_code == 0 + + result = runner.invoke(rcli, ["--without-scm-ignore-files"]) + assert result.stdout == "frozenset()" + assert result.exit_code == 0 diff --git a/tests/test_termui.py b/tests/test_termui.py index 5d8b9fb803..af63689f10 100644 --- a/tests/test_termui.py +++ b/tests/test_termui.py @@ -6,6 +6,8 @@ import click._termui_impl from click._compat import WIN +from click.exceptions import BadParameter +from click.exceptions import MissingParameter class FakeClock: @@ -485,3 +487,196 @@ def cmd(arg1): # is False result = runner.invoke(cmd, input="my-input", standalone_mode=False) assert "my-default-value" not in result.output + + +BOOLEAN_FLAG_PROMPT_CASES = [ + ### + ### Test cases with prompt=True explicitly enabled for the flag. + ### + # Prompt is allowed and the flag has no default, so it prompts. + ({"prompt": True}, [], "[y/N]", "y", True), + ({"prompt": True}, [], "[y/N]", "n", False), + # Empty input default to False. + ({"prompt": True}, [], "[y/N]", "", False), + # Changing the default to True, makes the prompt change to [Y/n]. + ({"prompt": True, "default": True}, [], "[Y/n]", "", True), + ({"prompt": True, "default": True}, [], "[Y/n]", "y", True), + ({"prompt": True, "default": True}, [], "[Y/n]", "n", False), + # False is the default's default, so it prompts with [y/N]. + ({"prompt": True, "default": False}, [], "[y/N]", "", False), + ({"prompt": True, "default": False}, [], "[y/N]", "y", True), + ({"prompt": True, "default": False}, [], "[y/N]", "n", False), + # Defaulting to None, prompts with [y/n], which makes the user explicitly choose + # between True or False. + ({"prompt": True, "default": None}, [], "[y/n]", "y", True), + ({"prompt": True, "default": None}, [], "[y/n]", "n", False), + # Random string default is treated as a truthy value, so it prompts with [Y/n]. + ({"prompt": True, "default": "foo"}, [], "[Y/n]", "", True), + ({"prompt": True, "default": "foo"}, [], "[Y/n]", "y", True), + ({"prompt": True, "default": "foo"}, [], "[Y/n]", "n", False), + ### + ### Test cases with required=True explicitly enabled for the flag. + ### + # A required flag just raises an error unless a default is set. + ({"required": True}, [], None, None, MissingParameter), + ({"required": True, "default": True}, [], None, None, True), + ({"required": True, "default": False}, [], None, None, False), + ({"required": True, "default": None}, [], None, None, None), + ({"required": True, "default": "on"}, [], None, None, True), + ({"required": True, "default": "off"}, [], None, None, False), + ({"required": True, "default": "foo"}, [], None, None, BadParameter), + ### + ### Explicitly passing the flag to the CLI bypass any prompt, whatever the + ### configuration of the flag. + ### + # Flag allowing a prompt. + ({"prompt": True}, ["--flag"], None, None, True), + ({"prompt": True}, ["--no-flag"], None, None, False), + ({"prompt": True, "default": None}, ["--flag"], None, None, True), + ({"prompt": True, "default": None}, ["--no-flag"], None, None, False), + ({"prompt": True, "default": True}, ["--flag"], None, None, True), + ({"prompt": True, "default": True}, ["--no-flag"], None, None, False), + ({"prompt": True, "default": False}, ["--flag"], None, None, True), + ({"prompt": True, "default": False}, ["--no-flag"], None, None, False), + ({"prompt": True, "default": "foo"}, ["--flag"], None, None, True), + ({"prompt": True, "default": "foo"}, ["--no-flag"], None, None, False), + # Required flag. + ({"required": True}, ["--flag"], None, None, True), + ({"required": True}, ["--no-flag"], None, None, False), + ({"required": True, "default": None}, ["--flag"], None, None, True), + ({"required": True, "default": None}, ["--no-flag"], None, None, False), + ({"required": True, "default": True}, ["--flag"], None, None, True), + ({"required": True, "default": True}, ["--no-flag"], None, None, False), + ({"required": True, "default": False}, ["--flag"], None, None, True), + ({"required": True, "default": False}, ["--no-flag"], None, None, False), + ({"required": True, "default": "foo"}, ["--flag"], None, None, True), + ({"required": True, "default": "foo"}, ["--no-flag"], None, None, False), +] + +FLAG_VALUE_PROMPT_CASES = [ + ### + ### Test cases with prompt=True explicitly enabled for the flag. + ### + # Prompt is allowed and the flag has no default, so it prompts. + # But the flag_value is not set, so it defaults to a string. + # XXX ({"prompt": True}, [], "", "", ""), + ({"prompt": True}, [], "", "y", "y"), + ({"prompt": True}, [], "", "n", "n"), + ({"prompt": True}, [], "", "foo", "foo"), + # This time we provide a boolean flag_value, which makes the flag behave like a + # boolean flag, and use the appropriate variation of [y/n]. + ({"prompt": True, "flag_value": True}, [], "[y/N]", "", False), + ({"prompt": True, "flag_value": True}, [], "[y/N]", "y", True), + ({"prompt": True, "flag_value": True}, [], "[y/N]", "n", False), + ({"prompt": True, "flag_value": False}, [], "[y/N]", "", False), + ({"prompt": True, "flag_value": False}, [], "[y/N]", "y", True), + ({"prompt": True, "flag_value": False}, [], "[y/N]", "n", False), + # Other flag values changes the auto-detection of the flag type. + # XXX ({"prompt": True, "flag_value": None}, [], "", "", ""), + ({"prompt": True, "flag_value": None}, [], "", "y", "y"), + ({"prompt": True, "flag_value": None}, [], "", "n", "n"), + # XXX ({"prompt": True, "flag_value": "foo"}, [], "", "", ""), + ({"prompt": True, "flag_value": "foo"}, [], "", "y", "y"), + ({"prompt": True, "flag_value": "foo"}, [], "", "n", "n"), + ### + ### Test cases with a flag_value and a default. + ### + # default=True + ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "", True), + ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "y", True), + ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "n", False), + ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "", True), + ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "y", True), + ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "n", False), + # default=False + ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "", False), + ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "y", True), + ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "n", False), + ({"prompt": True, "default": False, "flag_value": False}, [], "[y/N]", "", False), + ({"prompt": True, "default": False, "flag_value": False}, [], "[y/N]", "y", True), + ({"prompt": True, "default": False, "flag_value": False}, [], "[y/N]", "n", False), + # default=None + # XXX + # ( + # {"prompt": True, "default": None, "flag_value": True}, + # [], + # "[y/n]", + # "", + # False, + # ), + ({"prompt": True, "default": None, "flag_value": True}, [], "[y/n]", "y", True), + ({"prompt": True, "default": None, "flag_value": True}, [], "[y/n]", "n", False), + # XXX + # ( + # {"prompt": True, "default": None, "flag_value": False}, + # [], + # "[y/n]", + # "", + # False, + # ), + ({"prompt": True, "default": None, "flag_value": False}, [], "[y/n]", "y", True), + ({"prompt": True, "default": None, "flag_value": False}, [], "[y/n]", "n", False), + # If the flag_value is None, the flag behave like a string flag, whatever the + # default is. + ({"prompt": True, "default": True, "flag_value": None}, [], "[True]", "", "True"), + ({"prompt": True, "default": True, "flag_value": None}, [], "[True]", "y", "y"), + ({"prompt": True, "default": True, "flag_value": None}, [], "[True]", "n", "n"), + ( + {"prompt": True, "default": False, "flag_value": None}, + [], + "[False]", + "", + "False", + ), + ({"prompt": True, "default": False, "flag_value": None}, [], "[False]", "y", "y"), + ({"prompt": True, "default": False, "flag_value": None}, [], "[False]", "n", "n"), + # XXX ({"prompt": True, "default": None, "flag_value": None}, [], "", "", "False"), + ({"prompt": True, "default": None, "flag_value": None}, [], "", "y", "y"), + ({"prompt": True, "default": None, "flag_value": None}, [], "", "n", "n"), +] + + +@pytest.mark.parametrize( + ("opt_decls", "opt_params", "args", "prompt", "input", "expected"), + # Boolean flag prompt cases. + [("--flag/--no-flag", *case_params) for case_params in BOOLEAN_FLAG_PROMPT_CASES] + # Non-boolean flag prompt cases. + + [("--flag", *case_params) for case_params in FLAG_VALUE_PROMPT_CASES], +) +def test_flag_value_prompt( + runner, opt_decls, opt_params, args, prompt, input, expected +): + """Covers concerns raised in issue https://github.com/pallets/click/issues/1992.""" + + @click.command() + @click.option(opt_decls, **opt_params) + def cli(flag): + click.echo(repr(flag)) + + invoke_options = {"standalone_mode": False} + if input is not None: + assert isinstance(input, str) + invoke_options["input"] = f"{input}\n" + + result = runner.invoke(cli, args, **invoke_options) + + if expected in (MissingParameter, BadParameter): + assert isinstance(result.exception, expected) + assert not result.output + assert result.exit_code == 1 + + else: + expected_output = "" + if prompt is not None: + assert isinstance(prompt, str) + expected_output += "Flag" + if prompt: + expected_output += f" {prompt}" + expected_output += ": " + assert isinstance(input, str) + expected_output += f"{input}\n" + expected_output += f"{expected!r}\n" + + assert result.output == expected_output + assert not result.stderr + assert result.exit_code == 0 diff --git a/tests/test_utils.py b/tests/test_utils.py index e941415b6d..2e2a614791 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -5,6 +5,8 @@ import sys from collections import namedtuple from contextlib import nullcontext +from decimal import Decimal +from fractions import Fraction from functools import partial from io import StringIO from pathlib import Path @@ -16,6 +18,56 @@ import click._termui_impl import click.utils from click._compat import WIN +from click._utils import UNSET + + +def test_unset_sentinel(): + value = UNSET + + assert value + assert value is UNSET + assert value == UNSET + assert repr(value) == "Sentinel.UNSET" + assert str(value) == "Sentinel.UNSET" + assert bool(value) is True + assert value is not True + + # Try all native Python values that can be falsy or truthy. + # See: https://docs.python.org/3/library/stdtypes.html#truth-value-testing + real_values = ( + None, + True, + False, + 0, + 1, + 0.0, + 1.0, + 0j, + 1j, + Decimal(0), + Decimal(1), + Fraction(0, 1), + Fraction(1, 1), + "", + "a", + "UNSET", + "Sentinel.UNSET", + [1], + (1), + {1: "a"}, + set(), + set([1]), + frozenset(), + frozenset([1]), + range(0), + range(1), + ) + + for real_value in real_values: + assert value != real_value + assert value is not real_value + + assert value not in real_values def test_echo(runner): From 8645e70266479a4c3abd9b821eca608971a8c051 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 14:55:36 +0400 Subject: [PATCH 02/37] Do not simple-tick Python literal values Refs: https://github.com/pallets/click/pull/3030#discussion_r2288948990 --- src/click/core.py | 66 +++++++++++++++++++++++------------------------ 1 file changed, 33 insertions(+), 33 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index 52c19cde6d..ab49079c65 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -187,7 +187,7 @@ class Context: the name of the script. :param obj: an arbitrary object of user data. :param auto_envvar_prefix: the prefix to use for automatic environment - variables. If this is `None` then reading + variables. If this is ``None`` then reading from environment variables is disabled. This does not affect manually set environment variables which are always read. @@ -210,11 +210,11 @@ class Context: invocation. Default values will also be ignored. This is useful for implementing things such as completion support. - :param allow_extra_args: if this is set to `True` then extra arguments + :param allow_extra_args: if this is set to ``True`` then extra arguments at the end will not raise an error and will be kept on the context. The default is to inherit from the command. - :param allow_interspersed_args: if this is set to `False` then options + :param allow_interspersed_args: if this is set to ``False`` then options and arguments cannot be mixed. The default is to inherit from the command. :param ignore_unknown_options: instructs click to ignore options it does @@ -289,7 +289,7 @@ def __init__( color: bool | None = None, show_default: bool | None = None, ) -> None: - #: the parent context or `None` if none exists. + #: the parent context or ``None`` if none exists. self.parent = parent #: the :class:`Command` for this context. self.command = command @@ -494,7 +494,7 @@ def scope(self, cleanup: bool = True) -> cabc.Iterator[Context]: """This helper method can be used with the context object to promote it to the current thread local (see :func:`get_current_context`). The default behavior of this is to invoke the cleanup functions which - can be disabled by setting `cleanup` to `False`. The cleanup + can be disabled by setting `cleanup` to ``False``. The cleanup functions are typically used for things such as closing file handles. If the cleanup is intended the context object can also be directly @@ -835,7 +835,7 @@ def get_parameter_source(self, name: str) -> ParameterSource | None: :rtype: ParameterSource .. versionchanged:: 8.0 - Returns `None` if the parameter was not provided from any + Returns ``None`` if the parameter was not provided from any source. """ return self._parameter_source.get(name) @@ -864,7 +864,7 @@ class Command: If enabled this will add ``--help`` as argument if no arguments are passed :param hidden: hide this command from help outputs. - :param deprecated: If `True` or non-empty string, issues a message + :param deprecated: If ``True`` or non-empty string, issues a message indicating that the command is deprecated and highlights its deprecation in --help. The message can be customized by using a string as the value. @@ -931,7 +931,7 @@ def __init__( self.context_settings: cabc.MutableMapping[str, t.Any] = context_settings #: the callback to execute when the command fires. This might be - #: `None` in which case nothing happens. + #: ``None`` in which case nothing happens. self.callback = callback #: the list of parameters for this command in the order they #: should show up in the help page and execute. Eager parameters @@ -1025,7 +1025,7 @@ def get_help_option_names(self, ctx: Context) -> list[str]: def get_help_option(self, ctx: Context) -> Option | None: """Returns the help option object. - Skipped if :attr:`add_help_option` is `False`. + Skipped if :attr:`add_help_option` is ``False``. .. versionchanged:: 8.1.8 The help option is now cached to avoid creating it multiple times. @@ -1324,7 +1324,7 @@ def main( handle exceptions and convert them into error messages and the function will never return but shut down the interpreter. If - this is set to `False` they will be + this is set to ``False`` they will be propagated to the caller and the return value of this function is the return value of :meth:`invoke`. @@ -1368,7 +1368,7 @@ def main( # note that `rv` may actually contain data like "1" which # has obvious effects # more subtle case: `rv=[None, None]` can come out of - # chained commands which all returned `None` -- so it's not + # chained commands which all returned ``None`` -- so it's not # even always obvious that `rv` indicates success/failure # by its truthiness/falsiness ctx.exit() @@ -1708,7 +1708,7 @@ def cli(input): def process_result(result, input): return result + input - :param replace: if set to `True` an already existing result + :param replace: if set to ``True`` an already existing result callback will be removed. .. versionchanged:: 8.0 @@ -1735,7 +1735,7 @@ def function(value: t.Any, /, *args: t.Any, **kwargs: t.Any) -> t.Any: def get_command(self, ctx: Context, cmd_name: str) -> Command | None: """Given a context and a command name, this returns a :class:`Command` - object if it exists or returns `None`. + object if it exists or returns ``None``. """ return self.commands.get(cmd_name) @@ -2010,7 +2010,7 @@ class Parameter: the arity of the tuple). If ``nargs=-1``, all remaining parameters are collected. :param metavar: how the value is represented in the help page. - :param expose_value: if this is `True` then the value is passed onwards + :param expose_value: if this is ``True`` then the value is passed onwards to the command callback and stored on the context, otherwise it's skipped. :param is_eager: eager values are processed before non eager ones. This @@ -2025,7 +2025,7 @@ class Parameter: given. Takes ``ctx, param, incomplete`` and must return a list of :class:`~click.shell_completion.CompletionItem` or a list of strings. - :param deprecated: If `True` or non-empty string, issues a message + :param deprecated: If ``True`` or non-empty string, issues a message indicating that the argument is deprecated and highlights its deprecation in --help. The message can be customized by using a string as the value. A deprecated parameter @@ -2060,7 +2060,7 @@ class Parameter: .. versionchanged:: 8.0 Setting a default is no longer required for ``nargs>1``, it will - default to `None`. ``multiple=True`` or ``nargs=-1`` will + default to ``None``. ``multiple=True`` or ``nargs=-1`` will default to ``()``. .. versionchanged:: 7.1 @@ -2145,11 +2145,11 @@ def to_info_dict(self) -> dict[str, t.Any]: CLI structure. .. versionchanged:: 8.3.0 - Returns `None` for the :attr:`default` if it is :attr:`UNSET`. + Returns ``None`` for the :attr:`default` if it is :attr:`UNSET`. We explicitly hide the :attr:`UNSET` value to the user, as we choose to make it an implementation detail. And because ``to_info_dict`` has been - designed for documentation purposes, we return `None` instead. + designed for documentation purposes, we return ``None`` instead. .. versionadded:: 8.0 """ @@ -2368,7 +2368,7 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any: parameter is not given by the CLI user. The callback is receiving the value as-is, which let the developer decide - how to handle the different cases of `None`, ``UNSET``, or any other value. + how to handle the different cases of ``None``, ``UNSET``, or any other value. """ value = self.type_cast_value(ctx, value) @@ -2387,7 +2387,7 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: Environment variables values are `always returned as strings `_. - This method returns `None` if: + This method returns ``None`` if: - the :attr:`envvar` property is not set on the :class:`Parameter`, - the environment variable is not found in the environment, @@ -2531,32 +2531,32 @@ class Option(Parameter): :param show_default: Show the default value for this option in its help text. Values are not shown by default, unless - :attr:`Context.show_default` is `True`. If this value is a + :attr:`Context.show_default` is ``True``. If this value is a string, it shows that string in parentheses instead of the actual value. This is particularly useful for dynamic options. For single option boolean flags, the default remains hidden if - its value is `False`. + its value is ``False``. :param show_envvar: Controls if an environment variable should be shown on the help page and error messages. Normally, environment variables are not shown. - :param prompt: If set to `True` or a non empty string then the - user will be prompted for input. If set to `True` the prompt + :param prompt: If set to ``True`` or a non empty string then the + user will be prompted for input. If set to ``True`` the prompt will be the option name capitalized. A deprecated option cannot be prompted. :param confirmation_prompt: Prompt a second time to confirm the value if it was prompted for. Can be set to a string instead of - `True` to customize the message. - :param prompt_required: If set to `False`, the user will be + ``True`` to customize the message. + :param prompt_required: If set to ``False``, the user will be prompted for input only when the option was specified as a flag without a value. - :param hide_input: If this is `True` then the input on the prompt + :param hide_input: If this is ``True`` then the input on the prompt will be hidden from the user. This is useful for password input. :param is_flag: forces this option to act as a flag. The default is auto detection. :param flag_value: which value should be used for this flag if it's enabled. This is set to a boolean automatically if the option string contains a slash to mark two options. - :param multiple: if this is set to `True` then the argument is accepted + :param multiple: if this is set to ``True`` then the argument is accepted multiple times and recorded. This is similar to ``nargs`` in how it works but supports arbitrary number of arguments. @@ -2583,7 +2583,7 @@ class Option(Parameter): .. versionchanged:: 8.1 The default of a single option boolean flag is not shown if the - default value is `False`. + default value is ``False``. .. versionchanged:: 8.0.1 ``type`` is detected from ``flag_value`` if given. @@ -2742,11 +2742,11 @@ def __init__( def to_info_dict(self) -> dict[str, t.Any]: """ .. versionchanged:: 8.3.0 - Returns `None` for the :attr:`flag_value` if it is :attr:`UNSET`. + Returns ``None`` for the :attr:`flag_value` if it is :attr:`UNSET`. We explicitly hide the :attr:`UNSET` value to the user, as we choose to make it an implementation detail. And because ``to_info_dict`` has been - designed for documentation purposes, we return `None` instead. + designed for documentation purposes, we return ``None`` instead. """ info_dict = super().to_info_dict() info_dict.update( @@ -3018,7 +3018,7 @@ def prompt_for_value(self, ctx: Context) -> t.Any: return prompt( self.prompt, - # Use `None` to inform the prompt() function to reiterate until a valid + # Use ``None`` to inform the prompt() function to reiterate until a valid # value is provided by the user if we have no default. default=None if default is UNSET else default, type=self.type, @@ -3068,7 +3068,7 @@ def value_from_envvar(self, ctx: Context) -> t.Any: :attr:`flag_value` if the latter is set. This method also takes care of repeated options (i.e. options with - :attr:`multiple` set to `True`). + :attr:`multiple` set to ``True``). """ raw_value = self.resolve_envvar_value(ctx) From 9be1039f9c6ee98e3333746173f23dc949afe761 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 15:23:03 +0400 Subject: [PATCH 03/37] Keep original inconsistent ticks to reduce noise --- src/click/core.py | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index ab49079c65..1ba7f2a221 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -187,7 +187,7 @@ class Context: the name of the script. :param obj: an arbitrary object of user data. :param auto_envvar_prefix: the prefix to use for automatic environment - variables. If this is ``None`` then reading + variables. If this is `None` then reading from environment variables is disabled. This does not affect manually set environment variables which are always read. @@ -210,11 +210,11 @@ class Context: invocation. Default values will also be ignored. This is useful for implementing things such as completion support. - :param allow_extra_args: if this is set to ``True`` then extra arguments + :param allow_extra_args: if this is set to `True` then extra arguments at the end will not raise an error and will be kept on the context. The default is to inherit from the command. - :param allow_interspersed_args: if this is set to ``False`` then options + :param allow_interspersed_args: if this is set to `False` then options and arguments cannot be mixed. The default is to inherit from the command. :param ignore_unknown_options: instructs click to ignore options it does @@ -289,7 +289,7 @@ def __init__( color: bool | None = None, show_default: bool | None = None, ) -> None: - #: the parent context or ``None`` if none exists. + #: the parent context or `None` if none exists. self.parent = parent #: the :class:`Command` for this context. self.command = command @@ -494,7 +494,7 @@ def scope(self, cleanup: bool = True) -> cabc.Iterator[Context]: """This helper method can be used with the context object to promote it to the current thread local (see :func:`get_current_context`). The default behavior of this is to invoke the cleanup functions which - can be disabled by setting `cleanup` to ``False``. The cleanup + can be disabled by setting `cleanup` to `False`. The cleanup functions are typically used for things such as closing file handles. If the cleanup is intended the context object can also be directly @@ -931,7 +931,7 @@ def __init__( self.context_settings: cabc.MutableMapping[str, t.Any] = context_settings #: the callback to execute when the command fires. This might be - #: ``None`` in which case nothing happens. + #: `None` in which case nothing happens. self.callback = callback #: the list of parameters for this command in the order they #: should show up in the help page and execute. Eager parameters @@ -1324,7 +1324,7 @@ def main( handle exceptions and convert them into error messages and the function will never return but shut down the interpreter. If - this is set to ``False`` they will be + this is set to `False` they will be propagated to the caller and the return value of this function is the return value of :meth:`invoke`. @@ -1368,7 +1368,7 @@ def main( # note that `rv` may actually contain data like "1" which # has obvious effects # more subtle case: `rv=[None, None]` can come out of - # chained commands which all returned ``None`` -- so it's not + # chained commands which all returned `None` -- so it's not # even always obvious that `rv` indicates success/failure # by its truthiness/falsiness ctx.exit() @@ -1708,7 +1708,7 @@ def cli(input): def process_result(result, input): return result + input - :param replace: if set to ``True`` an already existing result + :param replace: if set to `True` an already existing result callback will be removed. .. versionchanged:: 8.0 @@ -2010,7 +2010,7 @@ class Parameter: the arity of the tuple). If ``nargs=-1``, all remaining parameters are collected. :param metavar: how the value is represented in the help page. - :param expose_value: if this is ``True`` then the value is passed onwards + :param expose_value: if this is `True` then the value is passed onwards to the command callback and stored on the context, otherwise it's skipped. :param is_eager: eager values are processed before non eager ones. This From 40fb3751ec6075538df7242524e3ff8efe366d1f Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 15:25:32 +0400 Subject: [PATCH 04/37] Keep original inconsistent ticks to reduce noise --- src/click/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index 1ba7f2a221..2ad27e6644 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2556,7 +2556,7 @@ class Option(Parameter): :param flag_value: which value should be used for this flag if it's enabled. This is set to a boolean automatically if the option string contains a slash to mark two options. - :param multiple: if this is set to ``True`` then the argument is accepted + :param multiple: if this is set to `True` then the argument is accepted multiple times and recorded. This is similar to ``nargs`` in how it works but supports arbitrary number of arguments. From c442c05c315e61b38932e6d25a4e12dfc533882e Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 15:26:05 +0400 Subject: [PATCH 05/37] Rewrap text Fix: https://github.com/pallets/click/pull/3030/files#r2288953007 --- src/click/core.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index 2ad27e6644..a182e48e85 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2017,9 +2017,8 @@ class Parameter: should not be set for arguments or it will inverse the order of processing. :param envvar: environment variable(s) that are used to provide a default value for - this parameter. This can be a string or a sequence of strings. If a - sequence is given, only the first non-empty environment variable is - used for the parameter. + this parameter. This can be a string or a sequence of strings. If a sequence is + given, only the first non-empty environment variable is used for the parameter. :param shell_complete: A function that returns custom shell completions. Used instead of the param's type completion if given. Takes ``ctx, param, incomplete`` and must return a list From 634549fc180a4c7655eafedce5b84a9b140b0a35 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 15:32:32 +0400 Subject: [PATCH 06/37] Don't refactor variable names unnecessarily Address: https://github.com/pallets/click/pull/3030/files#r2289177206 --- src/click/core.py | 38 +++++++++++++++++++------------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index a182e48e85..ff81fdd49c 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2407,11 +2407,11 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: envvars = [self.envvar] if isinstance(self.envvar, str) else self.envvar for envvar in envvars: - raw_value = os.environ.get(envvar) + rv = os.environ.get(envvar) # Return the first non-empty value of the list of environment variables. - if raw_value: - return raw_value + if rv: + return rv # Else, absence of value is interpreted as an environment variable that is # not set, so proceed to the next one. @@ -2424,12 +2424,12 @@ def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: parameter is expecting multiple values (i.e. its :attr:`nargs` property is set to a value other than ``1``). """ - raw_value = self.resolve_envvar_value(ctx) + rv = self.resolve_envvar_value(ctx) - if raw_value is not None and self.nargs != 1: - return self.type.split_envvar_value(raw_value) + if rv is not None and self.nargs != 1: + return self.type.split_envvar_value(rv) - return raw_value + return rv def handle_parse_result( self, ctx: Context, opts: cabc.Mapping[str, t.Any], args: list[str] @@ -3036,10 +3036,10 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: to build dynamiccaly the environment variable name using the :python:`{ctx.auto_envvar_prefix}_{self.name.upper()}` template. """ - raw_value = super().resolve_envvar_value(ctx) + rv = super().resolve_envvar_value(ctx) - if raw_value is not None: - return raw_value + if rv is not None: + return rv if ( self.allow_from_autoenv @@ -3047,11 +3047,11 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: and self.name is not None ): envvar = f"{ctx.auto_envvar_prefix}_{self.name.upper()}" - raw_value = os.environ.get(envvar) + rv = os.environ.get(envvar) # Return the first non-empty value of the list of environment variables. - if raw_value: - return raw_value + if rv: + return rv # Else, absence of value is interpreted as an environment variable that is # not set, so proceed to the next one. @@ -3069,10 +3069,10 @@ def value_from_envvar(self, ctx: Context) -> t.Any: This method also takes care of repeated options (i.e. options with :attr:`multiple` set to ``True``). """ - raw_value = self.resolve_envvar_value(ctx) + rv = self.resolve_envvar_value(ctx) # Absent environment variable or an empty string is interpreted as unset. - if raw_value is None: + if rv is None: return None # Non-boolean flags are more liberal in what they accept. But a flag being a @@ -3081,22 +3081,22 @@ def value_from_envvar(self, ctx: Context) -> t.Any: if self.is_flag and not self.is_bool_flag: # If the flag_value is set and match the envvar value, return it # directly. - if self.flag_value is not UNSET and raw_value == self.flag_value: + if self.flag_value is not UNSET and rv == self.flag_value: return self.flag_value # Analyze the envvar value as a boolean to know if the flag is # activated or not. - return types.BoolParamType.str_to_bool(raw_value) + return types.BoolParamType.str_to_bool(rv) # Split the envvar value if it is allowed to be repeated. value_depth = (self.nargs != 1) + bool(self.multiple) if value_depth > 0: - multi_rv = self.type.split_envvar_value(raw_value) + multi_rv = self.type.split_envvar_value(rv) if self.multiple and self.nargs != 1: multi_rv = batch(multi_rv, self.nargs) # type: ignore[assignment] return multi_rv - return raw_value + return rv def consume_value( self, ctx: Context, opts: cabc.Mapping[str, Parameter] From 44c2bea54ef1f4d153138be2db9537a7f442d461 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 15:43:28 +0400 Subject: [PATCH 07/37] Do not change the original structure of the code Address: https://github.com/pallets/click/pull/3030/files#r2289153981 --- src/click/core.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index ff81fdd49c..42bc97cfdd 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2405,15 +2405,20 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: if not self.envvar: return None - envvars = [self.envvar] if isinstance(self.envvar, str) else self.envvar - for envvar in envvars: - rv = os.environ.get(envvar) + if isinstance(self.envvar, str): + rv = os.environ.get(self.envvar) - # Return the first non-empty value of the list of environment variables. if rv: return rv - # Else, absence of value is interpreted as an environment variable that is - # not set, so proceed to the next one. + else: + for envvar in self.envvar: + rv = os.environ.get(envvar) + + # Return the first non-empty value of the list of environment variables. + if rv: + return rv + # Else, absence of value is interpreted as an environment variable that is + # not set, so proceed to the next one. return None @@ -3049,11 +3054,8 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: envvar = f"{ctx.auto_envvar_prefix}_{self.name.upper()}" rv = os.environ.get(envvar) - # Return the first non-empty value of the list of environment variables. if rv: return rv - # Else, absence of value is interpreted as an environment variable that is - # not set, so proceed to the next one. return None From cf0e5242b3adfbfbeaff64b24a398b57d15ae94b Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 15:49:43 +0400 Subject: [PATCH 08/37] Keep the change entry concise Address: https://github.com/pallets/click/pull/3030/files#r2288963504 --- src/click/core.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index 42bc97cfdd..d79dbab583 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2146,10 +2146,6 @@ def to_info_dict(self) -> dict[str, t.Any]: .. versionchanged:: 8.3.0 Returns ``None`` for the :attr:`default` if it is :attr:`UNSET`. - We explicitly hide the :attr:`UNSET` value to the user, as we choose to - make it an implementation detail. And because ``to_info_dict`` has been - designed for documentation purposes, we return ``None`` instead. - .. versionadded:: 8.0 """ return { @@ -2161,6 +2157,9 @@ def to_info_dict(self) -> dict[str, t.Any]: "required": self.required, "nargs": self.nargs, "multiple": self.multiple, + # We explicitly hide the :attr:`UNSET` value to the user, as we choose to + # make it an implementation detail. And because ``to_info_dict`` has been + # designed for documentation purposes, we return ``None`` instead. "default": self.default if self.default is not UNSET else None, "envvar": self.envvar, } @@ -2747,16 +2746,15 @@ def to_info_dict(self) -> dict[str, t.Any]: """ .. versionchanged:: 8.3.0 Returns ``None`` for the :attr:`flag_value` if it is :attr:`UNSET`. - - We explicitly hide the :attr:`UNSET` value to the user, as we choose to - make it an implementation detail. And because ``to_info_dict`` has been - designed for documentation purposes, we return ``None`` instead. """ info_dict = super().to_info_dict() info_dict.update( help=self.help, prompt=self.prompt, is_flag=self.is_flag, + # We explicitly hide the :attr:`UNSET` value to the user, as we choose to + # make it an implementation detail. And because ``to_info_dict`` has been + # designed for documentation purposes, we return ``None`` instead. flag_value=self.flag_value if self.flag_value is not UNSET else None, count=self.count, hidden=self.hidden, From 60feda3c9240dcb7f362874bb6ae709eea151983 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 16:01:48 +0400 Subject: [PATCH 09/37] Do not mention UNSET Address: https://github.com/pallets/click/pull/3030/files#r2288967052 --- src/click/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index d79dbab583..c5c199c010 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2248,7 +2248,7 @@ def consume_value( sourced from the environment variable, the default map, or the parameter's default value. In that order of precedence. - If no value is found, the value is returned as :attr:`UNSET`. + If no value is found, an internal sentinel value is returned. """ # If the value is set, it means the parser produced a value from user input. value = opts.get(self.name, UNSET) # type: ignore From b39590fd8560e3ee6bec411ddd575761bdb1a113 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 16:06:42 +0400 Subject: [PATCH 10/37] Make the source selection logic clearer Address: https://github.com/pallets/click/pull/3030/files#r2288971948 --- src/click/core.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index c5c199c010..ca3f93c9b0 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2250,8 +2250,10 @@ def consume_value( If no value is found, an internal sentinel value is returned. """ - # If the value is set, it means the parser produced a value from user input. + # Collect from the parse the value passed by the user to the CLI. value = opts.get(self.name, UNSET) # type: ignore + # If the value is set, it means it was sourced from the command line by the + # parser, otherwise it left unset by default. source = ( ParameterSource.COMMANDLINE if value is not UNSET From f561f1880e6bbc8f2d199d50f08dfa2002090596 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 16:25:16 +0400 Subject: [PATCH 11/37] Do not mention UNSET and keep the change entry small Address: https://github.com/pallets/click/pull/3030/files#r2289122448 --- src/click/core.py | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index ca3f93c9b0..b1eb88eb68 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2359,16 +2359,13 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any: 1. Type cast the value using :meth:`type_cast_value`. 2. Check if the value is missing (see: :meth:`value_is_missing`), and raise :exc:`MissingParameter` if it is required. - 3. If a :attr:`callback` is set, call it to have the value replaced buy the - result of the callback. + 3. If a :attr:`callback` is set, call it to have the value replaced by the + result of the callback. The callback is receiving the value as-is, which let + the developer decide how to handle the different cases. .. versionchanged:: 8.3 - - The :attr:`callback` is now receiving the :attr:`UNSET` sentinel if the - parameter is not given by the CLI user. - - The callback is receiving the value as-is, which let the developer decide - how to handle the different cases of ``None``, ``UNSET``, or any other value. + The :attr:`callback` gets an internal sentinel if the parameter was not set + by the user and no default was specified. """ value = self.type_cast_value(ctx, value) From 2d1ceffe936a38ff2bfe8102f3a2188ba72e1e6f Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 16:55:51 +0400 Subject: [PATCH 12/37] Rework method documentation Address: https://github.com/pallets/click/pull/3030/files#r2289127505 https://github.com/pallets/click/pull/3030/files#r2289161176 --- src/click/core.py | 69 +++++++++++++++++++++++++------------------ tests/test_basic.py | 2 +- tests/test_options.py | 2 +- 3 files changed, 43 insertions(+), 30 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index b1eb88eb68..83e9f89296 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2437,51 +2437,64 @@ def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: def handle_parse_result( self, ctx: Context, opts: cabc.Mapping[str, t.Any], args: list[str] ) -> tuple[t.Any, list[str]]: - """Process the value produced by the parser from user input.""" + """Process the value produced by the parser from user input. + + Always process the value through the Parameter's :attr:`type`, wherever it + comes from. + + If the parameter is deprecated, this method warn the user about it. But only if + the value has been explicitly set by the user (and as such, is not coming from + a default). + """ with augment_usage_errors(ctx, param=self): value, source = self.consume_value(ctx, opts) ctx.set_parameter_source(self.name, source) # type: ignore - # Only try to process the value if it is not coming from the defaults. - # Defaults set by the users are under their control and so are left - # untouched as Python values: these values does not need to be processed. - if source not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP): - # If the parameter is deprecated, we want to warn the user about it. - # If the value is UNSET there is no need to warn the user of a - # parameter that has not been invoked. - if self.deprecated and value is not UNSET: - extra_message = ( - f" {self.deprecated}" - if isinstance(self.deprecated, str) - else "" - ) - message = _( - "DeprecationWarning: The {param_type} {name!r} is deprecated." - "{extra_message}" - ).format( - param_type=self.param_type_name, - name=self.human_readable_name, - extra_message=extra_message, - ) - echo(style(message, fg="red"), err=True) + # Display a deprecation warning if necessary. + if ( + self.deprecated + and value is not UNSET + and source not in (ParameterSource.DEFAULT, ParameterSource.DEFAULT_MAP) + ): + extra_message = ( + f" {self.deprecated}" if isinstance(self.deprecated, str) else "" + ) + message = _( + "DeprecationWarning: The {param_type} {name!r} is deprecated." + "{extra_message}" + ).format( + param_type=self.param_type_name, + name=self.human_readable_name, + extra_message=extra_message, + ) + echo(style(message, fg="red"), err=True) + # Process the value through the parameter's type. try: value = self.process_value(ctx, value) except Exception: if not ctx.resilient_parsing: raise - - # Ignore values that we have a hard time processing. + # In resilient parsing mode, we do not want to fail the command if the + # value is incompatible with the parameter type, so we reset the value + # to UNSET, which will be interpreted as a missing value. value = UNSET - if self.expose_value and self.name not in ctx.params: + # Add parameter's value to the context. + if ( + self.expose_value + # We skip adding the value if it was previously set by another parameter + # targeting the same variable name. This prevents parameters competing for + # the same name to override each other. + and self.name not in ctx.params + ): # Click is logically enforcing that the name is None if the parameter is - # not exposed. We still assert it here to please the type checker. + # not to be exposed. We still assert it here to please the type checker. assert self.name is not None, ( f"{self!r} parameter's name should not be None when exposing value." ) - # Normalize unset values to None, as we're about to pass them to the + # Normalize UNSET values to None, as we're about to pass them to the # command function and move them to the pure-Python realm of user-written # code. ctx.params[self.name] = value if value is not UNSET else None diff --git a/tests/test_basic.py b/tests/test_basic.py index 5e490d9d9b..e28a6d6b70 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -286,7 +286,7 @@ def cli(flag): ("upper", [], "upper"), ), ) -def test_flag_value(runner, default, args, expect): +def test_flag_value_dual_options(runner, default, args, expect): @click.command() @click.option("--upper", "transformation", flag_value="upper", default=default) @click.option("--lower", "transformation", flag_value="lower") diff --git a/tests/test_options.py b/tests/test_options.py index 8747699f2f..3db60e03d0 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1372,7 +1372,7 @@ def cmd(foo): (None, ["--xml"], "xml"), ], ) -def test_flag_value_callback(runner, default, args, expected): +def test_flag_value_dual_callback(runner, default, args, expected): """ Reproduction of the issue reported in https://github.com/pallets/click/pull/3030#discussion_r2271571819 From 2fbc8b1ea993c4d5549bcebe94a437d7a6a56b6f Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 17:00:02 +0400 Subject: [PATCH 13/37] Reduce unnecessary changes --- src/click/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index 83e9f89296..c1b778892f 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2430,7 +2430,7 @@ def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: rv = self.resolve_envvar_value(ctx) if rv is not None and self.nargs != 1: - return self.type.split_envvar_value(rv) + rv = self.type.split_envvar_value(rv) return rv From 8386598a77d5aa05c5239e8525b7b34eb75b4d57 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 17:04:49 +0400 Subject: [PATCH 14/37] Typo --- src/click/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index c1b778892f..e5529e87fb 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2281,7 +2281,7 @@ def consume_value( return value, source def type_cast_value(self, ctx: Context, value: t.Any) -> t.Any: - """Convert and validate a value against the option's + """Convert and validate a value against the parameter's :attr:`type`, :attr:`multiple`, and :attr:`nargs`. """ if value in (None, UNSET): From 11bfb2c9e246b91ea35ed75e23c9aad80240a3d7 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 17:38:54 +0400 Subject: [PATCH 15/37] Remove assertion already covered by the type annotation Address: https://github.com/pallets/click/pull/3030/files#r2289115405 --- src/click/termui.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/click/termui.py b/src/click/termui.py index b0c7bfa6bf..dcbb22216c 100644 --- a/src/click/termui.py +++ b/src/click/termui.py @@ -220,8 +220,6 @@ def confirm( .. versionadded:: 4.0 Added the ``err`` parameter. """ - assert default in (True, False, None), "default must be True, False, or None" - prompt = _build_prompt( text, prompt_suffix, From c498ced0e7b595880b70a81767ae86902714ee1c Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 19:19:06 +0400 Subject: [PATCH 16/37] Add comments about the test cases Refs: https://github.com/pallets/click/pull/3030/files#r2289213427 https://github.com/pallets/click/pull/3030/files#r2289216839 https://github.com/pallets/click/pull/3030/files#r2289227725 --- tests/test_basic.py | 47 +++++++++++--- tests/test_options.py | 148 +++++++++++++++++++++++++++++++++++------- 2 files changed, 164 insertions(+), 31 deletions(-) diff --git a/tests/test_basic.py b/tests/test_basic.py index e28a6d6b70..2c61a701a5 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -277,24 +277,53 @@ def cli(flag): @pytest.mark.parametrize( - ("default", "args", "expect"), + ("default", "args", "expected"), + # These test cases are similar to the ones in + # tests/test_options.py::test_default_dual_option_callback, so keep them in sync. ( - # See: https://github.com/pallets/click/issues/3024#issuecomment-3146199461 + # Each option is returning its own flag_value, whatever the default is. (True, ["--upper"], "upper"), (True, ["--lower"], "lower"), - (True, [], "True"), + (False, ["--upper"], "upper"), + (False, ["--lower"], "lower"), + (None, ["--upper"], "upper"), + (None, ["--lower"], "lower"), + (UNSET, ["--upper"], "upper"), + (UNSET, ["--lower"], "lower"), + # Check that the last option wins when both are specified. + (True, ["--upper", "--lower"], "lower"), + (True, ["--lower", "--upper"], "upper"), + # Check that the default is returned as-is when no option is specified. ("upper", [], "upper"), + ("lower", [], "lower"), + ("uPPer", [], "uPPer"), + ("lOwEr", [], "lOwEr"), + (" ᕕ( ᐛ )ᕗ ", [], " ᕕ( ᐛ )ᕗ "), + (None, [], None), + # Default is normalized to None if it is UNSET. + (UNSET, [], None), + # Non-string defaults are process as strings by the default Parameter's type. + (True, [], "True"), + (False, [], "False"), + (42, [], "42"), + (12.3, [], "12.3"), ), ) -def test_flag_value_dual_options(runner, default, args, expect): +def test_flag_value_dual_options(runner, default, args, expected): + """Check how default is processed when options compete for the same variable name. + + Covers the regression reported in + https://github.com/pallets/click/issues/3024#issuecomment-3146199461 + """ + @click.command() - @click.option("--upper", "transformation", flag_value="upper", default=default) - @click.option("--lower", "transformation", flag_value="lower") - def cli(transformation): - click.echo(repr(transformation), nl=False) + @click.option("--upper", "case", flag_value="upper", default=default) + @click.option("--lower", "case", flag_value="lower") + def cli(case): + click.echo(repr(case), nl=False) result = runner.invoke(cli, args) - assert result.output == repr(expect) + assert result.output == repr(expected) def test_file_option(runner): diff --git a/tests/test_options.py b/tests/test_options.py index 3db60e03d0..2c67d13567 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -2,6 +2,7 @@ import os import re import sys +import tempfile if sys.version_info < (3, 11): enum.StrEnum = enum.Enum # type: ignore[assignment] @@ -1364,35 +1365,56 @@ def cmd(foo): @pytest.mark.parametrize( ("default", "args", "expected"), - [ - (None, [], "A real None"), - (UNSET, [], "A real UNSET"), - ("random", [], "random"), + # These test cases are similar to the ones in + # tests/test_basic.py::test_flag_value_dual_options, so keep them in sync. + ( + # Each option is returning its own flag_value, whatever the default is. + (True, ["--js"], "js"), + (True, ["--xml"], "xml"), + (False, ["--js"], "js"), + (False, ["--xml"], "xml"), (None, ["--js"], "js"), (None, ["--xml"], "xml"), - ], + (UNSET, ["--js"], "js"), + (UNSET, ["--xml"], "xml"), + # Check that the last option wins when both are specified. + (True, ["--js", "--xml"], "xml"), + (True, ["--xml", "--js"], "js"), + # Check that the default is returned as-is when no option is specified. + ("js", [], "js"), + ("xml", [], "xml"), + ("jS", [], "jS"), + ("xMl", [], "xMl"), + (" ᕕ( ᐛ )ᕗ ", [], " ᕕ( ᐛ )ᕗ "), + (None, [], None), + (UNSET, [], UNSET), + # Non-string defaults are process as strings by the default Parameter's type. + (True, [], "True"), + (False, [], "False"), + (42, [], "42"), + (12.3, [], "12.3"), + ), ) -def test_flag_value_dual_callback(runner, default, args, expected): - """ +def test_default_dual_option_callback(runner, default, args, expected): + """Check how default is processed by the callback when options compete for the same + variable name. + Reproduction of the issue reported in https://github.com/pallets/click/pull/3030#discussion_r2271571819 """ def _my_func(ctx, param, value): - if value is None: - return "A real None" - if value is UNSET: - return "A real UNSET" - return value + # Print the value received by the callback as-is, so we can check for it. + return f"Callback value: {value!r}" @click.command() @click.option("--js", "fmt", flag_value="js", callback=_my_func, default=default) @click.option("--xml", "fmt", flag_value="xml", callback=_my_func) def main(fmt): - click.secho(repr(fmt), nl=False) + click.secho(fmt, nl=False) result = runner.invoke(main, args) - assert result.output == repr(expected) + assert result.output == f"Callback value: {expected!r}" assert result.exit_code == 0 @@ -1688,19 +1710,89 @@ def cli(one, two): runner.invoke(cli, []) -def test_custom_type_click_class_flag_value(runner): - """A reproduction of +NO_CONFIG = object() +"""A sentinel value to indicate no configuration file is provided.""" + + +@pytest.mark.parametrize( + ("args", "expected"), + ( + # Option default to None. + ([], None), + # Config has no default value, and no flag value, so it requires an argument. + ( + ["--config"], + re.compile(re.escape("Error: Option '--config' requires an argument.\n")), + ), + ( + ["--no-config", "--config"], + re.compile(re.escape("Error: Option '--config' requires an argument.\n")), + ), + # Passing --no-config defaults to the sentinel value because of the flag_value, + # and then the custom type receives that sentinel and returns a message. + (["--no-config"], "No configuration file provided."), + # Passing --config with an argument returns the file path. + (["--config", "foo.conf"], "foo.conf"), + # An argument is not allowed for --no-config, so it raises an error. + ( + ["--no-config", "foo.conf"], + re.compile( + r"Usage: main \[OPTIONS\]\n" + r"Try 'main --help' for help.\n" + r"\n" + r"Error: Got unexpected extra argument (.+)\n" + ), + ), + # Passing --config with an argument that does not exist raises an error. + ( + ["--config", "random-file.conf"], + re.compile( + r"Usage: main \[OPTIONS\]\n" + r"Try 'main --help' for help.\n" + r"\n" + r"Error: Invalid value for '-c' / '--config': " + r"File 'random-file.conf' does not exist.\n" + ), + ), + ( + ["--config", "--no-config"], + re.compile( + r"Usage: main \[OPTIONS\]\n" + r"Try 'main --help' for help.\n" + r"\n" + r"Error: Invalid value for '-c' / '--config': " + r"File '--no-config' does not exist.\n" + ), + ), + ( + ["--config", "--no-config", "foo.conf"], + re.compile( + r"Usage: main \[OPTIONS\]\n" + r"Try 'main --help' for help.\n" + r"\n" + r"Error: Invalid value for '-c' / '--config': " + r"File '--no-config' does not exist.\n" + ), + ), + # --config is passed last and overrides the --no-config option. + (["--no-config", "--config", "foo.conf"], "foo.conf"), + ), +) +def test_dual_options_custom_type_sentinel_flag_value(runner, args, expected): + """Check that an object-based sentinel, used as a flag value, is returned as-is + to a custom type that is shared by two options, competing for the same variable + name. + + A reproduction of https://github.com/pallets/click/issues/3024#issuecomment-3146511356 """ - NO_CONFIG = object() - class ConfigParamType(click.ParamType): - name = "config" + """A custom type that accepts a file path or a sentinel value.""" def convert(self, value, param, ctx): if value is NO_CONFIG: - return value + return "No configuration file provided." else: return click.Path(exists=True, dir_okay=False).convert( value, param, ctx @@ -1712,8 +1804,20 @@ def convert(self, value, param, ctx): def main(config): click.echo(repr(config), nl=False) - result = runner.invoke(main) - assert result.output == repr(None) + with tempfile.NamedTemporaryFile(mode="w") as named_tempfile: + if "foo.conf" in args: + named_tempfile.write("Blah blah") + named_tempfile.flush() + args = [named_tempfile.name if a == "foo.conf" else a for a in args] + + result = runner.invoke(main, args) + + if isinstance(expected, re.Pattern): + assert re.match(expected, result.output) + else: + assert result.output == repr( + named_tempfile.name if expected == "foo.conf" else expected + ) class EngineType(enum.Enum): From f9ec2ead89406ab2d0d3ad547fb709b93c0cd144 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 19:30:39 +0400 Subject: [PATCH 17/37] Check falsy sentinels --- tests/test_options.py | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index 2c67d13567..4b8ac76627 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -3,6 +3,7 @@ import re import sys import tempfile +from typing import Literal if sys.version_info < (3, 11): enum.StrEnum = enum.Enum # type: ignore[assignment] @@ -1710,10 +1711,22 @@ def cli(one, two): runner.invoke(cli, []) -NO_CONFIG = object() -"""A sentinel value to indicate no configuration file is provided.""" +OBJECT_SENTINEL = object() +"""An object-based sentinel value.""" +class EnumSentinel(enum.Enum): + FALSY_SENTINEL = object() + + def __bool__(self) -> Literal[False]: + """Force the sentinel to be falsy to make sure it is not caught by Click internal + implementation. + """ + return False + + +# Any kind of sentinel value is recognized by Click as a valid flag value. +@pytest.mark.parametrize("sentinel", (OBJECT_SENTINEL, EnumSentinel.FALSY_SENTINEL)) @pytest.mark.parametrize( ("args", "expected"), ( @@ -1778,7 +1791,7 @@ def cli(one, two): (["--no-config", "--config", "foo.conf"], "foo.conf"), ), ) -def test_dual_options_custom_type_sentinel_flag_value(runner, args, expected): +def test_dual_options_custom_type_sentinel_flag_value(runner, sentinel, args, expected): """Check that an object-based sentinel, used as a flag value, is returned as-is to a custom type that is shared by two options, competing for the same variable name. @@ -1791,7 +1804,7 @@ class ConfigParamType(click.ParamType): """A custom type that accepts a file path or a sentinel value.""" def convert(self, value, param, ctx): - if value is NO_CONFIG: + if value is sentinel: return "No configuration file provided." else: return click.Path(exists=True, dir_okay=False).convert( @@ -1800,7 +1813,7 @@ def convert(self, value, param, ctx): @click.command() @click.option("-c", "--config", type=ConfigParamType()) - @click.option("--no-config", "config", flag_value=NO_CONFIG, type=ConfigParamType()) + @click.option("--no-config", "config", flag_value=sentinel, type=ConfigParamType()) def main(config): click.echo(repr(config), nl=False) From 6417acec00987fc79f451e87da994124dec7ccc9 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 19:31:03 +0400 Subject: [PATCH 18/37] Typo --- tests/test_options.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index 4b8ac76627..7b64ee55e7 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1478,7 +1478,7 @@ def main(fmt): ("bar", "t", "bar"), ("bar", "y", "bar"), # Deactivating the flag with a value recognized as False by the envvar, which - # explicitly return the 'False' as the option is explicitely declared as a + # explicitly return the 'False' as the option is explicitly declared as a # string. ("bar", "False", "False"), ("bar", "false", "False"), @@ -1490,7 +1490,7 @@ def main(fmt): ("bar", "f", "False"), ("bar", "n", "False"), # Any other value than the flag_value, or a recogned True or False value, fails - # to explicitely activate or deactivate the flag. So the flag default value is + # to explicitly activate or deactivate the flag. So the flag default value is # returned, which is None in this case. ("bar", " bar ", None), ("bar", "BAR", None), From b03501906f355cd629341cbdef6a7aeb457866c4 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 19:37:28 +0400 Subject: [PATCH 19/37] Add some discussion context --- tests/test_options.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/test_options.py b/tests/test_options.py index 7b64ee55e7..0746a5072c 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1721,6 +1721,10 @@ class EnumSentinel(enum.Enum): def __bool__(self) -> Literal[False]: """Force the sentinel to be falsy to make sure it is not caught by Click internal implementation. + + Falsy sentinels have been discussed in: + https://github.com/pallets/click/pull/3030#pullrequestreview-3106604795 + https://github.com/pallets/click/pull/3030#pullrequestreview-3108471552 """ return False From 354788eaba85d2ef405b300186a3752c2bc4dbaa Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 20:32:30 +0400 Subject: [PATCH 20/37] Comment the test cases Refs: https://github.com/pallets/click/pull/3030/files#r2289224850 --- tests/test_options.py | 68 ++++++++++++++++++++++++++++++------------- 1 file changed, 47 insertions(+), 21 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index 0746a5072c..09e9eb2463 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1854,53 +1854,79 @@ class Class2: @pytest.mark.parametrize( ("opt_params", "args", "expected"), [ - # Reproduction of cases from - # https://github.com/pallets/click/issues/3024#issuecomment-3146480714 - ({"type": EngineType, "flag_value": None}, [], None), + # Check that the flag value is returned as-is when the option is passed, and not normalized + # to a boolean, even if it is explicitly declared as a flag. ({"type": EngineType, "flag_value": None}, ["--pro"], None), - ({"type": EngineType, "flag_value": EngineType.MAX}, [], None), ( - {"type": EngineType, "flag_value": EngineType.MAX}, + {"type": EngineType, "is_flag": True, "flag_value": None}, ["--pro"], - EngineType.MAX, + None, ), - ({"type": EngineType, "default": EngineType.OSS}, [], EngineType.OSS), + ({"type": EngineType, "flag_value": EngineType.OSS}, ["--pro"], EngineType.OSS), ( - {"type": EngineType, "default": EngineType.OSS}, + {"type": EngineType, "is_flag": True, "flag_value": EngineType.OSS}, ["--pro"], - re.compile(re.escape("Error: Option '--pro' requires an argument.\n")), + EngineType.OSS, ), ( {"type": EngineType, "is_flag": True, "default": EngineType.OSS}, - [], + ["--pro"], EngineType.OSS, ), + # The default value is returned as-is when the option is not passed, whatever the flag value. + ({"type": EngineType, "flag_value": None}, [], None), + ({"type": EngineType, "is_flag": True, "flag_value": None}, [], None), + ({"type": EngineType, "flag_value": EngineType.OSS}, [], None), + ( + {"type": EngineType, "is_flag": True, "flag_value": EngineType.OSS}, + [], + None, + ), ( {"type": EngineType, "is_flag": True, "default": EngineType.OSS}, - ["--pro"], + [], EngineType.OSS, ), - # Reproduction of cases from - # https://github.com/pallets/click/issues/2012#issuecomment-892437060 + # The option has not enough parameters to be detected as flag-like, so it requires an argument. ( - {"flag_value": EngineType.OSS, "default": True}, - [], - "True", + {"type": EngineType, "default": EngineType.OSS}, + ["--pro"], + re.compile(re.escape("Error: Option '--pro' requires an argument.\n")), + ), + ({"type": EngineType, "default": EngineType.OSS}, [], EngineType.OSS), + # If a flag value is set, it is returned instead of the default value. + ( + {"type": EngineType, "flag_value": EngineType.OSS, "default": True}, + ["--pro"], + EngineType.OSS, ), ( {"type": EngineType, "flag_value": EngineType.OSS, "default": True}, [], EngineType.OSS, ), - ({"flag_value": 1, "default": True}, [], 1), + # Type is not specified and default to string, so the default value is + # returned as a string, even if it is a boolean. ({"flag_value": "1", "default": True}, [], "True"), - ({"flag_value": 1, "type": str, "default": True}, [], "True"), - ({"flag_value": "1", "type": str, "default": True}, [], "True"), - ({"flag_value": 1, "type": int, "default": True}, [], 1), - ({"flag_value": "1", "type": int, "default": True}, [], 1), + ({"flag_value": EngineType.OSS, "default": True}, [], "True"), + # See: the result is the same if we force the type to be str. + ({"type": str, "flag_value": 1, "default": True}, [], "True"), + ({"type": str, "flag_value": "1", "default": True}, [], "True"), + ({"type": str, "flag_value": EngineType.OSS, "default": True}, [], "True"), + # But having the flag value set to integer is automaticcally recognized by click. + ({"flag_value": 1, "default": True}, [], 1), + ({"type": int, "flag_value": 1, "default": True}, [], 1), + ({"type": int, "flag_value": "1", "default": True}, [], 1), ], ) def test_custom_type_flag_value_standalone_option(runner, opt_params, args, expected): + """Test how the type and flag_value influence the returned value. + + Cover cases reported in: + https://github.com/pallets/click/issues/3024#issuecomment-3146480714 + https://github.com/pallets/click/issues/2012#issuecomment-892437060 + """ + @click.command() @click.option("--pro", **opt_params) def scan(pro): From 7b84990ead4a663e4b57d5cdb83eca1e508c3d78 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 20:41:38 +0400 Subject: [PATCH 21/37] Describe the progression of tests Refs: https://github.com/pallets/click/pull/3030/files#r2289225645 https://github.com/pallets/click/pull/3030/files#r2289226067 https://github.com/pallets/click/pull/3030/files#r2289226489 --- tests/test_options.py | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index 09e9eb2463..1371345c3e 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1913,7 +1913,7 @@ class Class2: ({"type": str, "flag_value": 1, "default": True}, [], "True"), ({"type": str, "flag_value": "1", "default": True}, [], "True"), ({"type": str, "flag_value": EngineType.OSS, "default": True}, [], "True"), - # But having the flag value set to integer is automaticcally recognized by click. + # But having the flag value set to integer is automaticcally recognized by Click. ({"flag_value": 1, "default": True}, [], 1), ({"type": int, "flag_value": 1, "default": True}, [], 1), ({"type": int, "flag_value": "1", "default": True}, [], 1), @@ -1942,9 +1942,8 @@ def scan(pro): @pytest.mark.parametrize( ("opt1_params", "opt2_params", "args", "expected"), [ - ### - ### Reproduces https://github.com/pallets/click/issues/3024#issuecomment-3146508536 - ### + # Dual options sharing the same variable name, are not competitive, and the + # flag value is returned as-is. Especially when the type is force to be unprocessed. ( {"flag_value": EngineType.OSS, "type": UNPROCESSED}, {"flag_value": EngineType.PRO, "type": UNPROCESSED}, @@ -1963,10 +1962,8 @@ def scan(pro): ["--opt2"], EngineType.PRO, ), - ### - ### Reproduces https://github.com/pallets/click/issues/2012#issue-946471049 - ### - # Default type fallback to string. + # Check that passing exotic flag values like classes is supported, but are rendered to strings + # when the type is not specified. ({"flag_value": Class1, "default": True}, {"flag_value": Class2}, [], "True"), ( {"flag_value": Class1, "default": True}, @@ -2004,7 +2001,7 @@ def scan(pro): ), # Setting the default to a class, an instance of the class is returned instead # of the class itself, because the default is allowed to be callable (and - # consummd). And this what happens the type is. + # consummd). And this happens whatever the type is. ( {"flag_value": Class1, "default": Class1}, {"flag_value": Class2}, @@ -2029,9 +2026,7 @@ def scan(pro): [], re.compile(r""), ), - ### - ### Reproduces https://github.com/pallets/click/issues/2012#issuecomment-892437060 - ### + # Having the flag value set to integer is automaticcally recognized by Click. ( {"flag_value": 1, "default": True}, {"flag_value": "1"}, @@ -2049,6 +2044,15 @@ def scan(pro): def test_custom_type_flag_value_dual_options( runner, opt1_params, opt2_params, args, expected ): + """Test how flag values are processed with dual options competing for the same + variable name. + + Reproduce issues reported in: + https://github.com/pallets/click/issues/3024#issuecomment-3146508536 + https://github.com/pallets/click/issues/2012#issue-946471049 + https://github.com/pallets/click/issues/2012#issuecomment-892437060 + """ + @click.command() @click.option("--opt1", "dual_option", **opt1_params) @click.option("--opt2", "dual_option", **opt2_params) From 472bf383bcf403487b939242ddcd0bbb54257bb3 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 20:44:59 +0400 Subject: [PATCH 22/37] Describe test Refs: https://github.com/pallets/click/pull/3030/files#r2289226941 --- tests/test_options.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_options.py b/tests/test_options.py index 1371345c3e..f526565265 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -2067,7 +2067,10 @@ def cli(dual_option): def test_custom_type_frozenset_flag_value(runner): - """A reproduction of https://github.com/pallets/click/issues/2610""" + """Check that frozenset is correctly handled as a type, a flag value and a default. + + Reproduces https://github.com/pallets/click/issues/2610 + """ @click.command() @click.option( From e9e50023f87474939d99596a90b9c8d4335c03c5 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 20:51:14 +0400 Subject: [PATCH 23/37] Do not mention UNSET Refs: https://github.com/pallets/click/pull/3030#discussion_r2288957781 --- src/click/core.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index e5529e87fb..772548078e 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2144,7 +2144,7 @@ def to_info_dict(self) -> dict[str, t.Any]: CLI structure. .. versionchanged:: 8.3.0 - Returns ``None`` for the :attr:`default` if it is :attr:`UNSET`. + Returns ``None`` for the :attr:`default` if it was not set. .. versionadded:: 8.0 """ @@ -2757,7 +2757,7 @@ def __init__( def to_info_dict(self) -> dict[str, t.Any]: """ .. versionchanged:: 8.3.0 - Returns ``None`` for the :attr:`flag_value` if it is :attr:`UNSET`. + Returns ``None`` for the :attr:`flag_value` if it was not set. """ info_dict = super().to_info_dict() info_dict.update( From 9f7d99100aa8b1b81d8f011de679557e19e0c3d7 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Thu, 21 Aug 2025 20:54:26 +0400 Subject: [PATCH 24/37] Add description Refs: https://github.com/pallets/click/pull/3030#discussion_r2289215719 --- tests/test_termui.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_termui.py b/tests/test_termui.py index af63689f10..e7c9cf451e 100644 --- a/tests/test_termui.py +++ b/tests/test_termui.py @@ -646,7 +646,11 @@ def cmd(arg1): def test_flag_value_prompt( runner, opt_decls, opt_params, args, prompt, input, expected ): - """Covers concerns raised in issue https://github.com/pallets/click/issues/1992.""" + """Check how flag value are prompted and handled by all combinations of + ``prompt``, ``default``, and ``flag_value`` parameters. + + Covers concerns raised in issue https://github.com/pallets/click/issues/1992. + """ @click.command() @click.option(opt_decls, **opt_params) From 0acca45214a66eefe50716ecb0bcfcefa8fcc6eb Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 11:06:33 +0400 Subject: [PATCH 25/37] Mark recently documented methods as private Refs: https://github.com/pallets/click/pull/3030#discussion_r2291548633 https://github.com/pallets/click/pull/3030#discussion_r2291551517 --- src/click/core.py | 31 +++++++++++++++++++++++-------- 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index 772548078e..9b83105541 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2249,6 +2249,8 @@ def consume_value( default value. In that order of precedence. If no value is found, an internal sentinel value is returned. + + :meta private: """ # Collect from the parse the value passed by the user to the CLI. value = opts.get(self.name, UNSET) # type: ignore @@ -2337,13 +2339,14 @@ def convert(value: t.Any) -> t.Any: # tuple[t.Any, ...] return convert(value) def value_is_missing(self, value: t.Any) -> bool: - """ - A value is considered missing if: + """A value is considered missing if: - it is :attr:`UNSET`, - or if it is an empty sequence while the parameter is suppose to have non-single value (i.e. :attr:`nargs` is not ``1`` or :attr:`multiple` is set). + + :meta private: """ if value is UNSET: return True @@ -2366,6 +2369,8 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any: .. versionchanged:: 8.3 The :attr:`callback` gets an internal sentinel if the parameter was not set by the user and no default was specified. + + :meta private: """ value = self.type_cast_value(ctx, value) @@ -2399,6 +2404,8 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: The raw value extracted from the environment is not normalized and is returned as-is. Any normalization or reconciliation is performed later by the :class:`Parameter`'s :attr:`type`. + + :meta private: """ if not self.envvar: return None @@ -2426,6 +2433,8 @@ def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: Returns the string as-is or splits it into a sequence of strings if the parameter is expecting multiple values (i.e. its :attr:`nargs` property is set to a value other than ``1``). + + :meta private: """ rv = self.resolve_envvar_value(ctx) @@ -2445,6 +2454,8 @@ def handle_parse_result( If the parameter is deprecated, this method warn the user about it. But only if the value has been explicitly set by the user (and as such, is not coming from a default). + + :meta private: """ with augment_usage_errors(ctx, param=self): value, source = self.consume_value(ctx, opts) @@ -3050,6 +3061,8 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: the :attr:`envvar` property, we fallback on :attr:`Context.auto_envvar_prefix` to build dynamiccaly the environment variable name using the :python:`{ctx.auto_envvar_prefix}_{self.name.upper()}` template. + + :meta private: """ rv = super().resolve_envvar_value(ctx) @@ -3070,9 +3083,8 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: return None def value_from_envvar(self, ctx: Context) -> t.Any: - """ - For :class:`Option`, this method processes the raw environment variable string - the same way as :func:`Parameter.value_from_envvar` does. + """For :class:`Option`, this method processes the raw environment variable + string the same way as :func:`Parameter.value_from_envvar` does. But in the case of non-boolean flags, the value is analyzed to determine if the flag is activated or not, and returns a boolean of its activation, or the @@ -3080,6 +3092,8 @@ def value_from_envvar(self, ctx: Context) -> t.Any: This method also takes care of repeated options (i.e. options with :attr:`multiple` set to ``True``). + + :meta private: """ rv = self.resolve_envvar_value(ctx) @@ -3113,13 +3127,14 @@ def value_from_envvar(self, ctx: Context) -> t.Any: def consume_value( self, ctx: Context, opts: cabc.Mapping[str, Parameter] ) -> tuple[t.Any, ParameterSource]: - """ - For :class:`Option`, the value can be collected from an interactive prompt if - the option is a flag that needs a value (and the :attr:`prompt` property is + """For :class:`Option`, the value can be collected from an interactive prompt + if the option is a flag that needs a value (and the :attr:`prompt` property is set). Additionally, this method handles flag option that are activated without a value, in which case the :attr:`flag_value` is returned. + + :meta private: """ value, source = super().consume_value(ctx, opts) From bbe1eb6d41f54bd49690a5c385b0895d061a76c1 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 11:12:33 +0400 Subject: [PATCH 26/37] Remove duplicate test --- tests/test_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/test_utils.py b/tests/test_utils.py index 2e2a614791..85d2b11ba6 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -30,7 +30,6 @@ def test_unset_sentinel(): assert repr(value) == "Sentinel.UNSET" assert str(value) == "Sentinel.UNSET" assert bool(value) is True - assert value is not True # Try all native Python values that can be falsy or truthy. # See: https://docs.python.org/3/library/stdtypes.html#truth-value-testing From 06847da9126a99c88bf347adf213d33bfe4e4af2 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 15:16:15 +0400 Subject: [PATCH 27/37] Support legacy case of default aligning to flag_value for flag whose default=True Refs: https://github.com/pallets/click/pull/3030/files#r2288936493 --- CHANGES.rst | 15 ++++---- docs/options.md | 28 +++++++++++++- src/click/core.py | 28 ++++++++++++++ tests/test_basic.py | 4 +- tests/test_options.py | 42 +++++++++++++++------ tests/test_termui.py | 85 ++++++++++++++++++++++++++----------------- 6 files changed, 148 insertions(+), 54 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index b6e948e694..89b7fb7f45 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,13 +5,14 @@ Version 8.2.x Unreleased -- Rework flag value passing by disentangling use of ``default``. Formerly defaults - for ``flag_value`` were set by passing ``default=True``, now set by passing - ``default=``. This change breaks all default settings for flag value - type flags. In all other places in code, ``default`` takes on the default value. - Fixing this inconsistency allows the fixing of the linked issues and supports a - more consistent API. :issue:`1992` :issue:`2012`` :issue:`2514` :issue:`2610` - :issue:`3024` :pr:`3030` +- Rework relationship between ``flag_value`` and ``default``: the value given to + ``default`` is now left untouched, and keep the value it receive. So + ``default=`` is respected and ```` is passed on as-is + to the CLI function. With the exception of flag options, where setting + ``default=True`` maintain the legacy behavior of defaulting to the ``flag_value``. + This allow ``default`` to be of any type, including ``bool`` or ``None``, fixing + inconsistencies reported in: :issue:`1992` :issue:`2012` :issue:`2514` + :issue:`2610` :issue:`3024` :pr:`3030` - Custom ``callback`` function are now allowed to receive the ``UNSET`` sentinel value. :pr:`3030` - Allow ``default`` to be set on ``Argument`` for ``nargs = -1``. :issue:`2164` diff --git a/docs/options.md b/docs/options.md index 4b4a701059..198dd90133 100644 --- a/docs/options.md +++ b/docs/options.md @@ -314,7 +314,7 @@ If you want to define an alias for the second option only, then you will need to ## Flag Value -To have an flag pass a value to the underlying function set `flag_value`. This automatically sets `is_flag=True`. To influence the value-less behavior, force its value with `default='upper'`. Setting flag values can be used to create patterns like this: +To have an flag pass a value to the underlying function set `flag_value`. This automatically sets `is_flag=True`. To mark the flag as default, set `default=True`. Setting flag values can be used to create patterns like this: ```{eval-rst} .. click:example:: @@ -322,7 +322,7 @@ To have an flag pass a value to the underlying function set `flag_value`. This a import sys @click.command() - @click.option('--upper', 'transformation', flag_value='upper', default='upper') + @click.option('--upper', 'transformation', flag_value='upper', default=True) @click.option('--lower', 'transformation', flag_value='lower') def info(transformation): click.echo(getattr(sys.platform, transformation)()) @@ -335,6 +335,30 @@ To have an flag pass a value to the underlying function set `flag_value`. This a invoke(info) ``` +````{caution} +The `default` argument of options always [give to the underlying function its value *as-is*](api#click.Parameter). + +But for flags, the interaction between `flag_value` and `default` is a bit special. + +If a flag has a `flag_value`, setting `default` to `True` means that the flag is activated by default. Not that the value passed to the underlying function is the `True` Python value. Instead, the default value will be aligned to the `flag_value` behind the scenes. + +Which means, the in example above, this option: + +```python +@click.option('--upper', 'transformation', flag_value='upper', default=True) +``` + +is equivalent to: + +```python +@click.option('--upper', 'transformation', flag_value='upper', default='upper') +``` + +This was implemented to support legacy behavior, that will be removed in Click 9.0 to allow for default to take any value, including `True`. + +In the mean time, to avoid confusion, it is recommended to always set `default` to the actual default value you want to pass to the underlying function, and not use `True`, `False` or `None`. Unless that's the precise value you want to explicitly force as default. +```` + ## Values from Environment Variables To pass in a value in from a specific environment variable use `envvar`. diff --git a/src/click/core.py b/src/click/core.py index 9b83105541..1a00844350 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2595,6 +2595,15 @@ class Option(Parameter): :param hidden: hide this option from help outputs. :param attrs: Other command arguments described in :class:`Parameter`. + .. caution:: + Flags specifying a ``flag_value`` and whose ``default=True`` will have + their ``default`` aligned to the ``flag_value``. + + This means there is no way to setup a flag whose default ``True`` and + whose ``flag_value`` is something else than ``True``. + + This is to support legacy behavior that will be removed in Click 9.0. + .. versionchanged:: 8.2 ``envvar`` used with ``flag_value`` will always use the ``flag_value``, previously it would use the value of the environment variable. @@ -2722,6 +2731,25 @@ def __init__( if self.default is UNSET and not self.required: self.default = False + # XXX Support the legacy case of aligning the default value with the flag_value + # for flags whose default is explicitly set to True. As long as we have this + # condition, there is no way a flag can have a default set to True, unless its + # flag_value itself is set to True. Refs: + # https://github.com/pallets/click/issues/3024#issuecomment-3146199461 + # https://github.com/pallets/click/pull/3030/files#r2288936493 + if self.default is True and self.flag_value is not UNSET: + # This message is a convoluted way to explain that if you want things + # to be equal, make them equal. + # warnings.warn( + # "A flag's `default` value will no longer be aligned with its " + # "`flag_value` if `default=True` in Click 9.0. If you want the flag to " + # "get the same `default` as its `flag_value`, update the option to " + # "make its `default` parameter equal to its `flag_value`.", + # DeprecationWarning, + # stacklevel=2, + # ) + self.default = self.flag_value + # Set the default flag_value if it is not set. if self.flag_value is UNSET: if self.is_flag: diff --git a/tests/test_basic.py b/tests/test_basic.py index 2c61a701a5..dcf429c28c 100644 --- a/tests/test_basic.py +++ b/tests/test_basic.py @@ -302,8 +302,10 @@ def cli(flag): (None, [], None), # Default is normalized to None if it is UNSET. (UNSET, [], None), + # Legacy case: if default=True and flag_value is set, The value returned is the + # flag_value, not default itself. + (True, [], "upper"), # Non-string defaults are process as strings by the default Parameter's type. - (True, [], "True"), (False, [], "False"), (42, [], "42"), (12.3, [], "12.3"), diff --git a/tests/test_options.py b/tests/test_options.py index f526565265..756579e8b8 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1297,13 +1297,13 @@ def test_type_from_flag_value(): # Not passing --foo returns the default value as-is, in its Python type, then # converted by the option type. ({"type": bool, "default": True, "flag_value": True}, [], True), - ({"type": bool, "default": True, "flag_value": False}, [], True), + ({"type": bool, "default": True, "flag_value": False}, [], False), ({"type": bool, "default": False, "flag_value": True}, [], False), ({"type": bool, "default": False, "flag_value": False}, [], False), ({"type": bool, "default": None, "flag_value": True}, [], None), ({"type": bool, "default": None, "flag_value": False}, [], None), ({"type": str, "default": True, "flag_value": True}, [], "True"), - ({"type": str, "default": True, "flag_value": False}, [], "True"), + ({"type": str, "default": True, "flag_value": False}, [], "False"), ({"type": str, "default": False, "flag_value": True}, [], "False"), ({"type": str, "default": False, "flag_value": False}, [], "False"), ({"type": str, "default": "foo", "flag_value": True}, [], "foo"), @@ -1389,8 +1389,10 @@ def cmd(foo): (" ᕕ( ᐛ )ᕗ ", [], " ᕕ( ᐛ )ᕗ "), (None, [], None), (UNSET, [], UNSET), + # Legacy case: if default=True and flag_value is set, The value returned is the + # flag_value, not default itself. + (True, [], "js"), # Non-string defaults are process as strings by the default Parameter's type. - (True, [], "True"), (False, [], "False"), (42, [], "42"), (12.3, [], "12.3"), @@ -1906,17 +1908,30 @@ class Class2: EngineType.OSS, ), # Type is not specified and default to string, so the default value is - # returned as a string, even if it is a boolean. - ({"flag_value": "1", "default": True}, [], "True"), - ({"flag_value": EngineType.OSS, "default": True}, [], "True"), + # returned as a string, even if it is a boolean. Also, defaults to the + # flag_value instead of the default value to support legacy behavior. + ({"flag_value": "1", "default": True}, [], "1"), + ({"flag_value": "1", "default": 42}, [], "42"), + ({"flag_value": EngineType.OSS, "default": True}, [], "EngineType.OSS"), + ({"flag_value": EngineType.OSS, "default": 42}, [], "42"), # See: the result is the same if we force the type to be str. - ({"type": str, "flag_value": 1, "default": True}, [], "True"), - ({"type": str, "flag_value": "1", "default": True}, [], "True"), - ({"type": str, "flag_value": EngineType.OSS, "default": True}, [], "True"), + ({"type": str, "flag_value": 1, "default": True}, [], "1"), + ({"type": str, "flag_value": 1, "default": 42}, [], "42"), + ({"type": str, "flag_value": "1", "default": True}, [], "1"), + ({"type": str, "flag_value": "1", "default": 42}, [], "42"), + ( + {"type": str, "flag_value": EngineType.OSS, "default": True}, + [], + "EngineType.OSS", + ), + ({"type": str, "flag_value": EngineType.OSS, "default": 42}, [], "42"), # But having the flag value set to integer is automaticcally recognized by Click. ({"flag_value": 1, "default": True}, [], 1), + ({"flag_value": 1, "default": 42}, [], 42), ({"type": int, "flag_value": 1, "default": True}, [], 1), + ({"type": int, "flag_value": 1, "default": 42}, [], 42), ({"type": int, "flag_value": "1", "default": True}, [], 1), + ({"type": int, "flag_value": "1", "default": 42}, [], 42), ], ) def test_custom_type_flag_value_standalone_option(runner, opt_params, args, expected): @@ -1964,7 +1979,12 @@ def scan(pro): ), # Check that passing exotic flag values like classes is supported, but are rendered to strings # when the type is not specified. - ({"flag_value": Class1, "default": True}, {"flag_value": Class2}, [], "True"), + ( + {"flag_value": Class1, "default": True}, + {"flag_value": Class2}, + [], + re.compile(r"''"), + ), ( {"flag_value": Class1, "default": True}, {"flag_value": Class2}, @@ -1985,7 +2005,7 @@ def scan(pro): {"flag_value": Class1, "type": UNPROCESSED, "default": True}, {"flag_value": Class2, "type": UNPROCESSED}, [], - True, + re.compile(r""), ), ( {"flag_value": Class1, "type": UNPROCESSED, "default": True}, diff --git a/tests/test_termui.py b/tests/test_termui.py index e7c9cf451e..0b2ef80e67 100644 --- a/tests/test_termui.py +++ b/tests/test_termui.py @@ -489,6 +489,18 @@ def cmd(arg1): assert "my-default-value" not in result.output +REPEAT = object() +"""Sentinel value to indicate that the prompt is expected to be repeated. + +I.e. the value provided by the user is not satisfactory and need to be re-prompted. +""" + +INVALID = object() +"""Sentinel value to indicate that the prompt is expected to be invalid. + +On invalid input, Click will output an error message and re-prompt the user. +""" + BOOLEAN_FLAG_PROMPT_CASES = [ ### ### Test cases with prompt=True explicitly enabled for the flag. @@ -559,7 +571,7 @@ def cmd(arg1): ### # Prompt is allowed and the flag has no default, so it prompts. # But the flag_value is not set, so it defaults to a string. - # XXX ({"prompt": True}, [], "", "", ""), + ({"prompt": True}, [], "", "", REPEAT), ({"prompt": True}, [], "", "y", "y"), ({"prompt": True}, [], "", "n", "n"), ({"prompt": True}, [], "", "foo", "foo"), @@ -572,10 +584,10 @@ def cmd(arg1): ({"prompt": True, "flag_value": False}, [], "[y/N]", "y", True), ({"prompt": True, "flag_value": False}, [], "[y/N]", "n", False), # Other flag values changes the auto-detection of the flag type. - # XXX ({"prompt": True, "flag_value": None}, [], "", "", ""), + ({"prompt": True, "flag_value": None}, [], "", "", REPEAT), ({"prompt": True, "flag_value": None}, [], "", "y", "y"), ({"prompt": True, "flag_value": None}, [], "", "n", "n"), - # XXX ({"prompt": True, "flag_value": "foo"}, [], "", "", ""), + ({"prompt": True, "flag_value": "foo"}, [], "", "", REPEAT), ({"prompt": True, "flag_value": "foo"}, [], "", "y", "y"), ({"prompt": True, "flag_value": "foo"}, [], "", "n", "n"), ### @@ -585,9 +597,9 @@ def cmd(arg1): ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "", True), ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "y", True), ({"prompt": True, "default": True, "flag_value": True}, [], "[Y/n]", "n", False), - ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "", True), - ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "y", True), - ({"prompt": True, "default": True, "flag_value": False}, [], "[Y/n]", "n", False), + ({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "", False), + ({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "y", True), + ({"prompt": True, "default": True, "flag_value": False}, [], "[y/N]", "n", False), # default=False ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "", False), ({"prompt": True, "default": False, "flag_value": True}, [], "[y/N]", "y", True), @@ -596,31 +608,29 @@ def cmd(arg1): ({"prompt": True, "default": False, "flag_value": False}, [], "[y/N]", "y", True), ({"prompt": True, "default": False, "flag_value": False}, [], "[y/N]", "n", False), # default=None - # XXX - # ( - # {"prompt": True, "default": None, "flag_value": True}, - # [], - # "[y/n]", - # "", - # False, - # ), + ( + {"prompt": True, "default": None, "flag_value": True}, + [], + "[y/n]", + "", + INVALID, + ), ({"prompt": True, "default": None, "flag_value": True}, [], "[y/n]", "y", True), ({"prompt": True, "default": None, "flag_value": True}, [], "[y/n]", "n", False), - # XXX - # ( - # {"prompt": True, "default": None, "flag_value": False}, - # [], - # "[y/n]", - # "", - # False, - # ), + ( + {"prompt": True, "default": None, "flag_value": False}, + [], + "[y/n]", + "", + INVALID, + ), ({"prompt": True, "default": None, "flag_value": False}, [], "[y/n]", "y", True), ({"prompt": True, "default": None, "flag_value": False}, [], "[y/n]", "n", False), # If the flag_value is None, the flag behave like a string flag, whatever the # default is. - ({"prompt": True, "default": True, "flag_value": None}, [], "[True]", "", "True"), - ({"prompt": True, "default": True, "flag_value": None}, [], "[True]", "y", "y"), - ({"prompt": True, "default": True, "flag_value": None}, [], "[True]", "n", "n"), + ({"prompt": True, "default": True, "flag_value": None}, [], "", "", REPEAT), + ({"prompt": True, "default": True, "flag_value": None}, [], "", "y", "y"), + ({"prompt": True, "default": True, "flag_value": None}, [], "", "n", "n"), ( {"prompt": True, "default": False, "flag_value": None}, [], @@ -630,7 +640,7 @@ def cmd(arg1): ), ({"prompt": True, "default": False, "flag_value": None}, [], "[False]", "y", "y"), ({"prompt": True, "default": False, "flag_value": None}, [], "[False]", "n", "n"), - # XXX ({"prompt": True, "default": None, "flag_value": None}, [], "", "", "False"), + ({"prompt": True, "default": None, "flag_value": None}, [], "", "", REPEAT), ({"prompt": True, "default": None, "flag_value": None}, [], "", "y", "y"), ({"prompt": True, "default": None, "flag_value": None}, [], "", "n", "n"), ] @@ -672,15 +682,24 @@ def cli(flag): else: expected_output = "" if prompt is not None: + # Build the expected prompt. assert isinstance(prompt, str) - expected_output += "Flag" - if prompt: - expected_output += f" {prompt}" - expected_output += ": " + expected_prompt = f"Flag {prompt}: " if prompt else "Flag: " + + # Add the user input to the expected output. assert isinstance(input, str) - expected_output += f"{input}\n" - expected_output += f"{expected!r}\n" + expected_output += f"{expected_prompt}{input}\n" + + if expected is INVALID: + expected_output += "Error: invalid input\n" + + # The prompt is expected to be repeated. + if expected in (REPEAT, INVALID): + expected_output += expected_prompt + + if expected not in (REPEAT, INVALID): + expected_output += f"{expected!r}\n" assert result.output == expected_output assert not result.stderr - assert result.exit_code == 0 + assert result.exit_code == 0 if expected not in (REPEAT, INVALID) else 1 From 7f4fa21aa74c734a262626fa118b9c72d415102b Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 15:24:13 +0400 Subject: [PATCH 28/37] Lint --- src/click/core.py | 10 +++++----- tests/test_options.py | 24 ++++++++++++++---------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/click/core.py b/src/click/core.py index 1a00844350..b1df59d214 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2422,8 +2422,8 @@ def resolve_envvar_value(self, ctx: Context) -> str | None: # Return the first non-empty value of the list of environment variables. if rv: return rv - # Else, absence of value is interpreted as an environment variable that is - # not set, so proceed to the next one. + # Else, absence of value is interpreted as an environment variable that + # is not set, so proceed to the next one. return None @@ -2742,9 +2742,9 @@ def __init__( # to be equal, make them equal. # warnings.warn( # "A flag's `default` value will no longer be aligned with its " - # "`flag_value` if `default=True` in Click 9.0. If you want the flag to " - # "get the same `default` as its `flag_value`, update the option to " - # "make its `default` parameter equal to its `flag_value`.", + # "`flag_value` if `default=True` in Click 9.0. If you want the flag " + # "to get the same `default` as its `flag_value`, update the option " + # "to make its `default` parameter equal to its `flag_value`.", # DeprecationWarning, # stacklevel=2, # ) diff --git a/tests/test_options.py b/tests/test_options.py index 756579e8b8..c40fb1a869 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1721,8 +1721,8 @@ class EnumSentinel(enum.Enum): FALSY_SENTINEL = object() def __bool__(self) -> Literal[False]: - """Force the sentinel to be falsy to make sure it is not caught by Click internal - implementation. + """Force the sentinel to be falsy to make sure it is not caught by Click + internal implementation. Falsy sentinels have been discussed in: https://github.com/pallets/click/pull/3030#pullrequestreview-3106604795 @@ -1856,8 +1856,8 @@ class Class2: @pytest.mark.parametrize( ("opt_params", "args", "expected"), [ - # Check that the flag value is returned as-is when the option is passed, and not normalized - # to a boolean, even if it is explicitly declared as a flag. + # Check that the flag value is returned as-is when the option is passed, and + # not normalized to a boolean, even if it is explicitly declared as a flag. ({"type": EngineType, "flag_value": None}, ["--pro"], None), ( {"type": EngineType, "is_flag": True, "flag_value": None}, @@ -1875,7 +1875,8 @@ class Class2: ["--pro"], EngineType.OSS, ), - # The default value is returned as-is when the option is not passed, whatever the flag value. + # The default value is returned as-is when the option is not passed, whatever + # the flag value. ({"type": EngineType, "flag_value": None}, [], None), ({"type": EngineType, "is_flag": True, "flag_value": None}, [], None), ({"type": EngineType, "flag_value": EngineType.OSS}, [], None), @@ -1889,7 +1890,8 @@ class Class2: [], EngineType.OSS, ), - # The option has not enough parameters to be detected as flag-like, so it requires an argument. + # The option has not enough parameters to be detected as flag-like, so it + # requires an argument. ( {"type": EngineType, "default": EngineType.OSS}, ["--pro"], @@ -1925,7 +1927,8 @@ class Class2: "EngineType.OSS", ), ({"type": str, "flag_value": EngineType.OSS, "default": 42}, [], "42"), - # But having the flag value set to integer is automaticcally recognized by Click. + # But having the flag value set to integer is automaticcally recognized by + # Click. ({"flag_value": 1, "default": True}, [], 1), ({"flag_value": 1, "default": 42}, [], 42), ({"type": int, "flag_value": 1, "default": True}, [], 1), @@ -1958,7 +1961,8 @@ def scan(pro): ("opt1_params", "opt2_params", "args", "expected"), [ # Dual options sharing the same variable name, are not competitive, and the - # flag value is returned as-is. Especially when the type is force to be unprocessed. + # flag value is returned as-is. Especially when the type is force to be + # unprocessed. ( {"flag_value": EngineType.OSS, "type": UNPROCESSED}, {"flag_value": EngineType.PRO, "type": UNPROCESSED}, @@ -1977,8 +1981,8 @@ def scan(pro): ["--opt2"], EngineType.PRO, ), - # Check that passing exotic flag values like classes is supported, but are rendered to strings - # when the type is not specified. + # Check that passing exotic flag values like classes is supported, but are + # rendered to strings when the type is not specified. ( {"flag_value": Class1, "default": True}, {"flag_value": Class2}, From cd66233c35beaed9e9c477b9385d82665f410d08 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 15:26:40 +0400 Subject: [PATCH 29/37] Remove bad ref --- docs/options.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/options.md b/docs/options.md index 198dd90133..e26c0d3c06 100644 --- a/docs/options.md +++ b/docs/options.md @@ -336,7 +336,7 @@ To have an flag pass a value to the underlying function set `flag_value`. This a ``` ````{caution} -The `default` argument of options always [give to the underlying function its value *as-is*](api#click.Parameter). +The `default` argument of options always give to the underlying function its value *as-is*. But for flags, the interaction between `flag_value` and `default` is a bit special. From 4f70b379b55b3b7f7dec0d8b9d3e91a347c238e1 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 15:28:01 +0400 Subject: [PATCH 30/37] Fix typing --- src/click/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index b1df59d214..a6ade1011e 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2439,7 +2439,7 @@ def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: rv = self.resolve_envvar_value(ctx) if rv is not None and self.nargs != 1: - rv = self.type.split_envvar_value(rv) + rv = t.cast(cabc.Sequence[str], self.type.split_envvar_value(rv)) return rv From abaef53babf47f1094076bf397b69a0fa79f3f63 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 15:30:36 +0400 Subject: [PATCH 31/37] Typing --- src/click/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/click/core.py b/src/click/core.py index a6ade1011e..95813e9840 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2439,7 +2439,7 @@ def value_from_envvar(self, ctx: Context) -> str | cabc.Sequence[str] | None: rv = self.resolve_envvar_value(ctx) if rv is not None and self.nargs != 1: - rv = t.cast(cabc.Sequence[str], self.type.split_envvar_value(rv)) + return self.type.split_envvar_value(rv) return rv From bb2a1d952cd79151dc8ce859ac65230ff95278d3 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 17:54:21 +0400 Subject: [PATCH 32/37] Better doc and tests for multiple Refs: https://github.com/pallets/click/pull/3030#discussion_r2289207325 --- tests/test_options.py | 197 ++++++++++++++++++++++++++++++------------ 1 file changed, 143 insertions(+), 54 deletions(-) diff --git a/tests/test_options.py b/tests/test_options.py index c40fb1a869..10213e4728 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -174,88 +174,121 @@ def cli(message): @pytest.mark.parametrize( - ("args", "multiple", "nargs", "default", "expected"), + ("multiple", "nargs", "default", "expected"), [ # If multiple values are allowed, defaults should be iterable. - ([], True, 1, [], ()), - ([], True, 1, (), ()), - ([], True, 1, tuple(), ()), - ([], True, 1, set(), ()), - ([], True, 1, frozenset(), ()), - ([], True, 1, {}, ()), + (True, 1, [], ()), + (True, 1, (), ()), + (True, 1, tuple(), ()), + (True, 1, set(), ()), + (True, 1, frozenset(), ()), + (True, 1, {}, ()), # Special values. - ([], True, 1, None, ()), - ([], True, 1, UNSET, ()), + (True, 1, None, ()), + (True, 1, UNSET, ()), # Number of values are kept as-is in the default. - ([], True, 1, [1], (1,)), - ([], True, 1, [1, 2], (1, 2)), - ([], True, 1, [1, 2, 3], (1, 2, 3)), - ([], True, 1, [1.1, 2.2], (1.1, 2.2)), - ([], True, 1, ["1", "2"], ("1", "2")), - ([], True, 1, [None, None], (None, None)), - # XXX Why items of the defaults are converted to strings and not left as - # integers? - ([], True, 1, {1, 2}, ("1", "2")), - ([], True, 1, frozenset([1, 2]), ("1", "2")), - ([], True, 1, {1: "a", 2: "b"}, ("1", "2")), + (True, 1, [1], (1,)), + (True, 1, [1, 2], (1, 2)), + (True, 1, [1, 2, 3], (1, 2, 3)), + (True, 1, [1.1, 2.2], (1.1, 2.2)), + (True, 1, ["1", "2"], ("1", "2")), + (True, 1, [None, None], (None, None)), + # Contrary to list or tuples, native Python types not supported by Click are + # not recognized and are converted to the default format: tuple of strings. + # Refs: https://github.com/pallets/click/issues/3036 + (True, 1, {1, 2}, ("1", "2")), + (True, 1, frozenset([1, 2]), ("1", "2")), + (True, 1, {1: "a", 2: "b"}, ("1", "2")), # Multiple values with nargs > 1. - ([], True, 2, [], ()), - ([], True, 2, (), ()), - ([], True, 2, tuple(), ()), - ([], True, 2, set(), ()), - ([], True, 2, frozenset(), ()), - ([], True, 2, {}, ()), - ([], True, 2, None, ()), - ([], True, 2, UNSET, ()), - ([], True, 2, [[1, 2]], ((1, 2),)), - ([], True, 2, [[1, 2], [3, 4]], ((1, 2), (3, 4))), - ([], True, 2, [[1, 2], [3, 4], [5, 6]], ((1, 2), (3, 4), (5, 6))), - ([], True, 2, [[1.1, 2.2], [3.3, 4.4]], ((1.1, 2.2), (3.3, 4.4))), - ([], True, 2, [["1", "2"], ["3", "4"]], (("1", "2"), ("3", "4"))), - ([], True, 2, [[None, None], [None, None]], ((None, None), (None, None))), - ([], True, 2, [[1, 2.2], ["3", None]], ((1, 2.2), (3, None))), - ([], True, 2, [[1, 2.2], None], ((1, 2.2), None)), - # - ([], False, 2, [1, 2], (1, 2)), - # + (True, 2, [], ()), + (True, 2, (), ()), + (True, 2, tuple(), ()), + (True, 2, set(), ()), + (True, 2, frozenset(), ()), + (True, 2, {}, ()), + (True, 2, None, ()), + (True, 2, UNSET, ()), + (True, 2, [[1, 2]], ((1, 2),)), + (True, 2, [[1, 2], [3, 4]], ((1, 2), (3, 4))), + (True, 2, [[1, 2], [3, 4], [5, 6]], ((1, 2), (3, 4), (5, 6))), + (True, 2, [[1.1, 2.2], [3.3, 4.4]], ((1.1, 2.2), (3.3, 4.4))), + (True, 2, [["1", "2"], ["3", "4"]], (("1", "2"), ("3", "4"))), + (True, 2, [[None, None], [None, None]], ((None, None), (None, None))), + (True, 2, [[1, 2.2], ["3", None]], ((1, 2.2), (3, None))), + (True, 2, [[1, 2.2], None], ((1, 2.2), None)), + # Default of the right length works for non-multiples. + (False, 2, [1, 2], (1, 2)), ], ) -def test_good_defaults_for_multiple(runner, args, multiple, nargs, default, expected): +def test_good_defaults_for_multiple(runner, multiple, nargs, default, expected): @click.command() @click.option("-a", multiple=multiple, nargs=nargs, default=default) def cmd(a): click.echo(repr(a), nl=False) - result = runner.invoke(cmd, args) + result = runner.invoke(cmd) assert result.output == repr(expected) @pytest.mark.parametrize( - ("multiple", "nargs", "default", "message"), + ("multiple", "nargs", "default", "exception", "message"), [ # Non-iterables defaults. - (True, 1, "Yo", "Error: Invalid value for '-a': Value must be an iterable."), - (True, 1, "", "Error: Invalid value for '-a': Value must be an iterable."), ( True, 1, + "Yo", + None, + "Error: Invalid value for '-a': Value must be an iterable.", + ), + ( + True, + 1, + "", + None, + "Error: Invalid value for '-a': Value must be an iterable.", + ), + ( True, + 1, + True, + None, "Error: Invalid value for '-a': Value must be an iterable.", ), ( True, 1, False, + None, + "Error: Invalid value for '-a': Value must be an iterable.", + ), + ( + True, + 1, + 12, + None, + "Error: Invalid value for '-a': Value must be an iterable.", + ), + ( + True, + 1, + 7.9, + None, "Error: Invalid value for '-a': Value must be an iterable.", ), - (True, 1, 12, "Error: Invalid value for '-a': Value must be an iterable."), - (True, 1, 7.9, "Error: Invalid value for '-a': Value must be an iterable."), # - (False, 2, 42, "Error: Invalid value for '-a': Value must be an iterable."), + ( + False, + 2, + 42, + None, + "Error: Invalid value for '-a': Value must be an iterable.", + ), ( True, 2, ["test string which is not a list in the list"], + None, "Error: Invalid value for '-a': Value must be an iterable.", ), # Multiple options, each with 2 args, but with wrong length. @@ -263,31 +296,87 @@ def cmd(a): True, 2, (1,), + None, "Error: Invalid value for '-a': Value must be an iterable.", ), ( True, 2, (1, 2, 3), + None, "Error: Invalid value for '-a': Value must be an iterable.", ), + ( + True, + 2, + [tuple()], + ValueError, + r"'nargs' must be 0 \(or None\) for type , but it was 2\.", + ), + ( + True, + 2, + [(1,)], + ValueError, + r"'nargs' must be 1 \(or None\) for type , but it was 2\.", + ), + ( + True, + 2, + [(1, 2, 3)], + ValueError, + r"'nargs' must be 3 \(or None\) for type , but it was 2\.", + ), # A mix of valid and invalid defaults. ( True, 2, [[1, 2.2], []], + None, "Error: Invalid value for '-a': 2 values are required, but 0 were given.", ), + # Default values that are iterable but not of the right length. + ( + False, + 2, + [1], + None, + "Error: Invalid value for '-a': Takes 2 values but 1 was given.", + ), + ( + True, + 2, + [[1]], + ValueError, + r"'nargs' must be 1 \(or None\) for type , but it was 2\.", + ), ], ) -def test_bad_defaults_for_multiple(runner, multiple, nargs, default, message): - @click.command() - @click.option("-a", multiple=multiple, nargs=nargs, default=default) - def cmd(a): - click.echo(repr(a)) +def test_bad_defaults_for_multiple( + runner, multiple, nargs, default, exception, message +): + if exception: + assert issubclass(exception, Exception) + else: + assert exception is None + + with ( + pytest.raises(exception, match=re.compile(message)) + if exception + else nullcontext() + ): + + @click.command() + @click.option("-a", multiple=multiple, nargs=nargs, default=default) + def cmd(a): + click.echo(repr(a)) - result = runner.invoke(cmd, []) - assert message in result.stderr + result = runner.invoke(cmd) + assert message in result.stderr @pytest.mark.parametrize("env_key", ["MYPATH", "AUTO_MYPATH"]) From 96e4c7599025eaefa58b7e804805e366f511a219 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 18:18:10 +0400 Subject: [PATCH 33/37] Add explanation for the casting Refs: https://github.com/pallets/click/pull/3030#discussion_r2289180249 --- src/click/core.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/click/core.py b/src/click/core.py index 95813e9840..f68e332c4b 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -3059,6 +3059,12 @@ def prompt_for_value(self, ctx: Context) -> t.Any: # one. if default in (UNSET, None): default = None + # Nothing prevent you to declare an option that is auto-detected as a + # boolean flag, is allow to prompt but still declare a non-boolean default. + # So with this casting, we aligns the default value to the prompt behavior. + # The prompt is going to default to [Y/n]), and so not entering a value for + # input is expected to make the option takes True as the default. + # Refs: https://github.com/pallets/click/pull/3030#discussion_r2289180249 else: default = bool(default) return confirm(self.prompt, default) From a2b699a134ae4f2459808290f62e96e5b3c1d54f Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 18:20:38 +0400 Subject: [PATCH 34/37] Fix import --- tests/test_options.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_options.py b/tests/test_options.py index 10213e4728..e06ea73aa1 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -3,6 +3,7 @@ import re import sys import tempfile +from contextlib import nullcontext from typing import Literal if sys.version_info < (3, 11): From 029bbeda76aa824149f110408d56de7b402a87ed Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 18:39:26 +0400 Subject: [PATCH 35/37] Hide UNSET in the callback Refs: https://github.com/pallets/click/pull/3030#discussion_r2291553604 --- CHANGES.rst | 2 -- src/click/core.py | 13 +++++++------ tests/test_options.py | 3 ++- 3 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 89b7fb7f45..800ab3c019 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -13,8 +13,6 @@ Unreleased This allow ``default`` to be of any type, including ``bool`` or ``None``, fixing inconsistencies reported in: :issue:`1992` :issue:`2012` :issue:`2514` :issue:`2610` :issue:`3024` :pr:`3030` -- Custom ``callback`` function are now allowed to receive the ``UNSET`` sentinel - value. :pr:`3030` - Allow ``default`` to be set on ``Argument`` for ``nargs = -1``. :issue:`2164` :pr:`3030` - Show correct auto complete value for ``nargs`` option in combination with flag diff --git a/src/click/core.py b/src/click/core.py index f68e332c4b..b24a82cef4 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2363,12 +2363,9 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any: 2. Check if the value is missing (see: :meth:`value_is_missing`), and raise :exc:`MissingParameter` if it is required. 3. If a :attr:`callback` is set, call it to have the value replaced by the - result of the callback. The callback is receiving the value as-is, which let - the developer decide how to handle the different cases. - - .. versionchanged:: 8.3 - The :attr:`callback` gets an internal sentinel if the parameter was not set - by the user and no default was specified. + result of the callback. If the value was not set, the callback receive + ``None``. This keep the legacy behavior as it was before the introduction of + the :attr:`UNSET` sentinel. :meta private: """ @@ -2378,6 +2375,10 @@ def process_value(self, ctx: Context, value: t.Any) -> t.Any: raise MissingParameter(ctx=ctx, param=self) if self.callback is not None: + # Legacy case: UNSET is not exposed directly to the callback, but converted + # to None. + if value is UNSET: + value = None value = self.callback(ctx, self, value) return value diff --git a/tests/test_options.py b/tests/test_options.py index e06ea73aa1..3cfff55d19 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1478,7 +1478,8 @@ def cmd(foo): ("xMl", [], "xMl"), (" ᕕ( ᐛ )ᕗ ", [], " ᕕ( ᐛ )ᕗ "), (None, [], None), - (UNSET, [], UNSET), + # Legacy case: UNSET is not exposed directly to the callback, but converted to None. + (UNSET, [], None), # Legacy case: if default=True and flag_value is set, The value returned is the # flag_value, not default itself. (True, [], "js"), From b9abeaa315939122abf6bb35b636445b1a44dd65 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 18:41:54 +0400 Subject: [PATCH 36/37] Hint at the concepts carried by default --- src/click/core.py | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/click/core.py b/src/click/core.py index b24a82cef4..c2af253715 100644 --- a/src/click/core.py +++ b/src/click/core.py @@ -2080,6 +2080,16 @@ def __init__( param_decls: cabc.Sequence[str] | None = None, type: types.ParamType | t.Any | None = None, required: bool = False, + # XXX The default historically embed two concepts: + # - the declaration of a Parameter object carrying the default (handy to + # arbitrage the default value of coupled Parameters sharing the same + # self.name, like flag options), + # - and the actual value of the default. + # It is confusing and is the source of many issues discussed in: + # https://github.com/pallets/click/pull/3030 + # In the future, we might think of splitting it in two, not unlike + # Option.is_flag and Option.flag_value: we could have something like + # Parameter.is_default and Parameter.default_value. default: t.Any | t.Callable[[], t.Any] | None = UNSET, callback: t.Callable[[Context, Parameter, t.Any], t.Any] | None = None, nargs: int | None = None, From 285f45cf68070b13d460093c494b8307cdfe6e41 Mon Sep 17 00:00:00 2001 From: Kevin Deldycke Date: Sat, 23 Aug 2025 18:54:54 +0400 Subject: [PATCH 37/37] Lint --- tests/test_options.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_options.py b/tests/test_options.py index 3cfff55d19..35317c78eb 100644 --- a/tests/test_options.py +++ b/tests/test_options.py @@ -1478,7 +1478,8 @@ def cmd(foo): ("xMl", [], "xMl"), (" ᕕ( ᐛ )ᕗ ", [], " ᕕ( ᐛ )ᕗ "), (None, [], None), - # Legacy case: UNSET is not exposed directly to the callback, but converted to None. + # Legacy case: UNSET is not exposed directly to the callback, but converted to + # None. (UNSET, [], None), # Legacy case: if default=True and flag_value is set, The value returned is the # flag_value, not default itself.