From: Kristoffer Haugsbakk <kristofferhaugsbakk@fastmail.com>
Date: Mon, 23 Feb 2026 23:25:22 GMT
Subject: Re: [PATCH 1/2] format-patch: make format.noprefix a boolean
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:
>>[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.

> 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).