Re: [PATCH] Use OPT_CALLBACK and OPT_CALLBACK_F

From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Denton Liu <liu.denton@gmail.com>,
	Git Mailing List <git@vger.kernel.org>,
	Andrew White <andrew.white@audinate.com>
Subject: Re: [PATCH] Use OPT_CALLBACK and OPT_CALLBACK_F
Date: Tue, 28 Apr 2020 10:34:26 -0700	[thread overview]
Message-ID: <xmqq368n4iql.fsf@gitster.c.googlers.com> (raw)
In-Reply-To: <20200428084543.GC2381876@coredump.intra.peff.net> (Jeff King's message of "Tue, 28 Apr 2020 04:45:43 -0400")

Jeff King <peff@peff.net> writes:

> On Tue, Apr 28, 2020 at 04:36:28AM -0400, Denton Liu wrote:
>
>> In the codebase, there are many options which use OPTION_CALLBACK in a
>> plain ol' struct definition. However, we have the OPT_CALLBACK and
>> OPT_CALLBACK_F macros which are meant to abstract these plain struct
>> definitions away. These macros are useful as they semantically signal to
>> developers that these are just normal callback option with nothing fancy
>> happening.
>
> I think this is worth doing. It's a little easier to read, and sets a
> better example anyone copying the code.

Sure.

And it would be much nicer if the patch actually applies.  I am
getting "SHA-1 information for builtin/push.c is unusable" from 
"am -3".

I'd reject the first hunk on builtin/push.c as that part should be
done with an updated version of the patch that is titled "push:
unset PARSE_OPT_OPTARG for --recurse-submodules" and apply the rest
as an independent clean-up patch.

Thanks, both.