On Fri, Apr 23 2021, Jeff King wrote: > On Tue, Apr 20, 2021 at 12:59:16PM +0200, Ævar Arnfjörð Bjarmason wrote: > >> >> 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...