From 3b29417b49e6a274793c134b0d45a5e6df185c4c Mon Sep 17 00:00:00 2001 From: dskrypa Date: Tue, 30 Jun 2026 17:38:44 -0400 Subject: [PATCH] improved TriFlag by allowing default to match consts; fixed TriFlag to reject late default_cb registration when required=True --- Makefile | 3 +++ docs/_src/parameters.rst | 7 ++++- lib/cli_command_parser/parameters/base.py | 10 +++---- lib/cli_command_parser/parameters/options.py | 11 +++----- tests/test_parameters/test_flags.py | 28 +++++++++++--------- tests/test_parsing/test_parse_flags.py | 14 ++++++++++ 6 files changed, 47 insertions(+), 26 deletions(-) diff --git a/Makefile b/Makefile index e0f5b9c3..fdfeeec0 100644 --- a/Makefile +++ b/Makefile @@ -18,3 +18,6 @@ publish: sign: for f in dist/*; do echo "Signing $${f}"; gpg --armor --detach-sign $${f}; done + +verify: + for f in dist/*.asc; do gpg --verify $$f $${f%.asc}; done diff --git a/docs/_src/parameters.rst b/docs/_src/parameters.rst index 79fb665e..ef78806b 100644 --- a/docs/_src/parameters.rst +++ b/docs/_src/parameters.rst @@ -270,7 +270,12 @@ provided, respectively. :alt_long: The alternate long form to use. :alt_short: The alternate short form to use. :alt_help: The help text to display with the alternate option strings. -:default: The default value to use if neither the primary or alternate options are provided. Defaults to None. +:default: The default value to use if neither the primary nor alternate options are provided. Defaults to None. + + .. version-changed:: 2026-06-30 + + The ``default`` value may now match either value in ``consts`` (previously, matching values were rejected). + :name_mode: Override the configured :ref:`configuration:Parsing Options:option_name_mode` for the TriFlag. :type: A callable (function, class, etc.) that accepts a single string argument and returns a boolean value, which should be called on environment variable values, if any are configured for this TriFlag via diff --git a/lib/cli_command_parser/parameters/base.py b/lib/cli_command_parser/parameters/base.py index 8ef36524..40c4d07e 100644 --- a/lib/cli_command_parser/parameters/base.py +++ b/lib/cli_command_parser/parameters/base.py @@ -380,16 +380,16 @@ def register_default_cb(self, method: CommandMethod[D]) -> CommandMethod[D]: :return: The method, unchanged. """ if self.default is not _NotSet: - problem = f'default={self.default!r}' + problem = f'already has default={self.default!r}' elif self.default_cb: - problem = f'default_cb={self.default_cb!r}' + problem = f'already has default_cb={self.default_cb!r}' + elif self.required: + problem = 'has required=True' else: problem = None if problem: - raise ParameterDefinitionError( - f'Cannot register a default callback method for {self} because it already has {problem}' - ) + raise ParameterDefinitionError(f'Cannot register a default callback method for {self} because it {problem}') self.default_cb = DefaultCallback(method, True) return method diff --git a/lib/cli_command_parser/parameters/options.py b/lib/cli_command_parser/parameters/options.py index ceabc55e..ec6f5c5e 100644 --- a/lib/cli_command_parser/parameters/options.py +++ b/lib/cli_command_parser/parameters/options.py @@ -436,7 +436,7 @@ class TriFlag(BaseFlag[B, D], actions=(StoreConst, AppendConst)): :param alt_short: The alternate short form to use. :param alt_help: The help text to display with the alternate option strings. :param action: The action to take on individual parsed values. Only ``store_const`` (the default) is supported. - :param default: The default value to use if neither the primary or alternate options are provided. Defaults + :param default: The default value to use if neither the primary nor alternate options are provided. Defaults to None. :param name_mode: Override the configured :ref:`configuration:Parsing Options:option_name_mode` for this TriFlag. :param type: A callable (function, class, etc.) that accepts a single string argument and returns a boolean value, @@ -495,16 +495,13 @@ def __init__( raise ParameterDefinitionError(msg) from e if default is _NotSet and default_cb is None: - if not kwargs.get('required', False): + if kwargs.get('required', False): + self._default_cb_ok = False # prevent late registration of a default callback when param is required + else: default = None # type: ignore[assignment] else: self._default_cb_ok = False - if default in consts: - raise ParameterDefinitionError( - f'Invalid {default=} with {consts=} - the default must not match either value' - ) - alt_opt_strs = (opt for opt in (alt_short, alt_long) if opt) super().__init__(*option_strs, *alt_opt_strs, action=action, default=default, default_cb=default_cb, **kwargs) self.consts = consts diff --git a/tests/test_parameters/test_flags.py b/tests/test_parameters/test_flags.py index 2061ab9a..d995677f 100755 --- a/tests/test_parameters/test_flags.py +++ b/tests/test_parameters/test_flags.py @@ -76,19 +76,19 @@ class Foo(Command): def test_nargs_not_allowed(self): with self.assertRaises(TypeError): - Flag(nargs='+') + Flag(nargs='+') # type: ignore def test_choices_not_allowed(self): with self.assertRaises(TypeError): - Flag(choices=(1, 2)) + Flag(choices=(1, 2)) # type: ignore def test_allow_leading_dash_not_allowed(self): with self.assertRaises(TypeError): - Flag(allow_leading_dash=True) + Flag(allow_leading_dash=True) # type: ignore def test_default_cb_rejected(self): with self.assert_raises_contains_str(ParameterDefinitionError, "The 'default_cb' arg is not supported"): - Flag(default_cb=lambda: 123) + Flag(default_cb=lambda: 123) # type: ignore # endregion @@ -229,15 +229,7 @@ def test_bad_consts(self): fail_cases = [({'consts': None}, exc), ({'consts': (1,)}, exc), ({'consts': [1, 2, 3]}, exc)] self.assert_call_fails_cases(TriFlag, fail_cases) - def test_default_in_consts_rejected(self): - expected = 'the default must not match either value' - cases = [((None, 'foo'), None), (('foo', None), None), ((True, False), True), ((True, False), False)] - for consts, default in cases: - with self.subTest(consts=consts, default=default): - with self.assert_raises_contains_str(ParameterDefinitionError, expected): - TriFlag(consts=consts, default=default) - - def test_register_default_cb_rejected(self): + def test_register_default_cb_rejected_due_to_existing_default(self): with self.assert_raises_contains_str(ParameterDefinitionError, 'because it already has default='): class Foo(Command): @@ -247,6 +239,16 @@ class Foo(Command): def baz(self): pass + def test_register_default_cb_rejected_due_to_required(self): + with self.assert_raises_contains_str(ParameterDefinitionError, 'because it has required=True'): + + class Foo(Command): + bar = TriFlag(required=True) + + @bar.register_default_cb + def baz(self): + pass + # region Option Strings def test_bad_alt_short(self): diff --git a/tests/test_parsing/test_parse_flags.py b/tests/test_parsing/test_parse_flags.py index f7a1dd93..9edc78e9 100755 --- a/tests/test_parsing/test_parse_flags.py +++ b/tests/test_parsing/test_parse_flags.py @@ -437,6 +437,20 @@ def _bar(self): ] self.assert_parse_results_cases(Cmd, success_cases) + def test_default_matching_const(self): + class Cmd(Command): + foo = TriFlag(default=False) + bar = TriFlag(consts=(False, True), default=True) + + success_cases = [ + ([], {'foo': False, 'bar': True}), + (['--foo'], {'foo': True, 'bar': True}), + (['--bar'], {'foo': False, 'bar': False}), + (['--no-foo'], {'foo': False, 'bar': True}), + (['--no-bar'], {'foo': False, 'bar': True}), + ] + self.assert_parse_results_cases(Cmd, success_cases) + # region Env Var Handling def test_env_var(self):