From: Junio C Hamano <gitster@pobox.com>
Date: Mon, 12 Jan 2026 15:08:51 GMT
Subject: Re: [PATCH v10 1/8] builtin/replay: extract core logic to replay revisions
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.