Skip to content

fix: strip inline config comments the way git does - #2245

Open
rawsun007 wants to merge 1 commit into
gitpython-developers:mainfrom
rawsun007:inline-comment-stripping
Open

rawsun007 wants to merge 1 commit into
gitpython-developers:mainfrom
rawsun007:inline-comment-stripping

Conversation

@rawsun007

@rawsun007 rawsun007 commented Sep 17, 2026

Copy link
Copy Markdown
Contributor

Written by Claude Opus 5 in Claude Code, running through @rawsun007's account, per CONTRIBUTING "Prevent agent impersonation". I speak only for myself here.

In git, a # or ; outside quotes starts a comment — with or without a space before it, and whether or not the value is quoted. GitConfigParser cut only a ; that was preceded by whitespace in a value that did not start with ":

if vi in ("=", ":") and ";" in optval and not optval.strip().startswith('"'):
    pos = optval.find(";")
    if pos != -1 and optval[pos - 1].isspace():
        optval = optval[:pos]

Measured against git config -f <file> --get a.k on git 2.50.1:

[a]\n\tk = … git before after
value # comment value value # comment value
value ; comment value value value
value#nospace value value#nospace value
value;nospace value value;nospace value
a # b ; c a a # b a
"quoted" # after quoted quoted" # after quoted

The last row is the worst of them: because the value starts with " and does not end with one, the comment made it look like an unterminated quote, so it was read as the start of a multi-line value and the following lines were swallowed into it.

The fix is small because the correct scan is already in this file: is_line_continuation walks a value tracking quotes and backslash escapes. strip_inline_comment reuses that walk to cut the comment before the quote-structure branches, so all three branches see comment-free text, and the legacy ;-only block is gone.

A comment character inside quotes stays literal ("has # inside"has # inside), and both are pinned.

Escape handling is deliberately untouched. My first attempt also routed every unquoted value through parse_value, which resolves \t, \n and friends — that broke every Windows job, because a temp path like C:\Temp\test_x\config2 came back with a tab in it. git rejects that line outright (fatal: bad config line 2), so unescaping it would have matched neither git nor the previous behaviour. Whether GitPython should follow git and reject invalid escapes in unquoted values is a separate question; this PR does not touch it.

Verification: test/test_config.py is 46 passed, 2 skipped, 14 subtests. Reverting only git/config.py fails 5 of the 8 new subtests, leaving the two quoted-literal cases and the already-working value ; comment green. The full test/ run is 231 passed / 6 failed / 663 errors both with and without this change — those are a fixture-setup problem in my checkout, identical on a clean tree, and none are in test_config.py.

Not touched: [a] k = inline, a key on the same line as the section header, which git accepts and this parser rejects with NoOptionError. That is section-header parsing rather than comment handling, so it looked like its own change — happy to send it separately if you want it.

A `#` or `;` outside quotes starts a comment in git, with or without a
space before it and whether or not the value is quoted. The parser only
cut a `;` that was preceded by whitespace in an unquoted value, so
`name = Alice # work` read back with the comment attached, and
`k = "quoted" # after` was mistaken for an unterminated multi-line quote
and returned `quoted" # after`.

`strip_inline_comment` cuts the comment before the quote-structure
branches, using the same quote- and escape-aware scan that
`is_line_continuation` already uses, so all three branches see
comment-free text. Escape handling is untouched.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@rawsun007
rawsun007 force-pushed the inline-comment-stripping branch from 71af3aa to 452878f Compare September 17, 2026 09:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant