From e9ebe64672cd6f64b622dc669e414a40f0eb168e Mon Sep 17 00:00:00 2001 From: DanCardin Date: Mon, 16 Sep 2024 17:58:40 -0400 Subject: [PATCH 1/2] refactor: Pull handling of `--no-*` bool arguments out of the parser. --- src/cappa/arg.py | 33 ++++++++++++++++++++++++++++++++- src/cappa/argparse.py | 5 ----- src/cappa/parser.py | 7 +------ 3 files changed, 33 insertions(+), 12 deletions(-) diff --git a/src/cappa/arg.py b/src/cappa/arg.py index cab3aa1..4c58f4f 100644 --- a/src/cappa/arg.py +++ b/src/cappa/arg.py @@ -64,6 +64,10 @@ def value_actions(cls) -> typing.Set[ArgAction]: def is_custom(cls, action: ArgAction | Callable | None): return action is not None and not isinstance(action, ArgAction) + @property + def is_bool_action(self): + return self in {self.store_true, self.store_false} + @dataclasses.dataclass(order=True) class Group: @@ -192,7 +196,7 @@ def collect( ) result.append(normalized_arg) - return result + return list(explode_negated_bool_args(result)) def normalize( self, @@ -606,6 +610,33 @@ def infer_value_name(arg: Arg, field_name: str, num_args: int | None) -> str: return field_name +def explode_negated_bool_args(args: typing.Sequence[Arg]) -> typing.Iterable[Arg]: + """Expand `--foo/--no-foo` solo arguments into dual-arguments. + + Acts as a transform from `Arg(long='--foo/--no-foo')` to + `Annotated[Arg(long='--foo', action=ArgAction.store_true), Arg(long='--no-foo', action=ArgAction.store_false)]` + """ + for arg in args: + yielded = False + if isinstance(arg.action, ArgAction) and arg.action.is_bool_action and arg.long: + long = typing.cast(list[str], arg.long) + + negatives = [item for item in long if "--no-" in item] + positives = [item for item in long if "--no-" not in item] + positive_arg = dataclasses.replace( + arg, long=positives, action=ArgAction.store_true + ) + negative_arg = dataclasses.replace( + arg, long=negatives, action=ArgAction.store_false + ) + yield positive_arg + yield negative_arg + yielded = True + + if not yielded: + yield arg + + no_extra_arg_actions = { ArgAction.store_true, ArgAction.store_false, diff --git a/src/cappa/argparse.py b/src/cappa/argparse.py index e6b5608..c6b2eb0 100644 --- a/src/cappa/argparse.py +++ b/src/cappa/argparse.py @@ -342,11 +342,6 @@ def join_help(*segments): def get_action(arg: Arg) -> argparse.Action | type[argparse.Action] | str: action = arg.action if isinstance(action, ArgAction): - if action in {ArgAction.store_true, ArgAction.store_false}: - long = assert_type(arg.long, list) - has_no_option = any("--no-" in i for i in long) - if has_no_option: - return BooleanOptionalAction return action.value action = typing.cast(Callable, action) diff --git a/src/cappa/parser.py b/src/cappa/parser.py index 41227cd..325d0af 100644 --- a/src/cappa/parser.py +++ b/src/cappa/parser.py @@ -625,12 +625,7 @@ class Value(typing.Generic[T]): def store_bool(val: bool): - def store(arg: Arg, option: RawOption): - long = assert_type(arg.long, list) - has_no_option = any("--no-" in i for i in long) - if has_no_option: - return not option.name.startswith("--no-") - + def store(): return val return store From 6de0b62e5df761cf48c7c5b8077f5306997fc2bd Mon Sep 17 00:00:00 2001 From: DanCardin Date: Mon, 16 Sep 2024 17:58:56 -0400 Subject: [PATCH 2/2] fix: Only apply `--no-*` handling when there is both a positive and negative variant. --- CHANGELOG.md | 2 ++ pyproject.toml | 2 +- src/cappa/arg.py | 21 +++++++++++---------- src/cappa/argparse.py | 16 ---------------- src/cappa/parser.py | 15 ++++++++------- tests/arg/test_bool.py | 26 ++++++++++++++++++++++++++ 6 files changed, 48 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f8cacc..d652b1c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### 0.22.5 - fix: Refactor parser combinators into dedicated module, and document the behavior more thoroughly. +- refactor: Pull handling of `--no-*` bool arguments out of the parser +- fix: Only apply `--no-*` handling when there is both a positive and negative variant ### 0.22.4 diff --git a/pyproject.toml b/pyproject.toml index 6e17220..d5f228b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "cappa" -version = "0.22.4" +version = "0.22.5" description = "Declarative CLI argument parser." urls = {repository = "https://github.com/dancardin/cappa"} diff --git a/src/cappa/arg.py b/src/cappa/arg.py index 4c58f4f..8be2dac 100644 --- a/src/cappa/arg.py +++ b/src/cappa/arg.py @@ -619,19 +619,20 @@ def explode_negated_bool_args(args: typing.Sequence[Arg]) -> typing.Iterable[Arg for arg in args: yielded = False if isinstance(arg.action, ArgAction) and arg.action.is_bool_action and arg.long: - long = typing.cast(list[str], arg.long) + long = typing.cast(typing.List[str], arg.long) negatives = [item for item in long if "--no-" in item] positives = [item for item in long if "--no-" not in item] - positive_arg = dataclasses.replace( - arg, long=positives, action=ArgAction.store_true - ) - negative_arg = dataclasses.replace( - arg, long=negatives, action=ArgAction.store_false - ) - yield positive_arg - yield negative_arg - yielded = True + if negatives and positives: + positive_arg = dataclasses.replace( + arg, long=positives, action=ArgAction.store_true + ) + negative_arg = dataclasses.replace( + arg, long=negatives, action=ArgAction.store_false + ) + yield positive_arg + yield negative_arg + yielded = True if not yielded: yield arg diff --git a/src/cappa/argparse.py b/src/cappa/argparse.py index c6b2eb0..1adb9b2 100644 --- a/src/cappa/argparse.py +++ b/src/cappa/argparse.py @@ -93,22 +93,6 @@ def print_help(self, file=None): raise HelpExit(self.command.help_formatter(self.command, self.prog)) -class BooleanOptionalAction(argparse.Action): - """Simplified backport of same-named class from 3.9 onward. - - We know more about the called context here, and thus need much less of the - logic. Also, we support 3.8, which does not have the original class, so we - couldn't use it anyway. - """ - - def __init__(self, **kwargs): - super().__init__(nargs=0, **kwargs) - - def __call__(self, parser, namespace, values, option_string=None): - assert isinstance(option_string, str) - setattr(namespace, self.dest, not option_string.startswith("--no-")) - - def custom_action(arg: Arg, action: Callable): class CustomAction(argparse.Action): def __call__( # type: ignore diff --git a/src/cappa/parser.py b/src/cappa/parser.py index 325d0af..b94fdcd 100644 --- a/src/cappa/parser.py +++ b/src/cappa/parser.py @@ -624,11 +624,12 @@ class Value(typing.Generic[T]): value: T -def store_bool(val: bool): - def store(): - return val +def store_true(): + return True - return store + +def store_false(): + return False def store_count(context: ParseContext, arg: Arg): @@ -645,13 +646,13 @@ def store_append(context: ParseContext, arg: Arg, value: Value[typing.Any]): return result -process_options = { +process_options: dict[ArgAction, typing.Callable] = { ArgAction.help: HelpAction.from_context, ArgAction.version: VersionAction.from_arg, ArgAction.completion: CompletionAction.from_value, ArgAction.set: store_set, - ArgAction.store_true: store_bool(True), - ArgAction.store_false: store_bool(False), + ArgAction.store_true: store_true, + ArgAction.store_false: store_false, ArgAction.count: store_count, ArgAction.append: store_append, } diff --git a/tests/arg/test_bool.py b/tests/arg/test_bool.py index 7f401de..1c83f31 100644 --- a/tests/arg/test_bool.py +++ b/tests/arg/test_bool.py @@ -168,3 +168,29 @@ class ArgTest: with patch("os.environ", new={"ENV_DEFAULT": "1"}): test = parse(ArgTest, "--env-default", backend=backend) assert test.env_default is True + + +@backends +def test_sole_no_arg(backend): + @dataclass + class ArgTest: + no_dry_run: bool = False + + test = parse(ArgTest, backend=backend) + assert test.no_dry_run is False + + test = parse(ArgTest, "--no-dry-run", backend=backend) + assert test.no_dry_run is True + + +@backends +def test_sole_no_arg_inverted(backend): + @dataclass + class ArgTest: + no_dry_run: bool = True + + test = parse(ArgTest, backend=backend) + assert test.no_dry_run is True + + test = parse(ArgTest, "--no-dry-run", backend=backend) + assert test.no_dry_run is False