| Message ID | 20240206225122.1095766-4-shyamthakkar001@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | add-patch: '@' as a synonym for 'HEAD' | expand |
Hi Ghanshyam On 06/02/2024 22:50, Ghanshyam Thakkar wrote: > The comment suggested to compare commit objects instead of string > comparison to 'HEAD' for supporting more ways of saying 'HEAD' (e.g. > '@'). However, this approach would also count a non-checked out branch > pointing to same commit as HEAD, as HEAD. This would cause confusion to > the user. I forgot to say before, but I think this could be squashed into the next patch so we remove the comment at the same time as fixing the issue it describes. Best Wishes Phillip > Junio described it best as[1]: > > "Users may consider 'HEAD' and '@' the same and > may want them to behave the same way, but the user, when explicitly > naming '$branch', means they want to "check contents out of that > OTHER thing named '$branch', not the current branch"; it may or > may not happen to be pointing at the same commit as HEAD, but if > the user meant to say "check contents out of the current commit, > (partially) reverting the local changes I have", the user would have > said HEAD. After all, the user may not even be immediately aware > that '$branch' happens to point at the same commit as HEAD." > > [1]: https://lore.kernel.org/git/xmqqmssohu69.fsf@gitster.g/ > > Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> > --- > add-patch.c | 8 -------- > 1 file changed, 8 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/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
The comment suggested to compare commit objects instead of string comparison to 'HEAD' for supporting more ways of saying 'HEAD' (e.g. '@'). However, this approach would also count a non-checked out branch pointing to same commit as HEAD, as HEAD. This would cause confusion to the user. Junio described it best as[1]: "Users may consider 'HEAD' and '@' the same and may want them to behave the same way, but the user, when explicitly naming '$branch', means they want to "check contents out of that OTHER thing named '$branch', not the current branch"; it may or may not happen to be pointing at the same commit as HEAD, but if the user meant to say "check contents out of the current commit, (partially) reverting the local changes I have", the user would have said HEAD. After all, the user may not even be immediately aware that '$branch' happens to point at the same commit as HEAD." [1]: https://lore.kernel.org/git/xmqqmssohu69.fsf@gitster.g/ Signed-off-by: Ghanshyam Thakkar <shyamthakkar001@gmail.com> --- add-patch.c | 8 -------- 1 file changed, 8 deletions(-)