On Tue, Dec 11, 2018 at 3:13 AM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Duy Nguyen <pclouds@xxxxxxxxx> 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
