Re: [PATCH 1/2] format-patch: make format.noprefix a boolean

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 19, 2026, 18:03 UTC
Message-ID: <xmqqy0ko626g.fsf@gitster.g>
In-Reply-To: <format.noprefix_boolean.39d@msgid.xyz>

kristofferhaugsbakk@fastmail.com writes:

Show 32 quoted lines

> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> The config `format.noprefix` was added in 8d5213de (format-patch: add
> format.noprefix option, 2023-03-09) to support no-prefix on paths.
> That was immediately after making git-format-patch(1) not respect
> `diff.noprefix`.[1]
>
> The intent was to mirror `diff.noprefix`. But this config was
> unintentionally[2] implemented by enabling no-prefix if any kind of
> value is set.
>
> † 1: c169af8f (format-patch: do not respect diff.noprefix, 2023-03-09)
> † 2: https://lore.kernel.org/all/20260211073553.GA1867915@coredump.intra.peff.net/
>
> Let’s indeed mirror `diff.noprefix` by treating it as a boolean.
>
> This is a breaking change. And as far as breaking changes go it is
> pretty benign:
>
> • The documentation claims that this config is equivalent to
>   `diff.noprefix`; this is just a bug fix if the documentation is
>   what defines the application interface
> • Only users with non-boolean values will run into problems when we
>   try to parse it as a boolean. But what would (1) make them suspect
>   they could do that in the first place, and (2) have motivated them to
>   do it?
> • Users who have set this to `false` and expect that to mean *enable
>   format.noprefix* (current behavior) will now have the opposite
>   experience. Which is not a reasonable setup.
>
> 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".

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

Show 10 quoted lines

> +		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".

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.

> +			exit(status);
> +		}
>  		return 0;
>  	}