add-patch: classify '@' as a synonym for 'HEAD'

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/add-patch.c b/add-patch.c
index 79eda16..68f525b 100644 (file)
--- 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 a6e3093..067c251 100644 (file)
--- 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 8390bfe..f0bf29a 100644 (file)
--- 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 747eb55..c4f9bf0 100755 (executable)
--- 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 8202ef8..bce284c 100755 (executable)
--- 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 b5c5c0f..3dc9184 100755 (executable)
--- 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 05079c7..453872c 100755 (executable)
--- a/t/t7105-reset-patch.sh
+++ b/t/t7105-reset-patch.sh
@@ -26,12 +26,16 @@ 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" '
+               set_and_save_state dir/foo work work &&
+               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 &&