Re: [PATCH v10 1/8] builtin/replay: extract core logic to replay revisions

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 12, 2026, 15:08 UTC
Message-ID: <xmqqv7h6na0s.fsf@gitster.g>
In-Reply-To: <20260112-b4-pks-history-builtin-v10-1-e3c6aa5b4cec@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> -	die_for_incompatible_opt2(!!onto_name, "--onto",
> -				  !!*advance_name, "--advance");
> +	if (!(!!onto_name ^ !!*advance_name))
> +		BUG("expected either onto_name or *advance_name in this function");

This brings our crypticness to a whole new level. onto_name not being NULL is a sign that "--onto" was given, while *advance_name pointer points at the string that "--advance" option has received. We are saying that only one of these two must be non-NULL, and the other must be NULL.

I know !!VAR is an idiom to turn any pointer into 0 (=NULL) or 1 (!=NULL), but isn't the latter (i.e., normalizing all non-NULL pointer to 1) a bit overkill, which becomes only necessary because the construction wants to use "^" as "sides of this operator are different Boolean values" operator. And then to add on top, the whole thing is !(negated). I wonder if

	if (!onto_name != !*advance_name)
		BUG("one and only one of --onto/--advance must be given");

is easier to follow without being overly cute?

Thanks.