Re: [PATCH v8 0/2] builtin/config.c: support `--type=<type>` as preferred alias for `--type`
- From
Taylor Blau <me@ttaylorr.com>
- Date
- Apr 11, 2018, 03:49 UTC
- Message-ID
- <20180411034941.GA63158@syl.local>
- In-Reply-To
- <xmqq1sfmzbbg.fsf@gitster-ct.c.googlers.com>
On Wed, Apr 11, 2018 at 12:11:47PM +0900, Junio C Hamano wrote:
Show 20 quoted lines
> Taylor Blau <me@ttaylorr.com> writes:
>
> >> > +#define OPT_CALLBACK_VALUE(s, l, h, f, i) \
> >> > + { OPTION_CALLBACK, (s), (l), NULL, NULL, (h), PARSE_OPT_NOARG | \
> >> > + PARSE_OPT_NONEG, (f), (i) }
> >> > +
> >> > +static struct option builtin_config_options[];
> >>
> >> OK. I am not sure if OPT_CALLBACK_VALUE() needs to take 'f', as you
> >> always pass the option_parse_type function to it.
> >
> > That's fair. I left this in as an indication that something like this
> > _might_ want to make its way into parse-options.h as a general-purpose
> > utility, but was not yet ready to do so. Thus, I defined it inside
> > builtin/config.c.
>
> I understood the reasoning, but as your current verdict is that this
> is not yet ready for parse-options.[ch], I think it is probably
> preferrable to reduce repeated passing of the same function to the
> macro, at least while it is in this builgin/config.c file.Ah, that seems fair to me. I have removed the duplicate 'f' parameter and all of the option_parse_type()'s in builtin/config.c within my local copy, and will happily include these changes in the subsequent re-roll.
I'll wait for more feedback before sending this, however.
Thanks, Taylor