Re: [PATCH 1/8] merge: make xopts a strvec

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 31, 2023, 15:46 UTC
Message-ID: <xmqqcyz3z032.fsf@gitster.g>
In-Reply-To: <20230831071230.GA3197647@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 17 quoted lines

> The "xopts" variable uses a custom array with ALLOC_GROW(). Using a
> strvec simplifies things a bit. We need fewer variables, and we can also
> ditch our custom parseopt callback in favor of OPT_STRVEC().
>
> As a bonus, this means that "--no-strategy-option", which was previously
> a silent noop, now does something useful: like other list-like options,
> it will clear the list of -X options seen so far.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> I guess you could argue this is a backwards-incompatible change, but the
> existing behavior of --no-strategy-option is so dumb that I can't
> believe somebody would prefer it (plus revert/cherry-pick already use
> OPT_STRVEC for their matching "-X").
>
> I didn't bother adding a test since we're just re-using OPT_STRVEC code
> that is used elsewhere.

I do not think of any useful way to have "--no-strategy-option" on the command line (either as an early part of an alias or in a script) that does nothing (it's not like the command requires at least one -X option on the command line), either. Just like fb60b9f3 (sequencer: use struct strvec to store merge strategy options, 2023-04-10), which met no complaints about a possible fallout by the behaviour change, I do not think that this change even deserves an entry in the backward compatibility notes.

We did not have strvec, let alone OPT_STRVEC(), back when the -Xopts was invented around the Git 1.7.0 cycle. The loss of custom xopts code is very nice.

Thanks.