diff --git a/git/config.py b/git/config.py index 0da90bab2..d6f2706f1 100644 --- a/git/config.py +++ b/git/config.py @@ -45,7 +45,7 @@ from git.repo.base import Repo T_ConfigParser = TypeVar("T_ConfigParser", bound="GitConfigParser") -T_OMD_value = TypeVar("T_OMD_value", str, bytes, int, float, bool) +T_OMD_value = TypeVar("T_OMD_value", str, bytes, int, float, bool, None) if sys.version_info[:3] < (3, 7, 2): # typing.Ordereddict not added until Python 3.7.2. @@ -291,6 +291,12 @@ class GitConfigParser(cp.RawConfigParser, metaclass=MetaParserBuilder): :note: If used as a context manager, this will release the locked file. + + :note: + Options without a value are stored as ``None`` and written without ``=``. + :meth:`get_value` and :meth:`get_values` return an empty string for them, + while :meth:`getboolean` returns ``True``. An explicit empty value is + stored as an empty string and reads as ``False`` with :meth:`getboolean`. """ # { Configuration @@ -348,7 +354,7 @@ def __init__( Reference to repository to use if ``[includeIf]`` sections are found in configuration files. """ - cp.RawConfigParser.__init__(self, dict_type=_OMD) + cp.RawConfigParser.__init__(self, dict_type=_OMD, allow_no_value=True) self._dict: Callable[..., _OMD] self._defaults: _OMD self._sections: _OMD @@ -587,8 +593,12 @@ def parse_value(value: str) -> str: # Preserves multiple values for duplicate optnames. cursect.add(optname, optval) else: - # Check if it's an option with no value - it's just ignored by git. - if not self.OPTVALUEONLY.match(line): + # A valueless option is an implicit boolean true, not an empty value. + mo = self.OPTVALUEONLY.match(line) + if mo: + optname = self.optionxform(mo.group("option").rstrip()) + cursect.add(optname, None) + else: if not e: e = cp.ParsingError(fpname) e.append(lineno, repr(line)) @@ -625,6 +635,7 @@ def _all_items(section: str) -> List[Tuple[str, str]]: for key, values in self._sections[section].items_all() if key != "__name__" for value in values + if value is not None ] paths = [] @@ -760,13 +771,16 @@ def _write(self, fp: IO) -> None: def write_section(name: str, section_dict: _OMD) -> None: fp.write(("[%s]\n" % name).encode(defenc)) - values: Sequence[str] # Runtime only gets str in tests, but should be whatever _OMD stores. - v: str + values: List[Any] + v: Any for key, values in section_dict.items_all(): if key == "__name__": continue for v in values: + if v is None: + fp.write(("\t%s\n" % key).encode(defenc)) + continue value = self._value_to_string(v) if any(char in value for char in '\n\t\b\\"#;') or value[:1].isspace() or value[-1:].isspace(): value = value.replace("\\", "\\\\").replace('"', '\\"') @@ -783,11 +797,11 @@ def write_section(name: str, section_dict: _OMD) -> None: for name, value in self._sections.items(): write_section(name, value) - def items(self, section_name: str) -> List[Tuple[str, str]]: # type: ignore[override] + def items(self, section_name: str) -> List[Tuple[str, Union[str, None]]]: # type: ignore[override] """:return: list((option, value), ...) pairs of all items in the given section""" return [(k, v) for k, v in super().items(section_name) if k != "__name__"] - def items_all(self, section_name: str) -> List[Tuple[str, List[str]]]: + def items_all(self, section_name: str) -> List[Tuple[str, List[Union[str, None]]]]: """:return: list((option, [values...]), ...) pairs of all items in the given section""" rv = _OMD(self._defaults) @@ -841,6 +855,8 @@ def write(self) -> None: for key, values in section.items_all(): if key != "__name__": for raw_value in values: + if raw_value is None: + continue if "\r" in self._value_to_string(raw_value) or "\x00" in self._value_to_string(raw_value): raise ValueError("Git config values must not contain CR or NUL") @@ -877,7 +893,6 @@ def read_only(self) -> bool: """:return: ``True`` if this instance may change the configuration file""" return self._read_only - # FIXME: Figure out if default or return type can really include bool. def get_value( self, section: str, @@ -894,7 +909,7 @@ def get_value( did not exist. :return: - A properly typed value, either int, float or string + A properly typed value, either int, float, string or bool :raise TypeError: In case the value could not be understood. @@ -925,7 +940,7 @@ def get_values( in case the option did not exist. :return: - A list of properly typed values, either int, float or string + A list of properly typed values, either int, float, string or bool :raise TypeError: In case the value could not be understood. @@ -941,7 +956,19 @@ def get_values( return [self._string_to_value(valuestr) for valuestr in lst] - def _string_to_value(self, valuestr: str) -> Union[int, float, str, bool]: + def _convert_to_boolean(self, value: Union[str, None]) -> bool: + if value is None: + return True + if value == "": + return False + try: + return self.BOOLEAN_STATES[value.lower()] + except KeyError: + raise ValueError("Not a boolean: %s" % value) from None + + def _string_to_value(self, valuestr: Union[str, None]) -> Union[int, float, str, bool]: + if valuestr is None: + return "" types = (int, float) for numtype in types: try: diff --git a/test/test_config.py b/test/test_config.py index 3031721de..2910f3e6c 100644 --- a/test/test_config.py +++ b/test/test_config.py @@ -864,8 +864,58 @@ def test_empty_config_value(self): assert cr.get_value("core", "filemode"), "Should read keys with values" - with self.assertRaises(cp.NoOptionError): - cr.get_value("color", "ui") + self.assertTrue(cr.has_option("color", "ui")) + self.assertIsNone(cr.get("color", "ui")) + self.assertEqual(cr.get_value("color", "ui"), "") + self.assertIs(cr.getboolean("color", "ui"), True) + + @with_rw_directory + def test_implicit_boolean_round_trip(self, rw_dir): + config_path = osp.join(rw_dir, "config") + with open(config_path, "wb") as config_file: + config_file.write( + b"[include]\n" + b"\toptional\n" + b"[flag]\n" + b"\timplicit\n" + b"\ttrailing-space \n" + b"\ttrailing-tab\t\n" + b"\tempty =\n" + b'\tquoted = ""\n' + b"\tmultiple = false\n" + b"\tmultiple\n" + b"\tmultiple =\n" + b"\tmultiple" + ) + git_config = ["git", "config", "--file", config_path] + original = subprocess.check_output(git_config + ["--null", "--list"]) + + with GitConfigParser(config_path, read_only=False) as config: + for option in ("implicit", "trailing-space", "trailing-tab"): + self.assertIsNone(config.get("flag", option)) + self.assertEqual(config.get_value("flag", option), "") + self.assertIs(config.getboolean("flag", option), True) + for option in ("empty", "quoted"): + self.assertEqual(config.get("flag", option), "") + self.assertEqual(config.get_value("flag", option), "") + self.assertIs(config.getboolean("flag", option), False) + self.assertEqual(config.get_values("flag", "multiple"), [False, "", "", ""]) + self.assertIsNone(dict(config.items("flag"))["multiple"]) + self.assertEqual(dict(config.items_all("flag"))["multiple"], ["false", None, "", None]) + config.set_value("other", "value", "updated") + + self.assertEqual( + subprocess.check_output(git_config + ["--null", "--list"]), + original + b"other.value\nupdated\0", + ) + self.assertEqual( + subprocess.check_output(git_config + ["--type=bool", "--get-all", "flag.multiple"]), + b"false\ntrue\nfalse\ntrue\n", + ) + with GitConfigParser(config_path) as config: + self.assertIs(config.getboolean("flag", "implicit"), True) + self.assertIs(config.getboolean("flag", "empty"), False) + self.assertEqual(dict(config.items_all("flag"))["multiple"], ["false", None, "", None]) def test_config_with_quotes(self): cr = GitConfigParser(fixture_path("git_config_with_quotes"), read_only=True)