Skip to content

Don't remove quotes if \ or " are present inside #2048

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Add tests of ConfigParser with "..." with " or \ inside
These are cases where just removing the outer quotes without doing
anything to the text inside does not give the correct result, and
where keeping the quotes may be preferable, in that it was the
long-standing behavior of `GitConfigParser`.

That this was the long-standing behavior may justify bringing it
back when the `"`-`"`-enclosed text contains such characters, but
it does not justify preserving it indefinitely: it will still be
better to parse the escape sequences, at least in the type case
that all of them in a value's representation are well-formed.
  • Loading branch information
EliahKagan committed Jun 8, 2025
commit 7bcea08873ca4052ab743f9e2f7cff42e7fe62d8
9 changes: 9 additions & 0 deletions test/fixtures/git_config_with_quotes_escapes
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[custom]
hasnewline = "first\nsecond"
hasbackslash = "foo\\bar"
hasquote = "ab\"cd"
hastrailingbackslash = "word\\"
hasunrecognized = "p\qrs"
hasunescapedquotes = "ab"cd"e"
ordinary = "hello world"
unquoted = good evening
20 changes: 20 additions & 0 deletions test/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -428,6 +428,26 @@ def test_config_with_quotes_with_whitespace_outside_value(self):
cr = GitConfigParser(fixture_path("git_config_with_quotes_whitespace_outside"), read_only=True)
self.assertEqual(cr.get("init", "defaultBranch"), "trunk")

def test_config_with_quotes_containing_escapes(self):
"""For now just suppress quote removal. But it would be good to interpret most of these."""
cr = GitConfigParser(fixture_path("git_config_with_quotes_escapes"), read_only=True)

# These can eventually be supported by substituting the represented character.
self.assertEqual(cr.get("custom", "hasnewline"), R'"first\nsecond"')
self.assertEqual(cr.get("custom", "hasbackslash"), R'"foo\\bar"')
self.assertEqual(cr.get("custom", "hasquote"), R'"ab\"cd"')
self.assertEqual(cr.get("custom", "hastrailingbackslash"), R'"word\\"')
self.assertEqual(cr.get("custom", "hasunrecognized"), R'"p\qrs"')

# It is less obvious whether and what to eventually do with this.
self.assertEqual(cr.get("custom", "hasunescapedquotes"), '"ab"cd"e"')

# Cases where quote removal is clearly safe should happen even after those.
self.assertEqual(cr.get("custom", "ordinary"), "hello world")

# Cases without quotes should still parse correctly even after those, too.
self.assertEqual(cr.get("custom", "unquoted"), "good evening")

def test_get_values_works_without_requiring_any_other_calls_first(self):
file_obj = self._to_memcache(fixture_path("git_config_multiple"))
cr = GitConfigParser(file_obj, read_only=True)
Expand Down
Loading