Subject: Re: [PATCH v8 2/8] config: add new way to pass config via `--config-env`

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 20 Apr 2021 12:59:16 +0200

On Sat, Apr 17 2021, Jeff King wrote:

> On Fri, Apr 16, 2021 at 05:40:36PM +0200, Ævar Arnfjörð Bjarmason wrote:
>
>> Bonus points to anyone sorting out some of the existing inconsistencies
>> when fixing this, i.e. --exec-path supports either the "=" form, or not,
>> but various other skip_prefix() in the same function don't, seemingly
>> (but I have not tested) for no good reason.
>
> I suspect just because it's more (per-option) work to support both
> types, and nobody really cared enough to do so.
>
>> It seems to me that having a skip_prefix_opt() or something would be a
>> good fix for this, i.e. a "maybe trim the last '='" version of
>> skip_prefix. Then we could just consistently use that.
>
> There's a similar situation in the revision parser (which does not use
> our regular parse-options). There we have a parse_long_opt() helper
> which does the right thing. We could use that more widely.
>
> I also wouldn't be surprised if we could leverage one of the
> sub-functions of parse-options, but it might turn into a rabbit hole.
> Converting the whole thing to the usual parse_options() might get
> awkward, since many of the options operate at time-of-parse, not after
> we've seen everything (I suspect many of them don't care either way, but
> you're always risking subtle regressions there).

So we could use parse_options() and guarantee the existing behavior if
they were all OPT_CALLBACK?