On Wed, Apr 11, 2018 at 12:11:47PM +0900, Junio C Hamano wrote:

Taylor Blau [off-list ref] writes:

quoted

quoted

quoted

+#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