diff --git a/configargparse.py b/configargparse.py index db92aa6..8e6ed89 100644 --- a/configargparse.py +++ b/configargparse.py @@ -956,6 +956,10 @@ def parse_known_args( for action in actions_with_env_var_values: key = action.env_var value = env_vars[key] + # Skip empty string env vars for args with nargs to match YAML behavior + # where empty values are treated as None/not present (see issue #296) + if value == "" and action.nargs: + continue # Make list-string into list. if action.nargs or isinstance(action, argparse._AppendAction): nargs = True @@ -968,10 +972,24 @@ def parse_known_args( value = [elem.strip() for elem in value[1:-1].split(",")] env_var_args += self.convert_item_to_command_line_arg(action, key, value) - if nargs: - args = args + env_var_args + # Insert env var args before the first optional arg (starts with -) + # to preserve -- separator and positional args that come after it. + insertion_index = 0 + for i, arg in enumerate(args): + if arg.startswith(tuple(self.prefix_chars)): + insertion_index = i + break else: - args = env_var_args + args + # No optional args found. Check if any positional arg uses REMAINDER. + # If so, prepend config args to avoid them being consumed by REMAINDER. + # Otherwise, append to end (original behavior) to avoid issues with + # nargs="+" consuming positional args. + has_remainder = any( + a.is_positional_arg and a.nargs == argparse.REMAINDER + for a in self._actions + ) + insertion_index = 0 if has_remainder else len(args) + args = args[:insertion_index] + env_var_args + args[insertion_index:] if env_var_args: self._source_to_settings[_ENV_VAR_SOURCE_KEY] = OrderedDict( @@ -1039,6 +1057,11 @@ def parse_known_args( ) ) + # Skip empty string values for args with nargs to match YAML behavior + # where empty values are treated as None/not present (see issue #296) + if value == "" and action and action.nargs: + continue + if not discard_this_key: config_args += self.convert_item_to_command_line_arg( action, key, value @@ -1054,10 +1077,24 @@ def parse_known_args( ): nargs = True - if nargs: - args = args + config_args + # Insert config args before the first optional arg (starts with -) + # to preserve -- separator and positional args that come after it. + insertion_index = 0 + for i, arg in enumerate(args): + if arg.startswith(tuple(self.prefix_chars)): + insertion_index = i + break else: - args = config_args + args + # No optional args found. Check if any positional arg uses REMAINDER. + # If so, prepend config args to avoid them being consumed by REMAINDER. + # Otherwise, append to end (original behavior) to avoid issues with + # nargs="+" consuming positional args. + has_remainder = any( + a.is_positional_arg and a.nargs == argparse.REMAINDER + for a in self._actions + ) + insertion_index = 0 if has_remainder else len(args) + args = args[:insertion_index] + config_args + args[insertion_index:] # save default settings for use by print_values() default_settings = OrderedDict() diff --git a/tests/test_configargparse.py b/tests/test_configargparse.py index d46fde6..bb92b89 100644 --- a/tests/test_configargparse.py +++ b/tests/test_configargparse.py @@ -1344,6 +1344,83 @@ def error_func(path): args="-g file.txt", ) + def testRemainderWithConfigFile(self): + """Test that nargs=REMAINDER works correctly with config files. + Regression test for issue #285.""" + # Create a config file + config_file = tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".cfg") + config_file.write("config_file_option=value_from_config\n") + config_file.flush() + config_file.close() + + try: + self.initParser() + self.parser.add_argument( + "--config", is_config_file=True, default=config_file.name + ) + self.parser.add_argument("--config_file_option", nargs="*", default=None) + self.parser.add_argument( + "remainder_option", nargs=argparse.REMAINDER, default=None + ) + + # Test that REMAINDER doesn't swallow config file args + ns = self.parse(args=["test"]) + self.assertEqual(ns.remainder_option, ["test"]) + self.assertEqual(ns.config_file_option, ["value_from_config"]) + + # Test with multiple remainder args + ns = self.parse(args=["test", "arg1", "arg2"]) + self.assertEqual(ns.remainder_option, ["test", "arg1", "arg2"]) + self.assertEqual(ns.config_file_option, ["value_from_config"]) + + finally: + os.unlink(config_file.name) + + def testEmptyValuesIgnored(self): + """Test that empty string values from config files and env vars are ignored. + Regression test for issue #296.""" + # Test 1: Empty value in config file + config_file = tempfile.NamedTemporaryFile( + mode="w", delete=False, suffix=".conf" + ) + config_file.write("test-opt=\n") # Empty value + config_file.flush() + config_file.close() + + try: + self.initParser() + self.parser.add_argument( + "--config", is_config_file=True, default=config_file.name + ) + self.parser.add_argument( + "--test-opt", nargs=2, default=["default1", "default2"] + ) + + # Empty config value should be ignored, default should be used + ns = self.parse(args=[]) + self.assertEqual(ns.test_opt, ["default1", "default2"]) + + finally: + os.unlink(config_file.name) + + # Test 2: Empty environment variable + self.initParser() + self.parser.add_argument( + "--test-opt", nargs=2, env_var="TEST_OPT", default=["default1", "default2"] + ) + + old_env = os.environ.get("TEST_OPT") + try: + os.environ["TEST_OPT"] = "" # Empty env var + # Empty env var should be ignored, default should be used + ns = self.parse(args=[]) + self.assertEqual(ns.test_opt, ["default1", "default2"]) + finally: + if old_env is not None: + os.environ["TEST_OPT"] = old_env + elif "TEST_OPT" in os.environ: + del os.environ["TEST_OPT"] + class TestConfigFileParsers(TestCase): """Test ConfigFileParser subclasses in isolation"""