Preserve implicit boolean config keys - #2238
Merged
Merged
Conversation
This was mostly a rubber-stamp, knowing the the whole implementation is quite a hack that is held together with ductape. Ideally, it will just work well enough at some point, to gain time for v4 to be made. <!-- agent --> GitConfigParser discarded keys written without an assignment. Reading such an option raised NoOptionError, and editing an unrelated setting silently removed it. Git treats a bare key as true but an explicitly empty value as false, so representing both as an empty string would lose their meaning. Store bare entries as None, following RawConfigParser's allow_no_value representation. Raw get/items access preserves the distinction, while get_value/get_values return an empty string as requested. Convert None to true and an empty string to false in getboolean, retaining the standard boolean spellings. Write None entries without an equals sign and exclude them from string validation and include-path expansion. The existing ordered multi-dict preserves repeated bare and assigned entries together. Update the regression that expected bare color.ui to disappear, and add a Git-backed read-modify-write test covering trailing whitespace, EOF without a newline, quoted and unquoted empty values, repeated keys, and a valueless non-path option in an include section. Both regressions failed before the fix. Compare Git's NUL-delimited listing before and after an unrelated edit and check the resulting repeated values with --type=bool --get-all. Git reference: checkout 1630431f326e15fcde608827b5ff38422528eb59, t/t1300-config.sh tests for novalue.variable and emptyvalue.variable, and parse.c:git_parse_maybe_bool_text. Runtime comparison used Git 2.50.1 (Apple Git-155). Assisted-by: GPT 6.0 Co-authored-by: GPT 6.0 <codex@openai.com>
Byron
force-pushed
the
read-implicit-bool
branch
from
September 13, 2026 15:16
0b6c168 to
2229668
Compare
Byron
marked this pull request as ready for review
September 13, 2026 15:42
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Valueless keys followed by inline comments are parsed incorrectly and need to be fixed.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Preserves Git-style valueless configuration keys through parsing, access, and write-back.
Changes:
- Stores valueless options as
None. - Preserves repeated entries and bare syntax during serialization.
- Adds regression coverage for boolean semantics and round-tripping.
File summaries
| File | Summary |
|---|---|
test/test_config.py |
Adds coverage for implicit booleans, empty values, duplicates, includes, and round-tripping. |
git/config.py |
Implements valueless-option handling and serialization; inline comments on bare keys still require correction. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Tasks
This section is for Byron only. Models continuing this PR must not add, remove, check, uncheck, rename, or reorder checkboxes here.
Everything below this line was generated by
Codex.Created by Codex on behalf of Byron. Byron will review before this is ready to merge.
Fixes #2237.
Bare config keys were dropped during parsing and disappeared when an unrelated setting was written. Preserve them as
None, followingRawConfigParser's valueless-option representation.get_value()andget_values()expose an empty string, whilegetboolean()returnsTrue; explicitly empty assignments returnFalsethroughgetboolean(). Write-back retains bare syntax and repeated occurrences.The regression covers bare keys, trailing whitespace, a final line without a newline, quoted and unquoted empty values, repeated values, and a valueless non-path option in an include section. It compares Git's NUL-delimited listing before and after an unrelated edit and verifies the resulting boolean values.
Validation on Python 3.12.14: 41 configuration tests passed, 2 existing skips; Ruff lint and formatting, mypy (46 source files), basedpyright, and
git diff --checkpassed. Both regressions failed before the fix.Codex review of
0b6c1682found no actionable regressions.Git reference:
1630431f326e15fcde608827b5ff38422528eb59,t/t1300-config.shtests for valueless versus empty values, andparse.c:git_parse_maybe_bool_text. Runtime comparisons used Git 2.50.1 (Apple Git-155).