Re: [PATCH 1/4 v4] revision.c: do not update argv with unknown option

[Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>]

Junio C Hamano <gitster@pobox.com> writes:

> Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> writes:
>
>> Siddharth Kannan <kannan.siddharth12@gmail.com> writes:
>>
>>> handle_revision_opt() tries to recognize and handle the given argument. If an
>>> option was unknown to it, it used to add the option to unkv[(*unkc)++].  This
>>> increment of unkc causes the variable in the caller to change.
>>>
>>> Teach handle_revision_opt to not update unknown arguments inside unkc anymore.
>>> This is now the responsibility of the caller.
>>>
>>> There are two callers of this function:
>>>
>>> 1. setup_revision: Changes have been made so that setup_revision will now
>>> update the unknown option in argv
>>
>> You're writting "Changes have been made", but I did not see any up to
>> this point in the series.
>
> Actually, I think you misread the patch and explanation.
> handle_revision_opt() used to be responsible for stuffing unknown
> ones to unkv[] array passed from the caller even when it returns 0
> (i.e. "I do not know what they are" case, as opposed to "I know what
> they are, I am not handling them here and leaving them in unkv[]"
> case--the latter returns non-zero).  The first hunk makes the
> function stop doing so, and to compensate, the second hunk, which is
> in setup_revisions()

Indeed, I misread the patch. The explanation could be a little bit more
"tired-reviewer-proof" by not using a past tone, perhaps

1. setup_revision, which is changed to ...

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/