Re: [PATCH v10 1/8] builtin/replay: extract core logic to replay revisions
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jan 12, 2026, 15:37 UTC
- Message-ID
- <aWUVHKMfG0vRb8_G@pks.im>
- In-Reply-To
- <xmqqv7h6na0s.fsf@gitster.g>
On Mon, Jan 12, 2026 at 07:08:51AM -0800, Junio C Hamano wrote:
Show 24 quoted lines
> 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?It should be:
if (!onto_name == !*advance_name)
BUG(...);But other than that it reads way better indeed. Fixed up locally, thanks!
Patrick