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

From: Jeff King <peff@peff.net>
Date: Apr 23, 2021, 10:05 UTC
Message-ID: <YIKb7qoeLtGjgsHr@coredump.intra.peff.net>
In-Reply-To: <87y2dd2pdn.fsf@evledraar.gmail.com>

On Tue, Apr 20, 2021 at 12:59:16PM +0200, Ævar Arnfjörð Bjarmason wrote:

Show 17 quoted lines

> >> 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?

I _think_ so, but the result might be quite hard to read (the logic would be scattered all over a bunch of tiny callbacks). But it might not be too bad. Especially if you figure out which ones actually need the time-of-parse logic and use more vanilla OPT_* for the others (that's the rabbit hole I alluded to).

I think things like the "--exec-path" behavior that Patrick mentioned would still work (I think it's just a stock OPTARG).

-Peff