Re: [PATCH] parse-options: fix SunCC compiler warning

From: Duy Nguyen <pclouds@gmail.com>
Date: Dec 11, 2018, 15:40 UTC
Message-ID: <CACsJy8BFn7xbJO8G_mTeeHBbZ3J52ZQkSAyrmyxe=BfVaapPEA@mail.gmail.com>
In-Reply-To: <xmqqk1kgn73c.fsf@gitster-ct.c.googlers.com>

On Tue, Dec 11, 2018 at 3:13 AM Junio C Hamano <gitster@pobox.com> wrote:

Show 18 quoted lines

>
> Duy Nguyen <pclouds@gmail.com> writes:
>
> > The reason it's in parse_options_step() is because -h is also handled
> > in there. Although -h does not bury exit() deep in the call chain. So
> > how about this as a replacement?
>
> So just like -h returns PARSE_OPT_HELP and causes the calling
> parse_options() to exit(129), the new _COMPLETE can be used to
> trigger an early & successful return?
>
> We'd also have to cover builtin/{blame,shortlog,update-index}.c
> where they switch() on the return value from parse_options_step(),
> but other than that, it probably is a better approach.  The users of
> parse_options_step() that avoid parse_options() want to handle the
> various "we stopped by hitting a non-option" cases in their own
> ways, so treating this special action the same way as -h is treated
> would be sensible.

Yeah, I saw that shortly after sending the patch.

> We _might_ want to standardize some of the case
> arms in these custom switch statements that appear in these three
> built-in command implementations, but that can be done more easily
> if this step is done like you showed below, I think.

I have a better plan: stop exposing parse-options loop to outside. What all these commands need is the ability to deal with unknown options (or non-options in update-index case). They could register callbacks to deal with those and keep calling parse_options() instead. I'm converting rev-list to use parse_options() and as an intermediate step, this callback idea should work well. The end game though is a single parse_options() call that covers everything in, say, git-log. But we will see.

-- 
Duy