Re: [PATCH 1/2] format-patch: make format.noprefix a boolean
- From
- Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
- Date
- Feb 23, 2026, 23:25 UTC
- Message-ID
- <ff92bec9-19b1-4107-9208-f692709ba9b4@app.fastmail.com>
- In-Reply-To
- <xmqqy0ko626g.fsf@gitster.g>
On Thu, Feb 19, 2026, at 19:03, Junio C Hamano wrote:
Show 12 quoted lines
>>[snip] >> Let’s only offer a breaking change fig leaf by hinting about the >> previous behavior before dying. > > One case that is often problematic is what happens to those who use > the same set of configuration variables with different versions of > Git, before and after such behaviour change. But I do not think > this is such a bad thing. The only reason why they had this > variable set (to any value, or to a value-less true) with existing > versions of Git is because they wanted to omit the prefixes, so when > a new version of Git dies with "Heh, 'nothanks' is not a valid > boolean value", they can edit the configuration variable to "1".
Yeah.
I like how this was handled for `core.commentString`: You can set both `core.commentChar` and the new config and still be able to run on old versions.
Show 22 quoted lines
> And from that point of view, I think the hint given together with
> the "bad boolean" error can and should be phrased a bit more
> strongly, i.e.,
>
>> + format_no_prefix = git_parse_maybe_bool(value);
>> + if (format_no_prefix < 0) {
>> + int status = die_message(
>> + _("bad boolean config value '%s' for '%s'"),
>> + value, var);
>> + fprintf(stderr,
>> + _("hint: '%s' used to accept any value but "
>> + "now only\n"
>> + "hint: accepts boolean values, like '%s'\n"),
>> + var, "diff.noprefix");
>
> The target audience of this (hint) is those who have set this
> variable to a non-boolean strring from the existing version of Git,
> and the only thing they meant to express was "I do not want any
> prefix", so "we used to accept any value as true, but now accepts
> only valid boolean values", perhaps? That would nudge those who
> wrote "[format] noprefix = NoThanks" to rewrite it correctly to
> "true" or "1", and not "no".Very true. I’ll change to spell out that any value used to be treated as `true`.
Right now it just says “used to accept any value”. But not what it means to accept any value...
> This is a related tangent, but shouldn't this use advise() without > configuration? There is no need to allocate an advice_type and use > advise_if_enabled(), because correcting a malformed configuration is > an action enough to squelch the message.
Oh, right. That fits well here.
In hindsight I think I should have used that in 5a312527 (whatchanged: hint about git-log(1) and aliasing, 2025-09-17).