For boolean configuration options, add separate "--foo" and "--no-foo" CLI flags (#303).

borgmatic/commands/arguments.py

@@ -500,20 +500,26 @@ def add_arguments_from_schema(arguments_group, schema, unparsed_arguments, names
     # The ...=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)
-    full_flag_name = f"--{flag_name}"
 
-    # As a UX nicety, allow boolean options that have a default of false to have command-line flags
-    # without values.
-    if schema_type == 'boolean' and schema.get('default') is False:
+    # As a UX nicety, add separate true and false flags for boolean options.
+    if schema_type == 'boolean':
         arguments_group.add_argument(
-            full_flag_name,
+            f'--{flag_name}',
             action='store_true',
             default=None,
             help=description,
         )
+        no_flag_name = '.'.join(names[:-1] + ('no-' + names[-1],)).replace('_', '-')
+        arguments_group.add_argument(
+            f'--{no_flag_name}',
+            dest=flag_name.replace('-', '_'),
+            action='store_false',
+            default=None,
+            help=f'Set the --{flag_name} value to false.',
+        )
     else:
         arguments_group.add_argument(
-            full_flag_name,
+            f'--{flag_name}',
             type=argument_type,
             metavar=metavar,
             help=description,
@@ -553,9 +559,6 @@ def make_parsers(schema, unparsed_arguments):
         action='store_true',
         help='Go through the motions, but do not actually write to any repositories',
     )
-    global_group.add_argument(
-        '-nc', '--no-color', dest='no_color', action='store_true', help='Disable colored output'
-    )
     global_group.add_argument(
         '-v',
         '--verbosity',

borgmatic/commands/borgmatic.py

@@ -1025,7 +1025,7 @@ def main(extra_summary_logs=[]):  # pragma: no cover
     any_json_flags = any(
         getattr(sub_arguments, 'json', False) for sub_arguments in arguments.values()
     )
-    color_enabled = should_do_markup(global_arguments.no_color or any_json_flags, configs)
+    color_enabled = should_do_markup(configs, any_json_flags)
 
     try:
         configure_logging(

borgmatic/config/schema.yaml

@@ -59,7 +59,6 @@ properties:
                     description: |
                         Whether the repository should be created append-only,
                         only used for the repo-create action. Defaults to false.
-                    default: false
                     example: true
                 storage_quota:
                     type: string
@@ -74,7 +73,6 @@ properties:
                         Whether any missing parent directories of the repository
                         path should be created, only used for the repo-create
                         action. Defaults to false.
-                    default: false
                     example: true
         description: |
             A required list of local or remote repositories with paths and
@@ -104,14 +102,12 @@ properties:
         description: |
             Stay in same file system; do not cross mount points beyond the given
             source directories. Defaults to false.
-        default: false
         example: true
     numeric_ids:
         type: boolean
         description: |
             Only store/extract numeric user and group identifiers. Defaults to
             false.
-        default: false
         example: true
     atime:
         type: boolean
@@ -122,13 +118,11 @@ properties:
     ctime:
         type: boolean
         description: Store ctime into archive. Defaults to true.
-        default: true
         example: false
     birthtime:
         type: boolean
         description: |
             Store birthtime (creation date) into archive. Defaults to true.
-        default: true
         example: false
     read_special:
         type: boolean
@@ -138,14 +132,12 @@ properties:
             used when backing up special devices such as /dev/zero. Defaults to
             false. But when a database hook is used, the setting here is ignored
             and read_special is considered true.
-        default: false
         example: true
     flags:
         type: boolean
         description: |
             Record filesystem flags (e.g. NODUMP, IMMUTABLE) in archive.
             Defaults to true.
-        default: true
         example: false
     files_cache:
         type: string
@@ -219,7 +211,6 @@ properties:
             Exclude directories that contain a CACHEDIR.TAG file. See
             http://www.brynosaurus.com/cachedir/spec.html for details. Defaults
             to false.
-        default: false
         example: true
     exclude_if_present:
         type: array
@@ -236,13 +227,11 @@ properties:
             If true, the exclude_if_present filename is included in backups.
             Defaults to false, meaning that the exclude_if_present filename is
             omitted from backups.
-        default: false
         example: true
     exclude_nodump:
         type: boolean
         description: |
             Exclude files with the NODUMP flag. Defaults to false.
-        default: false
         example: true
     borgmatic_source_directory:
         type: string
@@ -274,7 +263,6 @@ properties:
         description: |
             If true, then source directories (and root pattern paths) must
             exist. If they don't, an error is raised. Defaults to false.
-        default: false
         example: true
     encryption_passcommand:
         type: string
@@ -488,21 +476,18 @@ properties:
         description: |
             Bypass Borg error about a repository that has been moved. Defaults
             to false.
-        default: false
         example: true
     unknown_unencrypted_repo_access_is_ok:
         type: boolean
         description: |
             Bypass Borg error about a previously unknown unencrypted repository.
             Defaults to false.
-        default: false
         example: true
     check_i_know_what_i_am_doing:
         type: boolean
         description: |
             Bypass Borg confirmation about check with repair option. Defaults to
             false and an interactive prompt from Borg.
-        default: false
         example: true
     extra_borg_options:
         type: object
@@ -828,9 +813,7 @@ properties:
     color:
         type: boolean
         description: |
-            Apply color to console output. Can be overridden with --no-color
-            command-line flag. Defaults to true.
-        default: true
+            Apply color to console output. Defaults to true.
         example: false
     progress:
         type: boolean
@@ -838,7 +821,6 @@ properties:
             Display progress as each file or archive is processed when running
             supported actions. Corresponds to the "--progress" flag on those
             actions. Defaults to false.
-        default: false
         example: true
     statistics:
         type: boolean
@@ -846,7 +828,6 @@ properties:
             Display statistics for an archive when running supported actions.
             Corresponds to the "--stats" flag on those actions. Defaults to
             false.
-        default: false
         example: true
     list_details:
         type: boolean
@@ -854,7 +835,6 @@ properties:
             Display details for each file or archive as it is processed when
             running supported actions. Corresponds to the "--list" flag on those
             actions. Defaults to false.
-        default: false
         example: true
     skip_actions:
         type: array
@@ -1197,7 +1177,6 @@ properties:
                     backup itself. Defaults to true. Changing this to false
                     prevents "borgmatic bootstrap" from extracting configuration
                     files from the backup.
-                default: true
                 example: false
         description: |
             Support for the "borgmatic bootstrap" action, used to extract
@@ -1282,7 +1261,6 @@ properties:
                         schema elements. These statements will fail unless the
                         initial connection to the database is made by a
                         superuser.
-                    default: false
                     example: true
                 format:
                     type: string
@@ -1521,7 +1499,6 @@ properties:
                         Use the "--add-drop-database" flag with mariadb-dump,
                         causing the database to be dropped right before restore.
                         Defaults to true.
-                    default: true
                     example: false
                 options:
                     type: string
@@ -1669,7 +1646,6 @@ properties:
                         Use the "--add-drop-database" flag with mysqldump,
                         causing the database to be dropped right before restore.
                         Defaults to true.
-                    default: true
                     example: false
                 options:
                     type: string
@@ -2397,9 +2373,8 @@ properties:
             send_logs:
                 type: boolean
                 description: |
-                    Send borgmatic logs to Apprise services as part the
+                    Send borgmatic logs to Apprise services as part of the
                     "finish", "fail", and "log" states. Defaults to true.
-                default: true
                 example: false
             logs_size_limit:
                 type: integer
@@ -2509,14 +2484,12 @@ properties:
                 description: |
                     Verify the TLS certificate of the ping URL host. Defaults to
                     true.
-                default: true
                 example: false
             send_logs:
                 type: boolean
                 description: |
-                    Send borgmatic logs to Healthchecks as part the "finish",
+                    Send borgmatic logs to Healthchecks as part of the "finish",
                     "fail", and "log" states. Defaults to true.
-                default: true
                 example: false
             ping_body_limit:
                 type: integer
@@ -2549,7 +2522,6 @@ properties:
                     the slug URL scheme (https://hc-ping.com/<ping-key>/<slug>
                     as opposed to https://hc-ping.com/<uuid>).
                     Defaults to false.
-                default: false
                 example: true
         description: |
             Configuration for a monitoring integration with Healthchecks. Create
@@ -2589,7 +2561,6 @@ properties:
                 description: |
                     Verify the TLS certificate of the push URL host. Defaults to
                     true.
-                default: true
                 example: false
         description: |
             Configuration for a monitoring integration with Uptime Kuma using
@@ -2626,7 +2597,6 @@ properties:
                 description: |
                     Send borgmatic logs to PagerDuty when a backup errors.
                     Defaults to true.
-                default: true
                 example: false
         description: |
             Configuration for a monitoring integration with PagerDuty. Create an

borgmatic/logger.py

@@ -29,12 +29,13 @@ def interactive_console():
     return sys.stderr.isatty() and os.environ.get('TERM') != 'dumb'
 
 
-def should_do_markup(no_color, configs):
+def should_do_markup(configs, json_enabled):
     '''
-    Given the value of the command-line no-color argument, and a dict of configuration filename to
-    corresponding parsed configuration, determine if we should enable color marking up.
+    Given a dict of configuration filename to corresponding parsed configuration (which already have
+    any command-line overrides applied) and whether json is enabled, determine if we should enable
+    color marking up.
     '''
-    if no_color:
+    if json_enabled:
         return False
 
     if any(config.get('color', True) is False for config in configs.values()):

tests/unit/commands/test_arguments.py

@@ -1121,7 +1121,7 @@ def test_add_arguments_from_schema_with_array_and_nested_object_adds_multiple_fl
     )
 
 
-def test_add_arguments_from_schema_with_default_false_boolean_adds_valueless_flag():
+def test_add_arguments_from_schema_with_boolean_adds_two_valueless_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(bool)
@@ -1131,32 +1131,12 @@ def test_add_arguments_from_schema_with_default_false_boolean_adds_valueless_fla
         default=None,
         help='help',
     ).once()
-    flexmock(module).should_receive('add_array_element_arguments')
-
-    module.add_arguments_from_schema(
-        arguments_group=arguments_group,
-        schema={
-            'type': 'object',
-            'properties': {
-                'foo': {
-                    'type': 'boolean',
-                    'default': False,
-                }
-            },
-        },
-        unparsed_arguments=(),
-    )
-
-
-def test_add_arguments_from_schema_with_default_true_boolean_adds_value_flag():
-    arguments_group = flexmock()
-    flexmock(module).should_receive('make_argument_description').and_return('help')
-    flexmock(module.borgmatic.config.schema).should_receive('parse_type').and_return(bool)
     arguments_group.should_receive('add_argument').with_args(
-        '--foo',
-        type=bool,
-        metavar='FOO',
-        help='help',
+        '--no-foo',
+        dest='foo',
+        action='store_false',
+        default=None,
+        help=object,
     ).once()
     flexmock(module).should_receive('add_array_element_arguments')
 
@@ -1167,7 +1147,6 @@ def test_add_arguments_from_schema_with_default_true_boolean_adds_value_flag():
             'properties': {
                 'foo': {
                     'type': 'boolean',
-                    'default': True,
                 }
             },
         },
@@ -1175,16 +1154,23 @@ def test_add_arguments_from_schema_with_default_true_boolean_adds_value_flag():
     )
 
 
-def test_add_arguments_from_schema_with_defaultless_boolean_adds_value_flag():
+def test_add_arguments_from_schema_with_nested_boolean_adds_two_valueless_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(bool)
     arguments_group.should_receive('add_argument').with_args(
-        '--foo',
-        type=bool,
-        metavar='FOO',
+        '--foo.bar.baz-quux',
+        action='store_true',
+        default=None,
         help='help',
     ).once()
+    arguments_group.should_receive('add_argument').with_args(
+        '--foo.bar.no-baz-quux',
+        dest='foo.bar.baz_quux',
+        action='store_false',
+        default=None,
+        help=object,
+    ).once()
     flexmock(module).should_receive('add_array_element_arguments')
 
     module.add_arguments_from_schema(
@@ -1192,12 +1178,13 @@ def test_add_arguments_from_schema_with_defaultless_boolean_adds_value_flag():
         schema={
             'type': 'object',
             'properties': {
-                'foo': {
+                'baz_quux': {
                     'type': 'boolean',
                 }
             },
         },
         unparsed_arguments=(),
+        names=('foo', 'bar'),
     )
 
 

tests/unit/test_logger.py

@@ -44,19 +44,23 @@ def test_interactive_console_true_when_isatty_and_TERM_is_not_dumb(capsys):
         assert module.interactive_console() is True
 
 
-def test_should_do_markup_respects_no_color_value():
-    flexmock(module.os.environ).should_receive('get').and_return(None)
+def test_should_do_markup_respects_json_enabled_value():
+    flexmock(module.os.environ).should_receive('get').never()
     flexmock(module).should_receive('interactive_console').never()
-    assert module.should_do_markup(no_color=True, configs={}) is False
+    assert module.should_do_markup(configs={}, json_enabled=True) is False
 
 
 def test_should_do_markup_respects_config_value():
     flexmock(module.os.environ).should_receive('get').and_return(None)
     flexmock(module).should_receive('interactive_console').never()
-    assert module.should_do_markup(no_color=False, configs={'foo.yaml': {'color': False}}) is False
+    assert (
+        module.should_do_markup(configs={'foo.yaml': {'color': False}}, json_enabled=False) is False
+    )
 
     flexmock(module).should_receive('interactive_console').and_return(True).once()
-    assert module.should_do_markup(no_color=False, configs={'foo.yaml': {'color': True}}) is True
+    assert (
+        module.should_do_markup(configs={'foo.yaml': {'color': True}}, json_enabled=False) is True
+    )
 
 
 def test_should_do_markup_prefers_any_false_config_value():
@@ -65,11 +69,11 @@ def test_should_do_markup_prefers_any_false_config_value():
 
     assert (
         module.should_do_markup(
-            no_color=False,
             configs={
                 'foo.yaml': {'color': True},
                 'bar.yaml': {'color': False},
             },
+            json_enabled=False,
         )
         is False
     )
@@ -83,14 +87,16 @@ def test_should_do_markup_respects_PY_COLORS_environment_variable():
 
     flexmock(module).should_receive('to_bool').and_return(True)
 
-    assert module.should_do_markup(no_color=False, configs={}) is True
+    assert module.should_do_markup(configs={}, json_enabled=False) is True
 
 
-def test_should_do_markup_prefers_no_color_value_to_config_value():
+def test_should_do_markup_prefers_json_enabled_value_to_config_value():
     flexmock(module.os.environ).should_receive('get').and_return(None)
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=True, configs={'foo.yaml': {'color': True}}) is False
+    assert (
+        module.should_do_markup(configs={'foo.yaml': {'color': True}}, json_enabled=True) is False
+    )
 
 
 def test_should_do_markup_prefers_config_value_to_environment_variables():
@@ -98,7 +104,9 @@ def test_should_do_markup_prefers_config_value_to_environment_variables():
     flexmock(module).should_receive('to_bool').and_return(True)
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=False, configs={'foo.yaml': {'color': False}}) is False
+    assert (
+        module.should_do_markup(configs={'foo.yaml': {'color': False}}, json_enabled=False) is False
+    )
 
 
 def test_should_do_markup_prefers_no_color_value_to_environment_variables():
@@ -106,14 +114,14 @@ def test_should_do_markup_prefers_no_color_value_to_environment_variables():
     flexmock(module).should_receive('to_bool').and_return(True)
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=True, configs={}) is False
+    assert module.should_do_markup(configs={}, json_enabled=False) is False
 
 
 def test_should_do_markup_respects_interactive_console_value():
     flexmock(module.os.environ).should_receive('get').and_return(None)
     flexmock(module).should_receive('interactive_console').and_return(True)
 
-    assert module.should_do_markup(no_color=False, configs={}) is True
+    assert module.should_do_markup(configs={}, json_enabled=False) is True
 
 
 def test_should_do_markup_prefers_PY_COLORS_to_interactive_console_value():
@@ -124,7 +132,7 @@ def test_should_do_markup_prefers_PY_COLORS_to_interactive_console_value():
     flexmock(module).should_receive('to_bool').and_return(True)
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=False, configs={}) is True
+    assert module.should_do_markup(configs={}, json_enabled=False) is True
 
 
 def test_should_do_markup_prefers_NO_COLOR_to_interactive_console_value():
@@ -132,7 +140,7 @@ def test_should_do_markup_prefers_NO_COLOR_to_interactive_console_value():
     flexmock(module.os.environ).should_receive('get').with_args('NO_COLOR', None).and_return('True')
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=False, configs={}) is False
+    assert module.should_do_markup(configs={}, json_enabled=False) is False
 
 
 def test_should_do_markup_respects_NO_COLOR_environment_variable():
@@ -140,7 +148,7 @@ def test_should_do_markup_respects_NO_COLOR_environment_variable():
     flexmock(module.os.environ).should_receive('get').with_args('PY_COLORS', None).and_return(None)
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=False, configs={}) is False
+    assert module.should_do_markup(configs={}, json_enabled=False) is False
 
 
 def test_should_do_markup_ignores_empty_NO_COLOR_environment_variable():
@@ -148,7 +156,7 @@ def test_should_do_markup_ignores_empty_NO_COLOR_environment_variable():
     flexmock(module.os.environ).should_receive('get').with_args('PY_COLORS', None).and_return(None)
     flexmock(module).should_receive('interactive_console').and_return(True)
 
-    assert module.should_do_markup(no_color=False, configs={}) is True
+    assert module.should_do_markup(configs={}, json_enabled=False) is True
 
 
 def test_should_do_markup_prefers_NO_COLOR_to_PY_COLORS():
@@ -160,7 +168,7 @@ def test_should_do_markup_prefers_NO_COLOR_to_PY_COLORS():
     )
     flexmock(module).should_receive('interactive_console').never()
 
-    assert module.should_do_markup(no_color=False, configs={}) is False
+    assert module.should_do_markup(configs={}, json_enabled=False) is False
 
 
 def test_multi_stream_handler_logs_to_handler_for_log_level():