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

From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Alex Riesen <alexander.riesen@cetitec.com>,
	git@vger.kernel.org, Eric Wong <normalperson@yhbt.net>
Subject: Re: sub-fetches discard --ipv4|6 option
Date: Wed, 16 Sep 2020 12:34:27 -0400	[thread overview]
Message-ID: <20200916163427.GB17726@coredump.intra.peff.net> (raw)
In-Reply-To: <xmqqa6xqzpx9.fsf@gitster.c.googlers.com>

On Tue, Sep 15, 2020 at 02:32:50PM -0700, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > So I think the best you could do is:
> >
> >   1. Keep two separate option lists, "parent" and "child". The parent
> >      list has "--all" in it. The child list has stuff like "--ipv6".
> >
> >   2. Parse using the parent list with PARSE_OPT_KEEP_UNKNOWN. That lets
> >      you decide whether we're in a mode that is spawning child fetch
> >      processes.
> 
> Hmph, I vaguely recall discussion about cascading options[] list but
> do not find anything that may be involved in an implementation like
> that in <parse-options.h>.  I agree that neither of the above is so
> attractive.

I think we just use KEEP_UNKNOWN in those cases and ignore any downsides
to it.

> > I guess parse-options could provide a MAYBE_PASSTHRU flag. On the first
> > parse_options() call, it would skip over any such options, leaving them
> > in argv. On the second, the caller would tell it to actually parse them.
> 
> Or calling it USR1, which is a good way to make it crystal clear
> that parse_options() API does not do anything to it.  The code like
> "builtin/fetch.c" can locally give it a more meaningful name with
> "#define PARSE_OPT_RECURSIVE PARSE_OPT_USR1". if recursive is the
> appropriate name for the bit in the context of the options[] array.

Ah, that's a good suggestion. My earlier "USER" suggestion was
tongue-in-cheek, because I think it makes the resulting options list
quite confusing.  But a local #define fixes that nicely.

That said, it sounds from the other part of the thread like we'll need
better parse-options support anyway, so this "noop flag bit" idea
probably isn't a good direction anyway.

-Peff