[PATCH] refspec: make @ a valid refspec

[PATCH] refspec: make @ a valid refspec

[Felipe Contreras]

Since commit 9ba89f484e git learned how to push to a remote branch using
the source @, for example:

  git push origin @:master

However, if the right-hand side is missing, the push fails:

  git push origin @

It is obvious what is the desired behavior, and allowing the push makes
things more consistent.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 refspec.c             | 5 ++++-
 t/t5511-refspec.sh    | 2 ++
 t/t5516-fetch-push.sh | 9 +++++++++
 3 files changed, 15 insertions(+), 1 deletion(-)

diff --git a/refspec.c b/refspec.c
index c49347c2d7..adbfb3283a 100644
--- a/refspec.c
+++ b/refspec.c
@@ -71,7 +71,10 @@ static int parse_refspec(struct refspec_item *item, const char *refspec, int fet
 	}
 
 	item->pattern = is_glob;
-	item->src = xstrndup(lhs, llen);
+	if (llen == 1 && *lhs == '@')
+		item->src = xstrdup("HEAD");
+	else
+		item->src = xstrndup(lhs, llen);
 	flags = REFNAME_ALLOW_ONELEVEL | (is_glob ? REFNAME_REFSPEC_PATTERN : 0);
 
 	if (item->negative) {
diff --git a/t/t5511-refspec.sh b/t/t5511-refspec.sh
index f541f30bc2..f808649de4 100755
--- a/t/t5511-refspec.sh
+++ b/t/t5511-refspec.sh
@@ -58,6 +58,8 @@ test_refspec fetch 'HEAD~4:refs/remotes/frotz/new'		invalid
 
 test_refspec push 'HEAD'
 test_refspec fetch 'HEAD'
+test_refspec push '@'
+test_refspec fetch '@'
 test_refspec push 'refs/heads/ nitfol'				invalid
 test_refspec fetch 'refs/heads/ nitfol'				invalid
 
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index d11382f769..12c0c7c06b 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -501,6 +501,15 @@ test_expect_success 'push with config remote.*.push = HEAD' '
 	check_push_result testrepo $the_first_commit heads/local
 '
 
+test_expect_success 'push with @' '
+
+	mk_test testrepo heads/master &&
+	git checkout master &&
+	git push testrepo @ &&
+	check_push_result testrepo $the_commit heads/master
+
+'
+
 test_expect_success 'push with remote.pushdefault' '
 	mk_test up_repo heads/master &&
 	mk_test down_repo heads/master &&
-- 
2.29.2

Re: [PATCH] refspec: make @ a valid refspec

[Jeff King]

On Sun, Nov 22, 2020 at 10:46:41AM -0600, Felipe Contreras wrote:

> Since commit 9ba89f484e git learned how to push to a remote branch using
> the source @, for example:
> 
>   git push origin @:master
> 
> However, if the right-hand side is missing, the push fails:
> 
>   git push origin @

Yeah, I think this probably makes sense. I find the "@:master" refspec a
bit weird, but it is a natural consequence of parsing "@" on the LHS as
an arbitrary object to lookup (not a ref).

I did find the implementation a little curious:

> -	item->src = xstrndup(lhs, llen);
> +	if (llen == 1 && *lhs == '@')
> +		item->src = xstrdup("HEAD");
> +	else
> +		item->src = xstrndup(lhs, llen);

We already know we are parsing the LHS, so why must we expand a bare @,
whereas we do not in "@:master". The answer is that the "@" is not
expanded until later, and parse_refspec() gets unhappy that "@" is also
the implicit value for the RHS.

This also means that:

  git push origin @:foo

will now work when "foo" doesn't exist on the remote (if we find no
match on the remote, we guess how it should be fully qualified; one of
our heuristics involves seeing that HEAD points to a branch, and
therefore the other side should be a branch, too).

> It is obvious what is the desired behavior, and allowing the push makes
> things more consistent.

This same code is used by fetch, as well. There "git fetch origin
@:master" does not work at all now. This would make that work (to fetch
the remote HEAD into master), along with "git fetch origin @" (into
FETCH_HEAD only). Both seem sensible, and I cannot think of any other
reasonable meaning for them.

> ---
>  refspec.c             | 5 ++++-
>  t/t5511-refspec.sh    | 2 ++
>  t/t5516-fetch-push.sh | 9 +++++++++
>  3 files changed, 15 insertions(+), 1 deletion(-)

Would we want to cover the extra cases I mentioned above (@:foo, and the
two fetches), as well?

I wondered if there was a good place to mention this in the refspec
documentation, but it may just be an obvious fallout of the "@ is a
shortcut for HEAD" definition in gitrevisions(7). The only change is
that we're resolving the shortcut sooner so that more code can take
advantage of it.

-Peff

Re: [PATCH] refspec: make @ a valid refspec

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> I wondered if there was a good place to mention this in the refspec
> documentation, but it may just be an obvious fallout of the "@ is a
> shortcut for HEAD" definition in gitrevisions(7). The only change is
> that we're resolving the shortcut sooner so that more code can take
> advantage of it.

I too find that "@ is a shortcut for HEAD" looks ugly both at the UI
level and at the implementation level [*1*], but as long as we have
it, it is good to try to be consistent and allow "@" everywhere
where one would write "HEAD" in places where it is syntacitically
infeasible---at least we would be consistently ugly that way ;-).

The title of the change may want to be clarified to help readers of
"git shortlog".  It's not like it is only changing "@" and no other
variants valid refspec, but it forces readers to make sure that the
author did not forget to deal with ":@", "src:@", "@:dst", etc.
"make @ a synonym to HEAD in refspec" or something along the line,
perhaps.


[Footnote]

*1* I shouldn't complain too much, as my own invention to silently
turn "-" given from the command line into "@{-1}" is ugly at the
code level the same way, even though it may be better at the UI
level.

Re: [PATCH] refspec: make @ a valid refspec

[Felipe Contreras]

On Tue, Nov 24, 2020 at 2:01 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jeff King <peff@peff.net> writes:
>
> > I wondered if there was a good place to mention this in the refspec
> > documentation, but it may just be an obvious fallout of the "@ is a
> > shortcut for HEAD" definition in gitrevisions(7). The only change is
> > that we're resolving the shortcut sooner so that more code can take
> > advantage of it.
>
> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI
> level and at the implementation level [*1*], but as long as we have
> it, it is good to try to be consistent and allow "@" everywhere
> where one would write "HEAD" in places where it is syntacitically
> infeasible---at least we would be consistently ugly that way ;-).

Beauty is in the eye of the beholder.

I find HEAD to be an eyesore.

> The title of the change may want to be clarified to help readers of
> "git shortlog".  It's not like it is only changing "@" and no other
> variants valid refspec, but it forces readers to make sure that the
> author did not forget to deal with ":@", "src:@", "@:dst", etc.
> "make @ a synonym to HEAD in refspec" or something along the line,
> perhaps.

Right. I didn't notice it changed the semantics of those.

Given that, your suggested title makes more sense.

Cheers.

-- 
Felipe Contreras

Re: [PATCH] refspec: make @ a valid refspec

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI
>> level and at the implementation level [*1*], but as long as we have
>> it, it is good to try to be consistent and allow "@" everywhere
>> where one would write "HEAD" in places where it is syntacitically
>> infeasible---at least we would be consistently ugly that way ;-).
>
> Beauty is in the eye of the beholder.

Using "@" is rather "illogical" than "ugly", and at that point it is
not so subjective.

@-mark leads a suffix that applies some "magic" to what comes before
it (e.g. next@{1}, maint@{2.weeks.ago}, and master@{-1}).  Making @
a synonym for HEAD means '@' sometimes means a ref and most of the
time means the introducer of magic that applies to a ref.

Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th
previous commit the current branch pointed at, so a mnemonic for the
end user to remember the distinction between the two is that a bare
"@" is different from HEAD, which is a total opposite X-<.

This is all water under the bridge, though ;-)

> Given that, your suggested title makes more sense.

Sounds good.

Re: [PATCH] refspec: make @ a valid refspec

[Jacob Keller]

On Tue, Nov 24, 2020 at 2:45 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> >> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI
> >> level and at the implementation level [*1*], but as long as we have
> >> it, it is good to try to be consistent and allow "@" everywhere
> >> where one would write "HEAD" in places where it is syntacitically
> >> infeasible---at least we would be consistently ugly that way ;-).
> >
> > Beauty is in the eye of the beholder.
>
> Using "@" is rather "illogical" than "ugly", and at that point it is
> not so subjective.
>
> @-mark leads a suffix that applies some "magic" to what comes before
> it (e.g. next@{1}, maint@{2.weeks.ago}, and master@{-1}).  Making @
> a synonym for HEAD means '@' sometimes means a ref and most of the
> time means the introducer of magic that applies to a ref.
>
> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th
> previous commit the current branch pointed at, so a mnemonic for the
> end user to remember the distinction between the two is that a bare
> "@" is different from HEAD, which is a total opposite X-<.
>

However, @{0} *does* refer to what is currently checked out, which
would be head.. So in a sense @ meaning "the current branch" and
applying @{0} would always be HEAD, no?

I think it sort of works, but yea it is a bit messy.

Thanks,
Jake

Re: [PATCH] refspec: make @ a valid refspec

[Junio C Hamano]

Jacob Keller <jacob.keller@gmail.com> writes:

>> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th
>> previous commit the current branch pointed at, so a mnemonic for the
>> end user to remember the distinction between the two is that a bare
>> "@" is different from HEAD, which is a total opposite X-<.
>>
>
> However, @{0} *does* refer to what is currently checked out, which
> would be head.. So in a sense @ meaning "the current branch" and
> applying @{0} would always be HEAD, no?

Not really.  

It happens to hold true for @{0}, because by definition you couldn't
have been on a different branch than the current one when you made
the topmost commit on the current branch.  For @{1} and higher, it
is always "where was the current branch at N commits ago?" which is
different from "where was the HEAD at N commits ago?", unless you
always use a single branch and never switch away.

Re: [PATCH] refspec: make @ a valid refspec

[Jacob Keller]

On Tue, Nov 24, 2020 at 3:14 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:
>
> >> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th
> >> previous commit the current branch pointed at, so a mnemonic for the
> >> end user to remember the distinction between the two is that a bare
> >> "@" is different from HEAD, which is a total opposite X-<.
> >>
> >
> > However, @{0} *does* refer to what is currently checked out, which
> > would be head.. So in a sense @ meaning "the current branch" and
> > applying @{0} would always be HEAD, no?
>
> Not really.
>
> It happens to hold true for @{0}, because by definition you couldn't
> have been on a different branch than the current one when you made
> the topmost commit on the current branch.  For @{1} and higher, it
> is always "where was the current branch at N commits ago?" which is
> different from "where was the HEAD at N commits ago?", unless you
> always use a single branch and never switch away.
>

Right, once you add anything greater than zero it breaks down.. but
think about it a little differently: "@{N}" is sort of eliding the
branch name, which means we use the current branch. "branch@" (if it
were valid syntax) would be eliding the number which means "the most
recent version of branch". Thus, eliding both and using just "@" would
mean "the most recent version of the current branch", which cannot be
anything other than HEAD.

Of course I agree that "@ == HEAD" can't be used to go *backwards*
through that logic at all. But if you're moving forwards through it,
then "@" on its own can make sense as HEAD, but only as an implication
of "the most recent version of the current branch can't be anything
else"

Thanks,
Jake

Re: [PATCH] refspec: make @ a valid refspec

[Felipe Contreras]

On Tue, Nov 24, 2020 at 4:45 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
> >> I too find that "@ is a shortcut for HEAD" looks ugly both at the UI
> >> level and at the implementation level [*1*], but as long as we have
> >> it, it is good to try to be consistent and allow "@" everywhere
> >> where one would write "HEAD" in places where it is syntacitically
> >> infeasible---at least we would be consistently ugly that way ;-).
> >
> > Beauty is in the eye of the beholder.
>
> Using "@" is rather "illogical" than "ugly", and at that point it is
> not so subjective.

I disagree.

> @-mark leads a suffix that applies some "magic" to what comes before
> it (e.g. next@{1}, maint@{2.weeks.ago}, and master@{-1}).  Making @
> a synonym for HEAD means '@' sometimes means a ref and most of the
> time means the introducer of magic that applies to a ref.

All this was discussed back then [1].

In my mind "@" means the same as "@{0}", and "@{0}~0^0". So I don't
see any inconsistency.

Your mind may be different.

> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th
> previous commit the current branch pointed at, so a mnemonic for the
> end user to remember the distinction between the two is that a bare
> "@" is different from HEAD, which is a total opposite X-<.

Once again; each user is different. Personally I never use these
shortcuts precisely because I'm not entirely sure what I want to tell
git. I just use "git reflog" and manually pick the commit id I want.

I suspect most users don't use this notation either, so even if
there's an inconsistency (which I'm not entirely sure the @ shortcut
has anything to do with it), it would affect few users.

> This is all water under the bridge, though ;-)

Indeed.

And I take it we can agree that it's better to have instructions like:

  git push -u origin @

Rather than:

  git push -u origin master

Regardless of what some users think of "@", it's less contentious than "master".

Cheers.

[1] https://lore.kernel.org/git/1367264106-2351-1-git-send-email-felipe.contreras@gmail.com/

-- 
Felipe Contreras

Re: [PATCH] refspec: make @ a valid refspec

[Felipe Contreras]

On Tue, Nov 24, 2020 at 5:14 PM Junio C Hamano <gitster@pobox.com> wrote:
>
> Jacob Keller <jacob.keller@gmail.com> writes:

> > However, @{0} *does* refer to what is currently checked out, which
> > would be head.. So in a sense @ meaning "the current branch" and
> > applying @{0} would always be HEAD, no?
>
> Not really.
>
> It happens to hold true for @{0}, because by definition you couldn't
> have been on a different branch than the current one when you made
> the topmost commit on the current branch.  For @{1} and higher, it
> is always "where was the current branch at N commits ago?" which is
> different from "where was the HEAD at N commits ago?", unless you
> always use a single branch and never switch away.

But @{0} is always the same as HEAD@{0}, which is always the same as @.

-- 
Felipe Contreras

Re: [PATCH] refspec: make @ a valid refspec

[Felipe Contreras]

On Tue, Nov 24, 2020 at 5:47 PM Jacob Keller <jacob.keller@gmail.com> wrote:
>
> On Tue, Nov 24, 2020 at 3:14 PM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Jacob Keller <jacob.keller@gmail.com> writes:
> >
> > >> Worse yet, @{4} does not refer to HEAD@{4} but refers to the 4-th
> > >> previous commit the current branch pointed at, so a mnemonic for the
> > >> end user to remember the distinction between the two is that a bare
> > >> "@" is different from HEAD, which is a total opposite X-<.
> > >>
> > >
> > > However, @{0} *does* refer to what is currently checked out, which
> > > would be head.. So in a sense @ meaning "the current branch" and
> > > applying @{0} would always be HEAD, no?
> >
> > Not really.
> >
> > It happens to hold true for @{0}, because by definition you couldn't
> > have been on a different branch than the current one when you made
> > the topmost commit on the current branch.  For @{1} and higher, it
> > is always "where was the current branch at N commits ago?" which is
> > different from "where was the HEAD at N commits ago?", unless you
> > always use a single branch and never switch away.
>
> Right, once you add anything greater than zero it breaks down.. but
> think about it a little differently: "@{N}" is sort of eliding the
> branch name, which means we use the current branch. "branch@" (if it
> were valid syntax) would be eliding the number which means "the most
> recent version of branch". Thus, eliding both and using just "@" would
> mean "the most recent version of the current branch", which cannot be
> anything other than HEAD.

Yes, but to me HEAD is supposed to mean "the current branch", so
"branch@{1}" and "HEAD@{1}" should be the same, and they are not. So
to me they are the other way around.

1. branch@{1}, HEAD@{1}, @@{1}: current branch 1 commit ago
2. @{1}: 1 commit ago in general

Since it's the opposite of what I would expect, I simply don't use
these notations.

-- 
Felipe Contreras

Re: [PATCH] refspec: make @ a valid refspec

[Junio C Hamano]

Jacob Keller <jacob.keller@gmail.com> writes:

> Of course I agree that "@ == HEAD" can't be used to go *backwards*
> through that logic at all. But if you're moving forwards through it,
> then "@" on its own can make sense as HEAD, but only as an implication
> of "the most recent version of the current branch can't be anything
> else"

I'd rather put the lame excuses behind and accept that we ended up
with an ugly and illogical synonym that ;-)

And making it consistently available in places where HEAD is
accepted is a good thing.  Depending on the keyboard, it may be
easy to type, too.

Re: [PATCH] refspec: make @ a valid refspec

[Felipe Contreras]

Hello,

On Tue, Nov 24, 2020 at 6:09 PM Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Tue, Nov 24, 2020 at 4:45 PM Junio C Hamano <gitster@pobox.com> wrote:
> > Felipe Contreras <felipe.contreras@gmail.com> writes:

> > > Beauty is in the eye of the beholder.
> >
> > Using "@" is rather "illogical" than "ugly", and at that point it is
> > not so subjective.
>
> I disagree.

This is not an attempt to reignite the discussion, but just something
that might be of interest.

I created a poll [1] in reddit's r/git: Do you know @ is a shortcut for HEAD?

403 people gave a useful response. Of those; 74% didn't know such a
shortcut existed, 80% liked this shortcut.

But more importantly: 59% did like the shortcut, but didn't know it
existed. That is: of the people that liked the shortcut; 74% didn't
know it existed.

This tells us something: putting personal preferences aside; the Git
project doesn't seem to be doing a great job of advertising its
features. Features a good chunk--if not the overwhelming majority--of
users might like.

Other things of interest in the comments: one user didn't even know
what HEAD was, another used "head" (lowercase) (macOS user), and
lastly; the second most voted comment did take a shot at Git's known
unintuitiveness.

That's it. Just putting it here.

Cheers.

[1] https://www.reddit.com/r/git/comments/k15cqm/do_you_know_is_a_shortcut_for_head/

-- 
Felipe Contreras