{{ message }}
config: read boolean values the way git does - #2360
Open
rawsun007 wants to merge 1 commit into
Open
Conversation
Git accepts `yes`/`no`/`on`/`off` and any integer for a boolean, and this
repository already implements that in `parseConfigBool`, citing
`git_parse_maybe_bool_text`. Five keys did not use it:
- `commit.gpgsign` and `tag.gpgsign` went through `strconv.ParseBool`, so
`yes`, `on`, `no`, `off`, `2` and `-1` failed to parse and left the option
unset. A repository configured with `commit.gpgsign = yes` therefore got no
signing, silently, because unset means "use the default".
- `index.skipHash` and `uploadarchive.allowUnreachable` had the same
treatment.
- `extensions.worktreeConfig` compared the value against the literal
"true", so `1`, `yes` and `on` all read as false.
`core.protectNTFS` and `core.protectHFS` already used `parseConfigBool` and
were correct, which is what the new test contrasts against.
Expectations in TestUnmarshalBoolValues are the output of
`git config -f <file> --type=bool --get <key>` under git 2.50.1. An
unreadable value still leaves the key unset rather than erroring, so the
caller's default applies as before.
Assisted-by: Claude Code (Claude Opus 5)
Signed-off-by: Roshan Ramani <roshanramani.dev@gmail.com>
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
1 participant
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.
Git accepts
yes/no/on/offand any integer for a boolean. This repository already implements that inparseConfigBool, citinggit_parse_maybe_bool_text— but five keys did not use it.Measured against
git config -f <file> --type=bool --get, git 2.50.1:commit.gpgsignbeforecore.protectNTFS(already correct)yes/Yes/on/Onno/off2/-1true/1/false/0commit.gpgsignandtag.gpgsignusedstrconv.ParseBool, which rejects those spellings; the error was swallowed and the option left unset. Since unset means "use the default", a repository configured withcommit.gpgsign = yesgot no signing at all, with nothing reported.index.skipHashanduploadarchive.allowUnreachablehad the same treatment.extensions.worktreeConfigcompared against the literal"true", so1,yesandonread as false.All five now go through
parseConfigBool.core.protectNTFS/protectHFSalready did, which is what the new test contrasts against — same file, same value, two different answers before this change.Behaviour worth calling out explicitly: an unreadable value (
garbage) still leaves the key unset rather than erroring, exactly as before. Git errors there; go-git's contract is that the caller's default applies, andparseConfigBool's own doc comment says an empty value means unset. I did not change that, since it is a separate decision from which spellings parse.Verification:
TestUnmarshalBoolValuesexpectations are thatgit config --type=booloutput, not my reading ofparse.c.config/config.gofails the new test on the firstyescase.go test -short ./...— 68 packages pass, no failures.Note on the one pre-existing failure you may see locally:
plumbing/format/gitignore'sTestConformanceSuitefails on any git older than 2.52.0, exactly as your ownoracleVersionHintnote predicts (mine is 2.50.1). Unrelated to this change, and I left it alone — the comment says the visible disagreement is deliberate.This is a behaviour change rather than a pure refactor, so if you would rather it went through an issue or RFC first per
AI_POLICY.md, say so and I will move the write-up there.AI disclosure per
AI_POLICY.md: found and written with Claude Code (Claude Opus 5), recorded in the commit'sAssisted-by:trailer. @rawsun007 authorised the DCO sign-off and reviewed the change.