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

Re: [PATCH 03/15] handle_revision_arg: stop using "dotdot" as a generic pointer

From
Junio C Hamano <gitster@pobox.com>
Date
May 24, 2017, 02:45 UTC
Message-ID
<xmqqzie3gegc.fsf@gitster.mtv.corp.google.com>
In-Reply-To
<20170519125006.gbvzr2dqrcycmpx7@sigill.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 19 quoted lines
> The handle_revision_arg() function has a "dotdot" variable
> that it uses to find a ".." or "..." in the argument. If we
> don't find one, we look for other marks, like "^!". But we
> just keep re-using the "dotdot" variable, which is
> confusing.
>
> Let's introduce a separate "mark" variable that can be used
> for these other marks. They still reuse the same variable,
> but at least the name is no longer actively misleading.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> It may make sense to pull each of these into its own helper. I didn't
> really look because they're so small, and because the return semantics
> seemed confusing to me. Some of them return, and some of them keep
> parsing. Some of them restore the NUL they overwrite, and some do not.
>
> I didn't dig in to see if there are weird corner cases where they
> misbehave.

I do not quite know what corner cases you meant, but I agree that many places in the codepath we forget to restore "^" we temporarily overwrite. I suspect that none of them is deliberately leaving "^" unrestored and they are just being careless (or they truly do not care because they assume nobody will look at arg later).

And I think not restoring cannot be a correct thing to do. After all of these parsing, add_rev_cmdline() wants to see arg_ intact.

But let's keep reading; perhaps they are addressed in a later patch, or they are left as-is, and either is OK.

Previous: Jeff KingNext: Jeff King
Message 17 of 33 in “Demonstrate and partially work around a gitattributes problem”
  1. 0/2 Demonstrate and partially work around a gitattributes problemJohannes Schindelin, May 15, 2017
  2. 1/2 gitattributes: demonstrate that Git tries to read a bogus fileJohannes Schindelin, May 15, 2017
  3. 2/2 mingw: Suppress warning that <commit>:.gitattributes does not existJohannes Schindelin, May 15, 2017
  4. Jeff KingMay 16, 2017
  5. Jeff KingMay 16, 2017
  6. Junio C HamanoMay 17, 2017
  7. Jeff KingMay 17, 2017
  8. Johannes SchindelinMay 18, 2017
  9. Jeff KingMay 19, 2017
  10. 0/15 retain blob info for git diff HEAD:foo HEAD:barJeff King, May 19, 2017
  11. 01/15 handle_revision_arg: reset "dotdot" consistentlyJeff King, May 19, 2017
  12. Philip OakleyMay 20, 2017
  13. Jeff KingMay 23, 2017
  14. Philip OakleyMay 23, 2017
  15. 02/15 handle_revision_arg: simplify commit reference lookupsJeff King, May 19, 2017
  16. 03/15 handle_revision_arg: stop using "dotdot" as a generic pointerJeff King, May 19, 2017
  17. Junio C HamanoMay 24, 2017
  18. Jeff KingMay 24, 2017
  19. 04/15 handle_revision_arg: hoist ".." check out of range parsingJeff King, May 19, 2017
  20. 05/15 handle_revision_arg: add handle_dotdot() helperJeff King, May 19, 2017
  21. Junio C HamanoMay 24, 2017
  22. 06/15 sha1_name: consistently refer to object_context as "oc"Jeff King, May 19, 2017
  23. 07/15 get_sha1_with_context: always initialize oc->symlink_pathJeff King, May 19, 2017
  24. 08/15 get_sha1_with_context: dynamically allocate oc->pathJeff King, May 19, 2017
  25. 09/15 t4063: add tests of direct blob diffsJeff King, May 19, 2017
  26. 10/15 handle_revision_arg: record modes for "a..b" endpointsJeff King, May 19, 2017
  27. 11/15 handle_revision_arg: record paths for pending objectsJeff King, May 19, 2017
  28. 12/15 diff: pass whole pending entry in blobinfoJeff King, May 19, 2017
  29. 13/15 diff: use the word "path" instead of "name" for blobsJeff King, May 19, 2017
  30. 14/15 diff: use pending "path" if it is availableJeff King, May 19, 2017
  31. 15/15 diff: use blob path for blob/file diffsJeff King, May 19, 2017
  32. Junio C HamanoMay 24, 2017
  33. Jeff KingMay 24, 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.