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>
This commit is contained in:
Ghanshyam Thakkar
2024-02-13 05:35:29 +05:30
committed by Junio C Hamano
parent 2a540e432f
commit 5a8ed3fe45
7 changed files with 63 additions and 41 deletions

View File

@ -1729,14 +1729,6 @@ int run_add_p(struct repository *r, enum add_p_mode mode,
if (mode == ADD_P_STASH) if (mode == ADD_P_STASH)
s.mode = &patch_mode_stash; s.mode = &patch_mode_stash;
else if (mode == ADD_P_RESET) { 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")) if (!revision || !strcmp(revision, "HEAD"))
s.mode = &patch_mode_reset_head; s.mode = &patch_mode_reset_head;
else else

View File

@ -1224,7 +1224,9 @@ static void setup_new_branch_info_and_source_tree(
struct tree **source_tree = &opts->source_tree; struct tree **source_tree = &opts->source_tree;
struct object_id branch_rev; 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); setup_branch_path(new_branch_info);
if (!check_refname_format(new_branch_info->path, 0) && if (!check_refname_format(new_branch_info->path, 0) &&

View File

@ -281,7 +281,9 @@ static void parse_args(struct pathspec *pathspec,
verify_filename(prefix, argv[0], 1); 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, parse_pathspec(pathspec, 0,
PATHSPEC_PREFER_FULL | PATHSPEC_PREFER_FULL |

View File

@ -38,26 +38,32 @@ test_expect_success 'git checkout -p with staged changes' '
verify_state dir/foo index index verify_state dir/foo index index
' '
test_expect_success 'git checkout -p HEAD with NO staged changes: abort' ' for opt in "HEAD" "@"
set_and_save_state dir/foo work head && do
test_write_lines n y n | git checkout -p HEAD && test_expect_success "git checkout -p $opt with NO staged changes: abort" '
verify_saved_state bar && set_and_save_state dir/foo work head &&
verify_saved_state dir/foo 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 HEAD with NO staged changes: apply' ' test_expect_success "git checkout -p $opt with NO staged changes: apply" '
test_write_lines n y y | git checkout -p HEAD && test_write_lines n y y | git checkout -p $opt >output &&
verify_saved_state bar && verify_saved_state bar &&
verify_state dir/foo head head verify_state dir/foo head head &&
' test_grep "Discard" output
'
test_expect_success 'git checkout -p HEAD with change already staged' ' test_expect_success "git checkout -p $opt with change already staged" '
set_state dir/foo index index && set_state dir/foo index index &&
# the third n is to get out in case it mistakenly does not apply # the third n is to get out in case it mistakenly does not apply
test_write_lines n y n | git checkout -p HEAD && test_write_lines n y n | git checkout -p $opt >output &&
verify_saved_state bar && verify_saved_state bar &&
verify_state dir/foo head head verify_state dir/foo head head &&
' test_grep "Discard" output
'
done
test_expect_success 'git checkout -p HEAD^...' ' test_expect_success 'git checkout -p HEAD^...' '
# the third n is to get out in case it mistakenly does not apply # the third n is to get out in case it mistakenly does not apply

View File

@ -45,6 +45,18 @@ test_expect_success 'checkout branch does not detach' '
check_not_detached 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' ' test_expect_success 'checkout tag detaches' '
reset && reset &&
git checkout tag && git checkout tag &&

View File

@ -44,13 +44,17 @@ test_expect_success PERL 'git restore -p with staged changes' '
verify_state dir/foo index index verify_state dir/foo index index
' '
test_expect_success PERL 'git restore -p --source=HEAD' ' for opt in "HEAD" "@"
set_state dir/foo work index && do
# the third n is to get out in case it mistakenly does not apply test_expect_success PERL "git restore -p --source=$opt" '
test_write_lines n y n | git restore -p --source=HEAD && set_state dir/foo work index &&
verify_saved_state bar && # the third n is to get out in case it mistakenly does not apply
verify_state dir/foo head index 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^' ' test_expect_success PERL 'git restore -p --source=HEAD^' '
set_state dir/foo work index && set_state dir/foo work index &&

View File

@ -26,12 +26,16 @@ test_expect_success PERL 'saying "n" does nothing' '
verify_saved_state bar verify_saved_state bar
' '
test_expect_success PERL 'git reset -p' ' for opt in "HEAD" "@" ""
test_write_lines n y | git reset -p >output && do
verify_state dir/foo work head && test_expect_success PERL "git reset -p $opt" '
verify_saved_state bar && set_and_save_state dir/foo work work &&
test_grep "Unstage" output 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_expect_success PERL 'git reset -p HEAD^' '
test_write_lines n y | git reset -p HEAD^ >output && test_write_lines n y | git reset -p HEAD^ >output &&