Re: sub-fetches discard --ipv4|6 option

On Thu, Sep 17, 2020 at 04:33:39PM +0200, Alex Riesen wrote:

quoted

Adding a new strvec pointer to parse_options() works, but means updating
all of the callers, most of which will pass NULL. Possibly the existing
"flags" parameter to parse_options() could grow into a struct. That
requires modifying each caller, but at least solves the problem once and
for all.

With such complication a step-wise parsing sounds easier, given that at the
moment there is only one user for the feature. Are there *existing* callers
of parse_options with similar requirements?

I don't know offhand. I'd suspect some of the command which take
--recurse-submodules do something similar, but the number of steps
between the main command the submodule argv may make it awkward to use
a parse-options solution. I don't think we have a "push to each of these
remotes" option the way we do for fetch.

I feel that doing this kind of selection work in parse_options is an overkill:
if it is specific for just this use case, the implementation might be more
complex than necessary, while profiting just one caller.

Yeah, I agree it's complex, and I'm happy with simpler solutions (or
just fixing these ones as you did and punting on it for now).

quoted

Another option is to stick it into parse_opt_ctx_t. That's used only be
step-wise callers, of which there are very few.

Does that mean that currently there is no way to find out which option
corresponds to the last parsed command-line argument after a call to
parse_options_step? Which in turn makes the marking of recursive options
inaccessible to step-wise command line parsing code, right?

I'm not super familiar with the internals of parse-options, but it
doesn't look like it. Each step consumes an argv and matches it to a
"struct option", but I don't think you get to know which struct it was
matched to. It would be reasonable for it to keep a pointer in the
parse_opt_ctx_t (and of course you'd need some bit in the option struct
itself to say "I am a recursive option").

-Peff