From d2c3ed26a90877c79fca6756ae9bf0b0a3c3ac26 Mon Sep 17 00:00:00 2001 From: Dan Helfman Date: Wed, 2 Apr 2025 23:15:21 -0700 Subject: [PATCH] Make a CLI flag for any config option that's a list of scalars (#303). --- borgmatic/commands/arguments.py | 26 +++++---- borgmatic/config/arguments.py | 16 +++--- tests/unit/commands/test_arguments.py | 79 +++++++++++++++++++++------ tests/unit/config/test_arguments.py | 7 +++ 4 files changed, 93 insertions(+), 35 deletions(-) diff --git a/borgmatic/commands/arguments.py b/borgmatic/commands/arguments.py index 2e71ed1d2..c2ebbd885 100644 --- a/borgmatic/commands/arguments.py +++ b/borgmatic/commands/arguments.py @@ -300,12 +300,12 @@ def make_argument_description(schema, flag_name): description = schema.get('description') schema_type = schema.get('type') example = schema.get('example') - - if not description: - return None + pieces = [description] if description else [] if '[0]' in flag_name: - description += ' To specify a different list element, replace the "[0]" with another array index ("[1]", "[2]", etc.).' + pieces.append( + ' To specify a different list element, replace the "[0]" with another array index ("[1]", "[2]", etc.).' + ) if example and schema_type in ('array', 'object'): example_buffer = io.StringIO() @@ -313,11 +313,9 @@ def make_argument_description(schema, flag_name): yaml.default_flow_style = True yaml.dump(example, example_buffer) - description += f' Example value: "{example_buffer.getvalue().strip()}"' + pieces.append(f'Example value: "{example_buffer.getvalue().strip()}"') - description = description.replace('%', '%%') - - return description + return ' '.join(pieces).replace('%', '%%') def add_array_element_arguments(arguments_group, unparsed_arguments, flag_name): @@ -476,7 +474,8 @@ def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names # If this is an "array" type, recurse for each items type child option. Don't return yet so that # a flag also gets added below for the array itself. if schema_type == 'array': - properties = borgmatic.config.schema.get_properties(schema.get('items', {})) + items = schema.get('items', {}) + properties = borgmatic.config.schema.get_properties(items) if properties: for name, child in properties.items(): @@ -486,6 +485,11 @@ def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names unparsed_arguments, names[:-1] + (f'{names[-1]}[0]',) + (name,), ) + # If there aren't any children, then this is an array of scalars. Recurse accordingly. + else: + add_arguments_from_schema( + arguments_group, items, unparsed_arguments, names[:-1] + (f'{names[-1]}[0]',) + ) flag_name = '.'.join(names).replace('_', '-') @@ -497,8 +501,8 @@ def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names metavar = names[-1].upper() description = make_argument_description(schema, flag_name) - # The ...=str given here is to support specifying an object or an array as a YAML string on the - # command-line. + # The object=str and array=str given here is to support specifying an object or an array as a + # YAML string on the command-line. argument_type = borgmatic.config.schema.parse_type(schema_type, object=str, array=str) # As a UX nicety, add separate true and false flags for boolean options. diff --git a/borgmatic/config/arguments.py b/borgmatic/config/arguments.py index 3252d779f..e23587d73 100644 --- a/borgmatic/config/arguments.py +++ b/borgmatic/config/arguments.py @@ -36,15 +36,15 @@ def set_values(config, keys, value): list_key = match.group('list_name') list_index = int(match.group('index')) - if len(keys) == 1: - config[list_key][list_index] = value - - return - - if list_key not in config: - config[list_key] = [] - try: + if len(keys) == 1: + config[list_key][list_index] = value + + return + + if list_key not in config: + config[list_key] = [] + set_values(config[list_key][list_index], keys[1:], value) except IndexError: raise ValueError(f'Argument list index {first_key} is out of range') diff --git a/tests/unit/commands/test_arguments.py b/tests/unit/commands/test_arguments.py index 51a647614..efc437b35 100644 --- a/tests/unit/commands/test_arguments.py +++ b/tests/unit/commands/test_arguments.py @@ -577,19 +577,6 @@ def test_parse_arguments_for_actions_raises_error_when_no_action_is_specified(): module.parse_arguments_for_actions(('config',), action_parsers, global_parser) -def test_make_argument_description_without_description_bails(): - assert ( - module.make_argument_description( - schema={ - 'description': None, - 'type': 'not yours', - }, - flag_name='flag', - ) - is None - ) - - def test_make_argument_description_with_object_adds_example(): buffer = flexmock() buffer.should_receive('getvalue').and_return('{foo: example}') @@ -611,6 +598,26 @@ def test_make_argument_description_with_object_adds_example(): ) +def test_make_argument_description_without_description_and_with_object_sets_example(): + buffer = flexmock() + buffer.should_receive('getvalue').and_return('{foo: example}') + flexmock(module.io).should_receive('StringIO').and_return(buffer) + yaml = flexmock() + yaml.should_receive('dump') + flexmock(module.ruamel.yaml).should_receive('YAML').and_return(yaml) + + assert ( + module.make_argument_description( + schema={ + 'type': 'object', + 'example': {'foo': 'example'}, + }, + flag_name='flag', + ) + == 'Example value: "{foo: example}"' + ) + + def test_make_argument_description_with_object_skips_missing_example(): flexmock(module.ruamel.yaml).should_receive('YAML').never() @@ -647,6 +654,26 @@ def test_make_argument_description_with_array_adds_example(): ) +def test_make_argument_description_without_description_and_with_array_sets_example(): + buffer = flexmock() + buffer.should_receive('getvalue').and_return('[example]') + flexmock(module.io).should_receive('StringIO').and_return(buffer) + yaml = flexmock() + yaml.should_receive('dump') + flexmock(module.ruamel.yaml).should_receive('YAML').and_return(yaml) + + assert ( + module.make_argument_description( + schema={ + 'type': 'array', + 'example': ['example'], + }, + flag_name='flag', + ) + == 'Example value: "[example]"' + ) + + def test_make_argument_description_with_array_skips_missing_example(): flexmock(module.ruamel.yaml).should_receive('YAML').never() @@ -672,6 +699,15 @@ def test_make_argument_description_with_array_index_in_flag_name_adds_to_descrip ) +def test_make_argument_description_without_description_and_with_array_index_in_flag_name_sets_description(): + assert 'list element' in module.make_argument_description( + schema={ + 'type': 'something', + }, + flag_name='flag[0]', + ) + + def test_make_argument_description_escapes_percent_character(): assert ( module.make_argument_description( @@ -1043,10 +1079,21 @@ def test_add_arguments_from_schema_with_propertyless_option_adds_flag(): ) -def test_add_arguments_from_schema_with_array_adds_flag(): +def test_add_arguments_from_schema_with_array_of_scalars_adds_multiple_flags(): arguments_group = flexmock() flexmock(module).should_receive('make_argument_description').and_return('help') - flexmock(module.borgmatic.config.schema).should_receive('parse_type').and_return(str) + flexmock(module.borgmatic.config.schema).should_receive('parse_type').with_args( + 'integer', object=str, array=str + ).and_return(int) + flexmock(module.borgmatic.config.schema).should_receive('parse_type').with_args( + 'array', object=str, array=str + ).and_return(str) + arguments_group.should_receive('add_argument').with_args( + '--foo[0]', + type=int, + metavar='FOO[0]', + help='help', + ).once() arguments_group.should_receive('add_argument').with_args( '--foo', type=str, @@ -1072,7 +1119,7 @@ def test_add_arguments_from_schema_with_array_adds_flag(): ) -def test_add_arguments_from_schema_with_array_and_nested_object_adds_multiple_flags(): +def test_add_arguments_from_schema_with_array_of_objects_adds_multiple_flags(): arguments_group = flexmock() flexmock(module).should_receive('make_argument_description').and_return('help 1').and_return( 'help 2' diff --git a/tests/unit/config/test_arguments.py b/tests/unit/config/test_arguments.py index 2c07f8156..79f0236a8 100644 --- a/tests/unit/config/test_arguments.py +++ b/tests/unit/config/test_arguments.py @@ -50,6 +50,13 @@ def test_set_values_with_list_index_key_out_of_range_raises(): module.set_values(config=config, keys=('foo', 'bar[1]', 'baz'), value=5) +def test_set_values_with_final_list_index_key_out_of_range_raises(): + config = {'foo': {'bar': [{'option': 'value'}]}} + + with pytest.raises(ValueError): + module.set_values(config=config, keys=('foo', 'bar[1]'), value=5) + + def test_set_values_with_list_index_key_missing_list_and_out_of_range_raises(): config = {'other': 'value'}