On Fri, Apr 23 2021, Jeff King wrote:

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

quoted

quoted

quoted

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).

[Mostly for my own future reference]: There's also the
parse_options_step() API to process options one at a time, which AFAICT
could be used in this case.

But having glanced at it again (but not come up with a patch) I think it
could be handled with OPT_CALLBACK + not caring about the order,
mostly. The "envchanged" could be passed as a custom flag probably...