From 4b35763d5f5c8c5ec65e8c44835634a6f5b9fe87 Mon Sep 17 00:00:00 2001 From: wim glenn Date: Thu, 28 Apr 2022 21:56:44 -0500 Subject: [PATCH 1/5] ``pip config`` normalizes names, converting underscores into dashes. closes #9330 --- news/9330.bugfix.rst | 1 + src/pip/_internal/configuration.py | 9 +++++++-- tests/unit/test_configuration.py | 19 ++++++++++++++----- 3 files changed, 22 insertions(+), 7 deletions(-) create mode 100644 news/9330.bugfix.rst diff --git a/news/9330.bugfix.rst b/news/9330.bugfix.rst new file mode 100644 index 000000000..4cbd4192b --- /dev/null +++ b/news/9330.bugfix.rst @@ -0,0 +1 @@ +``pip config`` normalizes names, converting underscores into dashes \ No newline at end of file diff --git a/src/pip/_internal/configuration.py b/src/pip/_internal/configuration.py index a8092d1ae..a2b0607c5 100644 --- a/src/pip/_internal/configuration.py +++ b/src/pip/_internal/configuration.py @@ -142,13 +142,16 @@ class Configuration: def get_value(self, key: str) -> Any: """Get a value from the configuration.""" + orig_key = key + key = _normalize_name(key) try: return self._dictionary[key] except KeyError: - raise ConfigurationError(f"No such key - {key}") + raise ConfigurationError(f"No such key - {orig_key}") def set_value(self, key: str, value: Any) -> None: """Modify a value in the configuration.""" + key = _normalize_name(key) self._ensure_have_load_only() assert self.load_only @@ -167,11 +170,13 @@ class Configuration: def unset_value(self, key: str) -> None: """Unset a value in the configuration.""" + orig_key = key + key = _normalize_name(key) self._ensure_have_load_only() assert self.load_only if key not in self._config[self.load_only]: - raise ConfigurationError(f"No such key - {key}") + raise ConfigurationError(f"No such key - {orig_key}") fname, parser = self._get_parser_to_modify() diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 788d32b9b..22ea98395 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -185,12 +185,8 @@ class TestConfigurationModification(ConfigurationMixin): def test_no_specific_given_modification(self) -> None: self.configuration.load() - try: + with pytest.raises(ConfigurationError): self.configuration.set_value("test.hello", "10") - except ConfigurationError: - pass - else: - assert False, "Should have raised an error." def test_site_modification(self) -> None: self.configuration.load_only = kinds.SITE @@ -241,3 +237,16 @@ class TestConfigurationModification(ConfigurationMixin): # get the path to user config file assert mymock.call_count == 1 assert mymock.call_args[0][0] == (get_configuration_files()[kinds.GLOBAL][-1]) + + def test_normalization(self) -> None: + # underscores and dashes can be used interchangeably. + # internally, underscores get converted into dashes before reading/writing file + self.configuration.load_only = kinds.GLOBAL + self.configuration.load() + self.configuration.set_value("global.index-url", "example.org") + assert self.configuration.get_value("global.index_url") == "example.org" + assert self.configuration.get_value("global.index-url") == "example.org" + self.configuration.unset_value("global.index-url") + pat = r"^No such key - global\.index-url$" + with pytest.raises(ConfigurationError, match=pat): + self.configuration.get_value("global.index-url") From af20d93d71b46a4cbb8f94d003a0de830c14fc9a Mon Sep 17 00:00:00 2001 From: wim glenn Date: Thu, 28 Apr 2022 22:00:16 -0500 Subject: [PATCH 2/5] placate pre-commit --- news/9330.bugfix.rst | 2 +- tests/unit/test_configuration.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/news/9330.bugfix.rst b/news/9330.bugfix.rst index 4cbd4192b..d760313a6 100644 --- a/news/9330.bugfix.rst +++ b/news/9330.bugfix.rst @@ -1 +1 @@ -``pip config`` normalizes names, converting underscores into dashes \ No newline at end of file +``pip config`` normalizes names, converting underscores into dashes diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index 22ea98395..dabbb73ea 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -243,7 +243,7 @@ class TestConfigurationModification(ConfigurationMixin): # internally, underscores get converted into dashes before reading/writing file self.configuration.load_only = kinds.GLOBAL self.configuration.load() - self.configuration.set_value("global.index-url", "example.org") + self.configuration.set_value("global.index_url", "example.org") assert self.configuration.get_value("global.index_url") == "example.org" assert self.configuration.get_value("global.index-url") == "example.org" self.configuration.unset_value("global.index-url") From 6efe70858c5ce6f3de5eb0c66c314e39451009d6 Mon Sep 17 00:00:00 2001 From: wim glenn Date: Thu, 28 Apr 2022 22:05:54 -0500 Subject: [PATCH 3/5] mention that detail.. --- docs/html/development/contributing.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/html/development/contributing.rst b/docs/html/development/contributing.rst index e443bfa57..87734ee4d 100644 --- a/docs/html/development/contributing.rst +++ b/docs/html/development/contributing.rst @@ -88,7 +88,7 @@ The contents of this file are reStructuredText formatted text that will be used as the content of the news file entry. You do not need to reference the issue or PR numbers in the entry, since ``towncrier`` will automatically add a reference to all of the affected issues when -rendering the NEWS file. +rendering the NEWS file. There must be a newline at the end of the file. In order to maintain a consistent style in the ``NEWS.rst`` file, it is preferred to keep the news entry to the point, in sentence case, shorter than From 32f642d1236a558e5918b17cb91670fb31db5029 Mon Sep 17 00:00:00 2001 From: wim glenn Date: Fri, 29 Apr 2022 22:51:16 -0500 Subject: [PATCH 4/5] provide a better error message for "pip config get index-url" --- src/pip/_internal/configuration.py | 3 +++ tests/unit/test_configuration.py | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/pip/_internal/configuration.py b/src/pip/_internal/configuration.py index a2b0607c5..8fd46c9b8 100644 --- a/src/pip/_internal/configuration.py +++ b/src/pip/_internal/configuration.py @@ -147,6 +147,9 @@ class Configuration: try: return self._dictionary[key] except KeyError: + # disassembling triggers a more useful error message than simply + # "No such key" in the case that the key isn't in the form command.option + _disassemble_key(key) raise ConfigurationError(f"No such key - {orig_key}") def set_value(self, key: str, value: Any) -> None: diff --git a/tests/unit/test_configuration.py b/tests/unit/test_configuration.py index dabbb73ea..c6b44d45a 100644 --- a/tests/unit/test_configuration.py +++ b/tests/unit/test_configuration.py @@ -1,6 +1,7 @@ """Tests for all things related to the configuration """ +import re from unittest.mock import MagicMock import pytest @@ -87,6 +88,25 @@ class TestConfigurationLoading(ConfigurationMixin): err.value ) + def test_no_such_key_error_message_no_command(self) -> None: + self.configuration.load_only = kinds.GLOBAL + self.configuration.load() + expected_msg = ( + "Key does not contain dot separated section and key. " + "Perhaps you wanted to use 'global.index-url' instead?" + ) + pat = f"^{re.escape(expected_msg)}$" + with pytest.raises(ConfigurationError, match=pat): + self.configuration.get_value("index-url") + + def test_no_such_key_error_message_missing_option(self) -> None: + self.configuration.load_only = kinds.GLOBAL + self.configuration.load() + expected_msg = "No such key - global.index-url" + pat = f"^{re.escape(expected_msg)}$" + with pytest.raises(ConfigurationError, match=pat): + self.configuration.get_value("global.index-url") + class TestConfigurationPrecedence(ConfigurationMixin): # Tests for methods to that determine the order of precedence of From ae1c2e35e493d7a07cb0d300451625629ae07ce9 Mon Sep 17 00:00:00 2001 From: Tzu-ping Chung Date: Wed, 4 May 2022 07:33:06 +0800 Subject: [PATCH 5/5] Grammar fix in changelog --- news/9330.bugfix.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/9330.bugfix.rst b/news/9330.bugfix.rst index d760313a6..63e44c238 100644 --- a/news/9330.bugfix.rst +++ b/news/9330.bugfix.rst @@ -1 +1 @@ -``pip config`` normalizes names, converting underscores into dashes +``pip config`` now normalizes names by converting underscores into dashes.