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

[PATCH 3/5] sha1_name.c: simplify @-parsing in get_sha1_basic()

From
Ramkumar Ramachandra <artagnon@gmail.com>
Date
May 1, 2013, 16:20 UTC
Message-ID
<1367425235-14998-4-git-send-email-artagnon@gmail.com>
In-Reply-To
<1367425235-14998-1-git-send-email-artagnon@gmail.com>

Presently, the logic for @-parsing is horribly convoluted. Prove that it is very straightforward by laying out the three cases (@{N}, @{u[upstream], and @{-N}) and explaining how each case should be handled in comments. All tests pass, and no functional changes have been introduced.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 sha1_name.c | 65 ++++++++++++++++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 20 deletions(-)
diff --git a/sha1_name.c b/sha1_name.c
index be1d12c..f30e344 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -463,9 +463,26 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		 */
 		for (at = len - 4; at >= 0; at--) {
 			if (str[at] == '@' && str[at+1] == '{') {
+				/* Set reflog_len only if we
+				 * don't see a @{u[pstream]}.  The
+				 * @{N} and @{-N} forms have to do
+				 * with reflog digging.
+				 */
+
+				/* Setting len to at means that we are
+				 * only going to process the part
+				 * before the @ until we reach "if
+				 * (reflog)" at the end of the
+				 * function.  That is only applicable
+				 * in the @{N} case; in the @{-N} and
+				 * @{u[pstream]} cases, we will run it
+				 * through interpret_branch_name().
+				 */
+
 				if (!upstream_mark(str + at, len - at)) {
 					reflog_len = (len-1) - (at+2);
-					len = at;
+					if (str[at + 2] != '-')
+						len = at;
 				}
 				break;
 			}
@@ -476,22 +493,34 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	if (len && ambiguous_path(str, len))
 		return -1;
 
-	if (!len && reflog_len) {
-		struct strbuf buf = STRBUF_INIT;
-		int ret;
-		/* try the @{-N} syntax for n-th checkout */
-		ret = interpret_branch_name(str+at, &buf);
-		if (ret > 0) {
-			/* substitute this branch name and restart */
-			return get_sha1_1(buf.buf, buf.len, sha1, 0);
-		} else if (ret == 0) {
-			return -1;
+	if (reflog_len) {
+		if (!len)
+			/* In the @{N} case where there's nothing
+			 * before the @.
+			 */
+			refs_found = dwim_log("HEAD", 4, sha1, &real_ref);
+		else {
+			/* The @{N} case where there is something
+			 * before the @ for dwim_log to figure out,
+			 * and the @{-N} case.
+			 */
+			refs_found = dwim_log(str, len, sha1, &real_ref);
+
+			if (!refs_found && str[2] == '-') {
+				/* The @{-N} case that resolves to a
+				 * detached HEAD (ie. not a ref)
+				 */
+				struct strbuf buf = STRBUF_INIT;
+				if (interpret_branch_name(str, &buf) > 0) {
+					get_sha1_hex(buf.buf, sha1);
+					refs_found = 1;
+					reflog_len = 0;
+				}
+				strbuf_release(&buf);
+			}
 		}
-		/* allow "@{...}" to mean the current branch reflog */
-		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
-	} else if (reflog_len)
-		refs_found = dwim_log(str, len, sha1, &real_ref);
-	else
+	} else
+		/* The @{u[pstream]} case */
 		refs_found = dwim_ref(str, len, sha1, &real_ref);
 
 	if (!refs_found)
@@ -506,10 +535,6 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		unsigned long co_time;
 		int co_tz, co_cnt;
 
-		/* a @{-N} placed anywhere except the start is an error */
-		if (str[at+2] == '-')
-			return -1;
-
 		/* Is it asking for N-th entry, or approxidate? */
 		for (i = nth = 0; 0 <= nth && i < reflog_len; i++) {
 			char ch = str[at+2+i];
-- 
1.8.3.rc0.24.g6456091
Previous: Junio C HamanoNext: Felipe Contreras
Message 25 of 49 in “A natural solution to the @ -> HEAD problem”
  1. 0/5 A natural solution to the @ -> HEAD problemRamkumar Ramachandra, May 1, 2013
  2. 1/5 t1508 (at-combinations): more tests; document failuresRamkumar Ramachandra, May 1, 2013
  3. Junio C HamanoMay 1, 2013
  4. Ramkumar RamachandraMay 1, 2013
  5. Jeff KingMay 1, 2013
  6. Ramkumar RamachandraMay 1, 2013
  7. Junio C HamanoMay 1, 2013
  8. Felipe ContrerasMay 2, 2013
  9. Ramkumar RamachandraMay 2, 2013
  10. Felipe ContrerasMay 2, 2013
  11. Ramkumar RamachandraMay 2, 2013
  12. Ramkumar RamachandraMay 2, 2013
  13. Felipe ContrerasMay 2, 2013
  14. Ramkumar RamachandraMay 2, 2013
  15. Felipe ContrerasMay 2, 2013
  16. Ramkumar RamachandraMay 2, 2013
  17. Felipe ContrerasMay 2, 2013
  18. Ramkumar RamachandraMay 2, 2013
  19. David AguilarMay 4, 2013
  20. David AguilarMay 4, 2013
  21. 2/5 sha1_name.c: don't waste cycles in the @-parsing loopRamkumar Ramachandra, May 1, 2013
  22. Felipe ContrerasMay 1, 2013
  23. Ramkumar RamachandraMay 1, 2013
  24. Junio C HamanoMay 2, 2013
  25. 3/5 sha1_name.c: simplify @-parsing in get_sha1_basic()Ramkumar Ramachandra, May 1, 2013
  26. Felipe ContrerasMay 1, 2013
  27. Ramkumar RamachandraMay 1, 2013
  28. Jonathan NiederMay 1, 2013
  29. Ramkumar RamachandraMay 1, 2013
  30. Felipe ContrerasMay 1, 2013
  31. Ramkumar RamachandraMay 1, 2013
  32. Felipe ContrerasMay 1, 2013
  33. Ramkumar RamachandraMay 1, 2013
  34. Felipe ContrerasMay 1, 2013
  35. Ramkumar RamachandraMay 1, 2013
  36. 4/5 remote.c: teach branch_get() to treat symrefs other than HEADRamkumar Ramachandra, May 1, 2013
  37. Felipe ContrerasMay 1, 2013
  38. Ramkumar RamachandraMay 1, 2013
  39. Felipe ContrerasMay 1, 2013
  40. Ramkumar RamachandraMay 1, 2013
  41. Felipe ContrerasMay 1, 2013
  42. Ramkumar RamachandraMay 1, 2013
  43. Felipe ContrerasMay 1, 2013
  44. 5/5 refs.c: make @ a pseudo-ref alias to HEADRamkumar Ramachandra, May 1, 2013
  45. Felipe ContrerasMay 1, 2013
  46. Ramkumar RamachandraMay 1, 2013
  47. Felipe ContrerasMay 1, 2013
  48. Ramkumar RamachandraMay 1, 2013
  49. Ramkumar RamachandraMay 1, 2013

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.