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

[PATCH v5 1/2] add-patch: classify '@' as a synonym for 'HEAD'

From
Ghanshyam Thakkar <shyamthakkar001@gmail.com>
Date
Feb 11, 2024, 20:20 UTC
Message-ID
<20240211202035.7196-4-shyamthakkar001@gmail.com>
In-Reply-To
<20240206225122.1095766-3-shyamthakkar001@gmail.com>

Currently, (restore, checkout, reset) commands correctly take '@' as a synonym for 'HEAD'. However, in patch mode different prompts/messages are given on command line due to patch mode machinery not considering '@' to be a synonym for 'HEAD' due to literal string comparison with the word 'HEAD', and therefore assigning patch_mode_($command)_nothead and triggering reverse mode (-R in diff-index). The NEEDSWORK comment suggested comparing commit objects to get around this. However, doing so would also take a non-checked out branch pointing to the same commit as HEAD, as HEAD. This would cause confusion to the user.

Therefore, after parsing '@', replace it with 'HEAD' as reasonably early as possible. This also solves another problem of disparity between 'git checkout HEAD' and 'git checkout @' (latter detaches at the HEAD commit and the former does not).

Trade-offs:
- Some of the errors would show the revision argument as 'HEAD' when
  given '@'. This should be fine, as most users who probably use '@'
  would be aware that it is a shortcut for 'HEAD' and most probably
  used to use 'HEAD'. There is also relevant documentation in
  'gitrevisions' manpage about '@' being the shortcut for 'HEAD'. Also,
  the simplicity of the solution far outweighs this cost.
- Consider '@' as a shortcut for 'HEAD' even if 'refs/heads/@' exists
  at a different commit. Naming a branch '@' is an obvious foot-gun and
  many existing commands already take '@' for 'HEAD' even if
  'refs/heads/@' exists at a different commit or does not exist at all
  (e.g. 'git log @', 'git push origin @' etc.). Therefore this is an
  existing assumption and should not be a problem.
Helped-by: Junio C Hamano <gitster@pobox.com>
Helped-by: Phillip Wood <phillip.wood123@gmail.com>
Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com>
---
 add-patch.c                |  8 -------
 builtin/checkout.c         |  4 +++-
 builtin/reset.c            |  4 +++-
 t/t2016-checkout-patch.sh  | 46 +++++++++++++++++++++-----------------
 t/t2020-checkout-detach.sh | 12 ++++++++++
 t/t2071-restore-patch.sh   | 18 +++++++++------
 t/t7105-reset-patch.sh     | 15 ++++++++-----
 7 files changed, 64 insertions(+), 43 deletions(-)
diff --git a/add-patch.c b/add-patch.c
index 79eda168eb..68f525b35c 100644
--- a/add-patch.c
+++ b/add-patch.c
@@ -1729,14 +1729,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
 	if (mode == ADD_P_STASH)
 		s.mode = &patch_mode_stash;
 	else if (mode == ADD_P_RESET) {
-		/*
-		 * NEEDSWORK: Instead of comparing to the literal "HEAD",
-		 * compare the commit objects instead so that other ways of
-		 * saying the same thing (such as "@") are also handled
-		 * appropriately.
-		 *
-		 * This applies to the cases below too.
-		 */
 		if (!revision || !strcmp(revision, "HEAD"))
 			s.mode = &patch_mode_reset_head;
 		else
diff --git a/builtin/checkout.c b/builtin/checkout.c
index a6e30931b5..067c251933 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -1224,7 +1224,9 @@ static void setup_new_branch_info_and_source_tree(
 	struct tree **source_tree = &opts->source_tree;
 	struct object_id branch_rev;
 
-	new_branch_info->name = xstrdup(arg);
+	/* treat '@' as a shortcut for 'HEAD' */
+	new_branch_info->name = !strcmp(arg, "@") ? xstrdup("HEAD") :
+						    xstrdup(arg);
 	setup_branch_path(new_branch_info);
 
 	if (!check_refname_format(new_branch_info->path, 0) &&
diff --git a/builtin/reset.c b/builtin/reset.c
index 8390bfe4c4..f0bf29a478 100644
--- a/builtin/reset.c
+++ b/builtin/reset.c
@@ -281,7 +281,9 @@ static void parse_args(struct pathspec *pathspec,
 			verify_filename(prefix, argv[0], 1);
 		}
 	}
-	*rev_ret = rev;
+
+	/* treat '@' as a shortcut for 'HEAD' */
+	*rev_ret = !strcmp("@", rev) ? "HEAD" : rev;
 
 	parse_pathspec(pathspec, 0,
 		       PATHSPEC_PREFER_FULL |
diff --git a/t/t2016-checkout-patch.sh b/t/t2016-checkout-patch.sh
index 747eb5563e..c4f9bf09aa 100755
--- a/t/t2016-checkout-patch.sh
+++ b/t/t2016-checkout-patch.sh
@@ -38,26 +38,32 @@ test_expect_success 'git checkout -p with staged changes' '
 	verify_state dir/foo index index
 '
 
-test_expect_success 'git checkout -p HEAD with NO staged changes: abort' '
-	set_and_save_state dir/foo work head &&
-	test_write_lines n y n | git checkout -p HEAD &&
-	verify_saved_state bar &&
-	verify_saved_state dir/foo
-'
-
-test_expect_success 'git checkout -p HEAD with NO staged changes: apply' '
-	test_write_lines n y y | git checkout -p HEAD &&
-	verify_saved_state bar &&
-	verify_state dir/foo head head
-'
-
-test_expect_success 'git checkout -p HEAD with change already staged' '
-	set_state dir/foo index index &&
-	# the third n is to get out in case it mistakenly does not apply
-	test_write_lines n y n | git checkout -p HEAD &&
-	verify_saved_state bar &&
-	verify_state dir/foo head head
-'
+for opt in "HEAD" "@"
+do
+	test_expect_success "git checkout -p $opt with NO staged changes: abort" '
+		set_and_save_state dir/foo work head &&
+		test_write_lines n y n | git checkout -p $opt >output &&
+		verify_saved_state bar &&
+		verify_saved_state dir/foo &&
+		test_grep "Discard" output
+	'
+
+	test_expect_success "git checkout -p $opt with NO staged changes: apply" '
+		test_write_lines n y y | git checkout -p $opt >output &&
+		verify_saved_state bar &&
+		verify_state dir/foo head head &&
+		test_grep "Discard" output
+	'
+
+	test_expect_success "git checkout -p $opt with change already staged" '
+		set_state dir/foo index index &&
+		# the third n is to get out in case it mistakenly does not apply
+		test_write_lines n y n | git checkout -p $opt >output &&
+		verify_saved_state bar &&
+		verify_state dir/foo head head &&
+		test_grep "Discard" output
+	'
+done
 
 test_expect_success 'git checkout -p HEAD^...' '
 	# the third n is to get out in case it mistakenly does not apply
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 8202ef8c74..bce284c297 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -45,6 +45,18 @@ test_expect_success 'checkout branch does not detach' '
 	check_not_detached
 '
 
+for opt in "HEAD" "@"
+do
+	test_expect_success "checkout $opt no-op/don't detach" '
+		reset &&
+		cat .git/HEAD >expect &&
+		git checkout $opt &&
+		cat .git/HEAD >actual &&
+		check_not_detached &&
+		test_cmp expect actual
+	'
+done
+
 test_expect_success 'checkout tag detaches' '
 	reset &&
 	git checkout tag &&
diff --git a/t/t2071-restore-patch.sh b/t/t2071-restore-patch.sh
index b5c5c0ff7e..3dc9184b4a 100755
--- a/t/t2071-restore-patch.sh
+++ b/t/t2071-restore-patch.sh
@@ -44,13 +44,17 @@ test_expect_success PERL 'git restore -p with staged changes' '
 	verify_state dir/foo index index
 '
 
-test_expect_success PERL 'git restore -p --source=HEAD' '
-	set_state dir/foo work index &&
-	# the third n is to get out in case it mistakenly does not apply
-	test_write_lines n y n | git restore -p --source=HEAD &&
-	verify_saved_state bar &&
-	verify_state dir/foo head index
-'
+for opt in "HEAD" "@"
+do
+	test_expect_success PERL "git restore -p --source=$opt" '
+		set_state dir/foo work index &&
+		# the third n is to get out in case it mistakenly does not apply
+		test_write_lines n y n | git restore -p --source=$opt >output &&
+		verify_saved_state bar &&
+		verify_state dir/foo head index &&
+		test_grep "Discard" output
+	'
+done
 
 test_expect_success PERL 'git restore -p --source=HEAD^' '
 	set_state dir/foo work index &&
diff --git a/t/t7105-reset-patch.sh b/t/t7105-reset-patch.sh
index 05079c7246..0f597416d8 100755
--- a/t/t7105-reset-patch.sh
+++ b/t/t7105-reset-patch.sh
@@ -26,12 +26,15 @@ test_expect_success PERL 'saying "n" does nothing' '
 	verify_saved_state bar
 '
 
-test_expect_success PERL 'git reset -p' '
-	test_write_lines n y | git reset -p >output &&
-	verify_state dir/foo work head &&
-	verify_saved_state bar &&
-	test_grep "Unstage" output
-'
+for opt in "HEAD" "@" ""
+do
+	test_expect_success PERL "git reset -p $opt" '
+		test_write_lines n y | git reset -p $opt >output &&
+		verify_state dir/foo work head &&
+		verify_saved_state bar &&
+		test_grep "Unstage" output
+	'
+done
 
 test_expect_success PERL 'git reset -p HEAD^' '
 	test_write_lines n y | git reset -p HEAD^ >output &&

-- 
2.43.0
Previous: Ghanshyam ThakkarNext: Junio C Hamano
Message 26 of 46 in “add-patch: compare object id”
  1. Ghanshyam ThakkarJan 28, 2024
  2. 1/2 add-patch: compare object id instead of literal stringGhanshyam Thakkar, Jan 28, 2024
  3. Patrick SteinhardtJan 29, 2024
  4. Ghanshyam ThakkarJan 30, 2024
  5. Junio C HamanoJan 30, 2024
  6. Junio C HamanoJan 29, 2024
  7. Junio C HamanoJan 29, 2024
  8. Ghanshyam ThakkarJan 30, 2024
  9. 2/2 checkout: remove HEAD special caseGhanshyam Thakkar, Jan 28, 2024
  10. Patrick SteinhardtJan 29, 2024
  11. 0/2 add-patch: Support '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 2, 2024
  12. 1/2 add-patch: remove non-relevant NEEDSWORK commentGhanshyam Thakkar, Feb 2, 2024
  13. 2/2 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 2, 2024
  14. Junio C HamanoFeb 2, 2024
  15. Junio C HamanoFeb 2, 2024
  16. Ghanshyam ThakkarFeb 2, 2024
  17. Ghanshyam ThakkarFeb 2, 2024
  18. Ghanshyam ThakkarFeb 2, 2024
  19. 0/2 add-patch: '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 3, 2024
  20. 0/3 '@' as a synonym for 'HEAD' in patch modeGhanshyam Thakkar, Feb 6, 2024
  21. 0/2 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 11, 2024
  22. 0/2 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 13, 2024
  23. Phillip WoodFeb 14, 2024
  24. 1/2 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 13, 2024
  25. 2/2 add -p tests: remove PERL prerequisitesGhanshyam Thakkar, Feb 13, 2024
  26. 1/2 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 11, 2024
  27. Junio C HamanoFeb 12, 2024
  28. 2/2 add -p tests: remove PERL prerequisitesGhanshyam Thakkar, Feb 11, 2024
  29. 1/3 add-patch: remove unnecessary NEEDSWORK commentGhanshyam Thakkar, Feb 6, 2024
  30. Phillip WoodFeb 7, 2024
  31. 2/3 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 6, 2024
  32. Junio C HamanoFeb 7, 2024
  33. Phillip WoodFeb 7, 2024
  34. Ghanshyam ThakkarFeb 9, 2024
  35. 3/3 add -p tests: remove Perl prerequisiteGhanshyam Thakkar, Feb 6, 2024
  36. Phillip WoodFeb 7, 2024
  37. Phillip WoodFeb 7, 2024
  38. Junio C HamanoFeb 7, 2024
  39. Eric SunshineFeb 7, 2024
  40. 1/2 add-patch: remove unnecessary NEEDSWORK commentGhanshyam Thakkar, Feb 3, 2024
  41. 2/2 add-patch: classify '@' as a synonym for 'HEAD'Ghanshyam Thakkar, Feb 3, 2024
  42. Junio C HamanoFeb 3, 2024
  43. Ghanshyam ThakkarFeb 5, 2024
  44. Phillip WoodFeb 5, 2024
  45. Ghanshyam ThakkarFeb 5, 2024
  46. Junio C HamanoFeb 5, 2024

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.