Re: [PATCH 1/4 v4] revision.c: do not update argv with unknown option
- From
Matthieu Moy <matthieu.moy@grenoble-inp.fr>
- Date
- Feb 16, 2017, 18:22 UTC
- Message-ID
- <vpqwpcqgfmw.fsf@anie.imag.fr>
- In-Reply-To
- <xmqqwpcqxay0.fsf@gitster.mtv.corp.google.com>
Junio C Hamano <gitster@pobox.com> writes:
Show 27 quoted lines
> 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/