From 79bf641668bcf4ea438d670c8e7cebc28ab75086 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Thu, 27 Mar 2025 12:42:49 -0700 Subject: [PATCH] Set the action type when cloning an argument for a list index flag (#303). --- borgmatic/commands/arguments.py | 13 +++- tests/integration/commands/test_arguments.py | 2 + tests/unit/commands/test_arguments.py | 69 ++++++++++++++++---- 3 files changed, 71 insertions(+), 13 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 8a92efb9..8e0a7d85 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -362,9 +362,8 @@ def add_array_element_arguments(arguments_group, unparsed_arguments, flag_name): pattern = re.compile(fr'^--{flag_name.replace("[0]", r"\[\d+\]").replace(".", r"\.")}$') - # Find an existing list index flag (and its action) corresponding to the given flag name. If one - # isn't found, bail. try: + # Find an existing list index flag (and its action) corresponding to the given flag name. (argument_action, existing_flag_name) = next( (action, action_flag_name) for action in arguments_group._group_actions @@ -372,6 +371,15 @@ def add_array_element_arguments(arguments_group, unparsed_arguments, flag_name): if pattern.match(action_flag_name) if f'--{flag_name}'.startswith(action_flag_name) ) + + # Based on the type of the action (e.g. argparse._StoreTrueAction), look up the corresponding + # action registry name (e.g., "store_true") to pass to add_argument(action=...) below. + action_registry_name = next( + registry_name + for registry_name, action_type in arguments_group._registries['action'].items() + # Not using isinstance() here because we only want an exact match—no parent classes. + if type(argument_action) == action_type + ) except StopIteration: return @@ -383,6 +391,7 @@ def add_array_element_arguments(arguments_group, unparsed_arguments, flag_name): arguments_group.add_argument( unparsed_flag_name, + action=action_registry_name, choices=argument_action.choices, default=argument_action.default, dest=unparsed_flag_name.lstrip('-'), diff --git a/tests/integration/commands/test_arguments.py b/tests/integration/commands/test_arguments.py index 5f539660..d6841a2c 100644 --- a/tests/integration/commands/test_arguments.py +++ b/tests/integration/commands/test_arguments.py @@ -20,11 +20,13 @@ def test_add_array_element_arguments_adds_arguments_for_array_index_flags(): arguments_group = parser.add_argument_group('arguments') arguments_group.add_argument( '--foo[0].val', + action='store_true', dest='--foo[0].val', ) flexmock(arguments_group).should_receive('add_argument').with_args( '--foo[25].val', + action='store_true', choices=object, default=object, dest='foo[25].val', diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 379dba3b..a9479db5 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -672,13 +672,31 @@ def test_add_array_element_arguments_without_any_flags_bails(): ) +# Use this instead of a flexmock because it's not easy to check the type() of a flexmock instance. +Group_action = collections.namedtuple( + 'Group_action', + ( + 'option_strings', + 'choices', + 'default', + 'nargs', + 'required', + 'type', + ), + defaults=( + flexmock(), flexmock(), flexmock(), flexmock(), flexmock(), + ) +) + + def test_add_array_element_arguments_without_array_index_flags_bails(): arguments_group = flexmock( _group_actions=( - flexmock( + Group_action( option_strings=('--foo[0].val',), ), - ) + ), + _registries={'action': {'store_stuff': Group_action}}, ) arguments_group.should_receive('add_argument').never() @@ -692,10 +710,11 @@ def test_add_array_element_arguments_without_array_index_flags_bails(): def test_add_array_element_arguments_with_non_matching_array_index_flags_bails(): arguments_group = flexmock( _group_actions=( - flexmock( + Group_action( option_strings=('--foo[0].val',), ), - ) + ), + _registries={'action': {'store_stuff': Group_action}}, ) arguments_group.should_receive('add_argument').never() @@ -709,10 +728,11 @@ def test_add_array_element_arguments_with_non_matching_array_index_flags_bails() def test_add_array_element_arguments_with_identical_array_index_flag_bails(): arguments_group = flexmock( _group_actions=( - flexmock( + Group_action( option_strings=('--foo[0].val',), ), - ) + ), + _registries={'action': {'store_stuff': Group_action}}, ) arguments_group.should_receive('add_argument').never() @@ -723,10 +743,10 @@ def test_add_array_element_arguments_with_identical_array_index_flag_bails(): ) -def test_add_array_element_arguments_adds_arguments_for_array_index_flags(): +def test_add_array_element_arguments_without_action_type_in_registry_bails(): arguments_group = flexmock( _group_actions=( - flexmock( + Group_action( option_strings=('--foo[0].val',), choices=flexmock(), default=flexmock(), @@ -734,10 +754,35 @@ def test_add_array_element_arguments_adds_arguments_for_array_index_flags(): required=flexmock(), type=flexmock(), ), - ) + ), + _registries={'action': {'store_stuff': bool}}, + ) + arguments_group.should_receive('add_argument').never() + + module.add_array_element_arguments( + arguments_group=arguments_group, + unparsed_arguments=('--foo[25].val', 'fooval', '--bar[1].val', 'barval'), + flag_name='foo[0].val', + ) + + +def test_add_array_element_arguments_adds_arguments_for_array_index_flags(): + arguments_group = flexmock( + _group_actions=( + Group_action( + option_strings=('--foo[0].val',), + choices=flexmock(), + default=flexmock(), + nargs=flexmock(), + required=flexmock(), + type=flexmock(), + ), + ), + _registries={'action': {'store_stuff': Group_action}}, ) arguments_group.should_receive('add_argument').with_args( '--foo[25].val', + action='store_stuff', choices=object, default=object, dest='foo[25].val', @@ -756,7 +801,7 @@ def test_add_array_element_arguments_adds_arguments_for_array_index_flags(): def test_add_array_element_arguments_adds_arguments_for_array_index_flags_with_equals_sign(): arguments_group = flexmock( _group_actions=( - flexmock( + Group_action( option_strings=('--foo[0].val',), choices=flexmock(), default=flexmock(), @@ -764,10 +809,12 @@ def test_add_array_element_arguments_adds_arguments_for_array_index_flags_with_e required=flexmock(), type=flexmock(), ), - ) + ), + _registries={'action': {'store_stuff': Group_action}}, ) arguments_group.should_receive('add_argument').with_args( '--foo[25].val', + action='store_stuff', choices=object, default=object, dest='foo[25].val',