git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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/
Previous: Junio C HamanoNext: Siddharth Kannan
Message 5 of 16 in “WIP: allow "-" as a shorthand for "previous branch"”
  1. 0/4 WIP: allow "-" as a shorthand for "previous branch"Siddharth Kannan, Feb 16, 2017
  2. 1/4 revision.c: do not update argv with unknown optionSiddharth Kannan, Feb 16, 2017
  3. Matthieu MoyFeb 16, 2017
  4. Junio C HamanoFeb 16, 2017
  5. Matthieu MoyFeb 16, 2017
  6. Siddharth KannanFeb 16, 2017
  7. 2/4 revision.c: swap if/else blocksSiddharth Kannan, Feb 16, 2017
  8. 3/4 revision.c: args starting with "-" might be a revisionSiddharth Kannan, Feb 16, 2017
  9. 4/4 sha1_name.c: teach get_sha1_1 "-" shorthand for "@{-1}"Siddharth Kannan, Feb 16, 2017
  10. Junio C HamanoFeb 16, 2017
  11. Siddharth KannanFeb 20, 2017
  12. Junio C HamanoFeb 20, 2017
  13. Siddharth KannanFeb 22, 2017
  14. Matthieu MoyFeb 16, 2017
  15. Junio C HamanoFeb 16, 2017
  16. Siddharth KannanFeb 16, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.