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

[PATCH 04/15] handle_revision_arg: hoist ".." check out of range parsing

From
Jeff King <peff@peff.net>
Date
May 19, 2017, 12:51 UTC
Message-ID
<20170519125139.rrrlwpptqsvigus3@sigill.intra.peff.net>
In-Reply-To
<20170519124651.4q7waz75rmzfopgn@sigill.intra.peff.net>

Since 003c84f6d (specifying ranges: we did not mean to make ".." an empty set, 2011-05-02), we treat the argument ".." specially. We detect it by noticing that both sides of the range are empty, and that this is a non-symmetric two-dot range.

While correct, this makes the code overly complicated. We can just detect ".." up front before we try to do further parsing. This avoids having to de-munge the NUL from dotdot, and lets us eliminate an extra const array (which we needed only to do direct pointer comparisons).

It also removes the one code path from the range-parsing conditional that requires us to return -1. That will make it simpler to pull the dotdot parsing out into its own function.

Signed-off-by: Jeff King <peff@peff.net>
---
There's a similar call in rev-parse which could maybe take the same
cleanup. I didn't bother with it because my main goal was removing an
obstacle to factoring out the dotdot parsing (though in an ideal world,
rev-parse would actually use the same dotdot parsing code; I've no idea
how complicated that would be).
 revision.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)
diff --git a/revision.c b/revision.c
index 9c683874d..dec06ff8b 100644
--- a/revision.c
+++ b/revision.c
@@ -1443,6 +1443,14 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 
 	flags = flags & UNINTERESTING ? flags | BOTTOM : flags & ~BOTTOM;
 
+	if (!cant_be_filename && !strcmp(arg, "..")) {
+		/*
+		 * Just ".."?  That is not a range but the
+		 * pathspec for the parent directory.
+		 */
+		return -1;
+	}
+
 	dotdot = strstr(arg, "..");
 	if (dotdot) {
 		unsigned char from_sha1[20];
@@ -1450,27 +1458,15 @@ int handle_revision_arg(const char *arg_, struct rev_info *revs, int flags, unsi
 		const char *this = arg;
 		int symmetric = *next == '.';
 		unsigned int flags_exclude = flags ^ (UNINTERESTING | BOTTOM);
-		static const char head_by_default[] = "HEAD";
 		unsigned int a_flags;
 
 		*dotdot = 0;
 		next += symmetric;
 
 		if (!*next)
-			next = head_by_default;
+			next = "HEAD";
 		if (dotdot == arg)
-			this = head_by_default;
-		if (this == head_by_default && next == head_by_default &&
-		    !symmetric) {
-			/*
-			 * Just ".."?  That is not a range but the
-			 * pathspec for the parent directory.
-			 */
-			if (!cant_be_filename) {
-				*dotdot = '.';
-				return -1;
-			}
-		}
+			this = "HEAD";
 		if (!get_sha1_committish(this, from_sha1) &&
 		    !get_sha1_committish(next, sha1)) {
 			struct object *a_obj, *b_obj;
-- 
2.13.0.219.g63f6bc368
Previous: Jeff KingNext: Jeff King
Message 19 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.