diff --git a/configargparse.py b/configargparse.py index a64d0a0..3c74014 100644 --- a/configargparse.py +++ b/configargparse.py @@ -1717,9 +1717,11 @@ def get_items_for_config_file_output(self, source_to_settings, parsed_namespace) ): value = getattr(parsed_namespace, action.dest, None) if value is not None: - if isinstance(value, bool): - value = str(value).lower() - config_file_items[config_file_keys[0]] = value + config_file_items[config_file_keys[0]] = ( + _convert_parsed_value_to_config_file_value( + action, value + ) + ) elif source == _ENV_VAR_SOURCE_KEY: for key, (action, value) in settings.items(): @@ -1727,7 +1729,11 @@ def get_items_for_config_file_output(self, source_to_settings, parsed_namespace) if config_file_keys: value = getattr(parsed_namespace, action.dest, None) if value is not None: - config_file_items[config_file_keys[0]] = value + config_file_items[config_file_keys[0]] = ( + _convert_parsed_value_to_config_file_value( + action, value + ) + ) elif source.startswith(_CONFIG_FILE_SOURCE_KEY): for key, (action, value) in settings.items(): config_file_items[key] = value @@ -2235,6 +2241,31 @@ def _dispatches_to_subparsers(action): ) and isinstance(getattr(action, "choices", None), dict) +def _convert_parsed_value_to_config_file_value(action, value): + """Convert a parsed arg value to the value a config file should hold for it. + + For a flag (``store_true``, ``store_false`` or ``store_const``), the config + file value says whether to invoke the flag, not what it stores, so it is + ``"true"`` exactly when the parsed value is the one the flag stores. Other + booleans are written in lowercase too, so values from the command line and + from environment variables are written the same way. + + Args: + action: the action the value was parsed for. + value: the value the parser stored for the action. + + Returns: + object: ``"true"`` or ``"false"`` for flags and booleans, otherwise the + value unchanged. + """ + if isinstance(action, argparse._StoreConstAction): + # "is" rather than "==": a store_false default of 0 equals False + value = value is action.const + if isinstance(value, bool): + value = str(value).lower() + return value + + def already_on_command_line( existing_args_list, potential_command_line_args, prefix_chars ): diff --git a/tests/test_configargparse.py b/tests/test_configargparse.py index 52f37dc..e7b3f1c 100644 --- a/tests/test_configargparse.py +++ b/tests/test_configargparse.py @@ -1411,7 +1411,7 @@ def testConstructor_WriteOutConfigFileArgs2(self): cfg_f.seek(0) expected_config_file_contents = "config-file-settable-list = [a, b, c, d]\n" expected_config_file_contents += "arg1 = 10\n" - expected_config_file_contents += "config-file-settable-flag = True\n" + expected_config_file_contents += "config-file-settable-flag = true\n" expected_config_file_contents += "arg3 = bla3\n" expected_config_file_contents += "arg4 = bla4\n" expected_config_file_contents += "arg2 = 3\n" @@ -4183,6 +4183,150 @@ def __fspath__(self_inner): self.assertGreater(len(help_text), 0) +class TestStoreFalseConfigOutput(unittest.TestCase): + def make_parser(self, default=True): + parser = configargparse.ArgumentParser() + parser.add_argument( + "-n", + "--no-feature", + "--disable-feature", + action="store_false", + dest="feature", + default=default, + env_var="CONFIGARGPARSE_TEST_NO_FEATURE", + ) + parser.add_argument("--label", env_var="CONFIGARGPARSE_TEST_LABEL") + return parser + + def assert_round_trip(self, parser, namespace, reader): + with tempfile.TemporaryDirectory() as directory: + path = os.path.join(directory, "settings.conf") + with captured_output(): + parser.write_config_file(namespace, [path]) + with open(path) as config_file: + contents = config_file.read() + restored = reader.parse_args([], config_file_contents=contents, env_vars={}) + self.assertEqual(vars(restored), vars(namespace)) + # assertEqual alone would accept 0 coming back as False + for name, value in vars(namespace).items(): + self.assertIs(type(getattr(restored, name)), type(value), name) + return contents + + def test_store_false_command_line_round_trip(self): + for default in (True, False): + for option in ("-n", "--no-feature", "--disable-feature"): + with self.subTest(default=default, option=option): + parser = self.make_parser(default) + namespace = parser.parse_args([option], env_vars={}) + self.assertFalse(namespace.feature) + self.assert_round_trip(parser, namespace, self.make_parser(default)) + + def test_store_false_environment_round_trip(self): + for default in (True, False): + for value in ("true", "false"): + with self.subTest(default=default, value=value): + parser = self.make_parser(default) + with mock.patch.dict( + os.environ, + { + "CONFIGARGPARSE_TEST_NO_FEATURE": value, + "CONFIGARGPARSE_TEST_LABEL": "example", + }, + ): + namespace = parser.parse_args([]) + self.assertEqual(namespace.feature, default and value == "false") + self.assertEqual(namespace.label, "example") + self.assert_round_trip(parser, namespace, self.make_parser(default)) + + def test_store_false_config_round_trip(self): + for default in (True, False): + for value in ("true", "false"): + with self.subTest(default=default, value=value): + parser = self.make_parser(default) + namespace = parser.parse_args( + [], config_file_contents="no-feature = " + value, env_vars={} + ) + self.assert_round_trip(parser, namespace, self.make_parser(default)) + + def test_non_boolean_store_false_default_round_trip(self): + # 1 and "yes" are truthy, so writing them as-is would invoke the flag + for default in (0, 1, "yes"): + with self.subTest(default=default): + parser = self.make_parser(default) + namespace = parser.parse_args( + [], + env_vars={ + "CONFIGARGPARSE_TEST_NO_FEATURE": "false", + "CONFIGARGPARSE_TEST_LABEL": "example", + }, + ) + self.assertIs(namespace.feature, default) + self.assert_round_trip(parser, namespace, self.make_parser(default)) + + def test_store_false_defaults_are_not_added(self): + for default in (True, False): + with self.subTest(default=default): + parser = self.make_parser(default) + namespace = parser.parse_args([], env_vars={}) + contents = self.assert_round_trip( + parser, namespace, self.make_parser(default) + ) + self.assertEqual(contents, "") + + def test_other_boolean_actions_round_trip(self): + actions = [("store_true", False, ["--feature"])] + if hasattr(argparse, "BooleanOptionalAction"): + actions += [ + (argparse.BooleanOptionalAction, True, ["--no-feature"]), + (argparse.BooleanOptionalAction, False, ["--feature"]), + ] + for action, default, args in actions: + with self.subTest(action=action, default=default): + parser = configargparse.ArgumentParser() + parser.add_argument("--feature", action=action, default=default) + namespace = parser.parse_args(args, env_vars={}) + reader = configargparse.ArgumentParser() + reader.add_argument("--feature", action=action, default=default) + self.assert_round_trip(parser, namespace, reader) + + def test_store_const_round_trip(self): + def make_parser(): + parser = configargparse.ArgumentParser() + parser.add_argument( + "--fast", + action="store_const", + const="fast", + dest="mode", + default="slow", + env_var="CONFIGARGPARSE_TEST_FAST", + ) + parser.add_argument("--label", env_var="CONFIGARGPARSE_TEST_LABEL") + return parser + + cases = [ + (["--fast"], {}, "fast", "fast = true\n"), + ( + [], + {"CONFIGARGPARSE_TEST_FAST": "true", "CONFIGARGPARSE_TEST_LABEL": "x"}, + "fast", + "fast = true\nlabel = x\n", + ), + ( + [], + {"CONFIGARGPARSE_TEST_FAST": "false", "CONFIGARGPARSE_TEST_LABEL": "x"}, + "slow", + "fast = false\nlabel = x\n", + ), + ] + for args, env_vars, expected_mode, expected_contents in cases: + with self.subTest(args=args, env_vars=env_vars): + parser = make_parser() + namespace = parser.parse_args(args, env_vars=env_vars) + self.assertEqual(namespace.mode, expected_mode) + contents = self.assert_round_trip(parser, namespace, make_parser()) + self.assertEqual(contents, expected_contents) + + ################################################################################ # since configargparse should work as a drop-in replacement for argparse # in all situations, run argparse unittests on configargparse by modifying