threads / patch / 33681

patchrefs.c: interpret @ as HEAD

Subject: [PATCH] refs.c: interpret @ as HEAD

## tl;dr

21 messages between Apr 30, 2013 and May 1, 2013. Diffs are folded; open one to read it.

replies: 20people: 6as markdown or json

Ramkumar Ramachandra· Apr 30, 2013, 12:24 UTC · lore

The rev spec forms @{}, .., ... fill in HEAD as the missing argument automatically. Unfortunately, HEAD~<n> is a very common idiom and there is no way to make HEAD implicit here (due the shell expansion of ~<n>).

However, there is an alternative solution to the issue: overload the character @ to mean HEAD. Do this at the lowest possible layer of abstraction: in dwim_ref(), substitute @ with HEAD just before calling resolve_ref_unsafe(). The program will only reach this point after the other specs like ~, ^ and @{} have been resolved; therefore, it is safe to do it here.

This patch has the exact same effect as:
    $ git symbolic-ref @ HEAD

It means that you can now do @~1, @^2, and even topic..@. However, since the @-parsing happens before we ever reach the symref resolution, @@{u} is invalid. But this is okay, since @{u} already has an implicit HEAD in it.

Inspired-by: Felipe Contreras <felipe.contreras@gmail.com>
Inspired-by: Michael Haggerty <mhagger@alum.mit.edu>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 I haven't included documentation/ tests because I want feedback on
 this two-liner first.
 refs.c | 2 ++
 1 file changed, 2 insertions(+)
Show changes to refs.c +2 −0
diff --git a/refs.c b/refs.c
index de2d8eb..cb67b73 100644
--- a/refs.c
+++ b/refs.c
@@ -1604,6 +1604,8 @@ int dwim_ref(const char *str, int len, unsigned char *sha1, char **ref)
 
 		this_result = refs_found ? sha1_from_ref : sha1;
 		mksnpath(fullref, sizeof(fullref), *p, len, str);
+		if (!strcmp(fullref, "@"))
+			mksnpath(fullref, sizeof(fullref), *p, 4, "HEAD");
 		r = resolve_ref_unsafe(fullref, this_result, 1, &flag);
 		if (r) {
 			if (!refs_found++)

-- 
1.8.2.1.628.gcd33b41.dirty
Duy Nguyen· Apr 30, 2013, 12:31 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On Tue, Apr 30, 2013 at 7:24 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 27 quoted lines
> The rev spec forms @{}, .., ... fill in HEAD as the missing argument
> automatically.  Unfortunately, HEAD~<n> is a very common idiom and
> there is no way to make HEAD implicit here (due the shell expansion of
> ~<n>).
>
> However, there is an alternative solution to the issue: overload the
> character @ to mean HEAD.  Do this at the lowest possible layer of
> abstraction: in dwim_ref(), substitute @ with HEAD just before calling
> resolve_ref_unsafe().  The program will only reach this point after
> the other specs like ~, ^ and @{} have been resolved; therefore, it is
> safe to do it here.
>
> This patch has the exact same effect as:
>
>     $ git symbolic-ref @ HEAD
>
> It means that you can now do @~1, @^2, and even topic..@.  However,
> since the @-parsing happens before we ever reach the symref
> resolution, @@{u} is invalid.  But this is okay, since @{u} already
> has an implicit HEAD in it.
>
> Inspired-by: Felipe Contreras <felipe.contreras@gmail.com>
> Inspired-by: Michael Haggerty <mhagger@alum.mit.edu>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  I haven't included documentation/ tests because I want feedback on
>  this two-liner first.

You need more than that :) As Michael pointed out '@' is a perfectly valid ref name. If you do this, you need to reject '@' as invalid ref name. I guess another two lines in check_refname_format. But let's wait for more feedback first. -- Duy

Thomas Rast· Apr 30, 2013, 13:01 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 15 quoted lines
> The rev spec forms @{}, .., ... fill in HEAD as the missing argument
> automatically.  Unfortunately, HEAD~<n> is a very common idiom and
> there is no way to make HEAD implicit here (due the shell expansion of
> ~<n>).
>
> However, there is an alternative solution to the issue: overload the
> character @ to mean HEAD.  Do this at the lowest possible layer of
> abstraction: in dwim_ref(), substitute @ with HEAD just before calling
> resolve_ref_unsafe().  The program will only reach this point after
> the other specs like ~, ^ and @{} have been resolved; therefore, it is
> safe to do it here.
>
> This patch has the exact same effect as:
>
>     $ git symbolic-ref @ HEAD

But then why don't you just 'git symbolic-ref H HEAD' for a sort of "local alias"?

What annoys me more is that there's no way to say
  git symbolic-ref U @{u}

so that I can avoid that -- it's really clumsy to type on a Swiss German keyboard. We'd need some sort of ref-alias feature for that to work.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Ramkumar Ramachandra· Apr 30, 2013, 13:32 UTC · re: Thomas Rast · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Thomas Rast wrote:
> But then why don't you just 'git symbolic-ref H HEAD' for a sort of
> "local alias"?

Yes, I already have @ pointing to HEAD. And I think it's much nicer than H (also since H@{u} doesn't resolve [1]). The purpose of this patch is to standardize @ for everyone.

> What annoys me more is that there's no way to say
>
>   git symbolic-ref U @{u}

That's because the part after "ref: " in a symref is expected to point to a concrete ref, and doesn't go through the get_sha1_basic() machinery. Making symrefs peel recursively might or might not be a good idea; I haven't thought about it enough. However, it's not my itch: write a patch and start a discussion?

[1]: http://thread.gmane.org/gmane.comp.version-control.git/222852/focus=222916
Duy Nguyen· Apr 30, 2013, 15:04 UTC · re: Thomas Rast · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On Tue, Apr 30, 2013 at 03:01:22PM +0200, Thomas Rast wrote:
Show 6 quoted lines
> > This patch has the exact same effect as:
> >
> >     $ git symbolic-ref @ HEAD
> 
> But then why don't you just 'git symbolic-ref H HEAD' for a sort of
> "local alias"?

For me, it's because I don't want to do that on every repo. One day if I change my mind and make P the new alias, i'll need to update every repo again.

Show 7 quoted lines
> 
> What annoys me more is that there's no way to say
> 
>   git symbolic-ref U @{u}
> 
> so that I can avoid that -- it's really clumsy to type on a Swiss German
> keyboard.  We'd need some sort of ref-alias feature for that to work.

It's not hard to do. The below patch makes "." equivalent to HEAD and ".U" -> "@{u}". Refs are not supposed to have '.' at the beginning, so it's easy to figure somebody is using alias from what they send you. Supporting refalias.* config should be easy.

-- 8< --
Show changes to sha1_name.c +23 −1
diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..7842147 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1239,6 +1239,27 @@ static char *resolve_relative_path(const char *rel)
 			   rel);
 }
 
+static int get_sha1_with_alias(const char *name, int namelen,
+			       unsigned char *sha1, int flags)
+{
+	int ret;
+	struct strbuf sb = STRBUF_INIT;
+	int alias_len = strcspn(name, "@^~:");
+	if (alias_len > namelen)
+		alias_len = namelen;
+
+	if (alias_len == 0)	/* '.' is equal to HEAD */
+		strbuf_addstr(&sb, "HEAD");
+	else if (alias_len == 1 && !strncmp(name, "U", 1))
+		strbuf_addstr(&sb, "@{u}");
+	else
+		return error("unknown alias %.*s", alias_len, name);
+	strbuf_addstr(&sb, name + alias_len);
+	ret = get_sha1_1(sb.buf, sb.len, sha1, flags);
+	strbuf_release(&sb);
+	return ret;
+}
+
 static int get_sha1_with_context_1(const char *name,
 				   unsigned flags,
 				   const char *prefix,
@@ -1252,6 +1273,8 @@ static int get_sha1_with_context_1(const char *name,
 
 	memset(oc, 0, sizeof(*oc));
 	oc->mode = S_IFINVALID;
+	if (name[0] == '.')
+		return get_sha1_with_alias(name + 1, namelen - 1, sha1, flags);
 	ret = get_sha1_1(name, namelen, sha1, flags);
 	if (!ret)
 		return ret;
-- 8< --
Michael Haggerty· Apr 30, 2013, 16:08 UTC · re: Duy Nguyen · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On 04/30/2013 05:04 PM, Duy Nguyen wrote:
Show 12 quoted lines
> On Tue, Apr 30, 2013 at 03:01:22PM +0200, Thomas Rast wrote:
>> What annoys me more is that there's no way to say
>>
>>   git symbolic-ref U @{u}
>>
>> so that I can avoid that -- it's really clumsy to type on a Swiss German
>> keyboard.  We'd need some sort of ref-alias feature for that to work.
> 
> It's not hard to do. The below patch makes "." equivalent to HEAD and
> ".U" -> "@{u}". Refs are not supposed to have '.' at the beginning, so
> it's easy to figure somebody is using alias from what they send
> you. Supporting refalias.* config should be easy.
So what does "git log master...U" mean?
Michael
-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Junio C Hamano· Apr 30, 2013, 16:09 UTC · re: Duy Nguyen · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Duy Nguyen <pclouds@gmail.com> writes:
> It's not hard to do. The below patch makes "." equivalent to HEAD and
> ".U" -> "@{u}". Refs are not supposed to have '.' at the beginning, so
> it's easy ...

How is the equivalent of master..@{u} expressed? master...U? How is it disambiguated from a symmetric difference between master and U?

There are reasons why some characters are forbidden from appearing at certain places in refname component. Anybody who designs a new syntax needs to think _why_.

The restriction that a @ in refname cannot be followed by { is for the same kind of disambiguation. I do not mind a way to spell HEAD with shorter than 4 keystrokes, and as I said I suspect "@" may be the least bad one among what people may come up with in this thread, but I do not think we can explain it as "@ is a synonym to HEAD" [*1*]. We need to see if we can make the explanation we will give to end users is understandable.

[Footnote]

*1* I do not think "@ is a new synonym for HEAD" would not be a good explanation. Some questions you should ask yourselves to see why:

"git update-ref HEAD $commit" is accepted. If @ is a synonym for HEAD, "git update-ref @ $commit" should work exactly the same way, but is it desirable? Would we have $GIT_DIR/@ as the result? How about "git symbolic-ref"? Would @@{4} and HEAD@{4} be the same?

Duy Nguyen· Apr 30, 2013, 16:26 UTC · re: Junio C Hamano · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On Tue, Apr 30, 2013 at 11:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 12 quoted lines
> Duy Nguyen <pclouds@gmail.com> writes:
>
>> It's not hard to do. The below patch makes "." equivalent to HEAD and
>> ".U" -> "@{u}". Refs are not supposed to have '.' at the beginning, so
>> it's easy ...
>
> How is the equivalent of master..@{u} expressed?  master...U?  How is
> it disambiguated from a symmetric difference between master and U?
>
> There are reasons why some characters are forbidden from appearing
> at certain places in refname component. Anybody who designs a new
> syntax needs to think _why_.

I figured it had something to do with hidden filenames, not "..". There's only one char left for ref alias, the leading forward slash. But than opens another can of worms.. We could put still ref aliases into the same ref namespace, with lower precedence that actual refs, so no new syntax required. -- Duy

Ramkumar Ramachandra· Apr 30, 2013, 17:15 UTC · re: Duy Nguyen · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Duy Nguyen wrote:
> We could put still ref aliases
> into the same ref namespace, with lower precedence that actual refs,
> so no new syntax required.

Actually, ref-alises are the right way to solve the problem. Recursive symref peeling is a bad idea: I can't take my aliases with me, and they complicate unnecessarily.

Any thoughts on how to implement it? Should it go as deep as resolve_ref_unsafe()?

Duy Nguyen· May 1, 2013, 02:18 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On Wed, May 1, 2013 at 12:15 AM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 11 quoted lines
> Duy Nguyen wrote:
>> We could put still ref aliases
>> into the same ref namespace, with lower precedence that actual refs,
>> so no new syntax required.
>
> Actually, ref-alises are the right way to solve the problem.
> Recursive symref peeling is a bad idea: I can't take my aliases with
> me, and they complicate unnecessarily.
>
> Any thoughts on how to implement it?  Should it go as deep as
> resolve_ref_unsafe()?

Depends on how you define ref alias. resolve_ref_unsafe allows you to substitute one ref with another. Thomas was talking about substituting part of extended sha-1 syntax (U -> @{u}) so it can't be done down there. I still think get_sha1_with_context_1() is the right place. Still not so sure how to handle when we have both alias "U" and refs/heads/U. -- Duy

Thomas Rast· May 1, 2013, 08:35 UTC · re: Duy Nguyen · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Duy Nguyen <pclouds@gmail.com> writes:
Show 20 quoted lines
> On Wed, May 1, 2013 at 12:15 AM, Ramkumar Ramachandra
> <artagnon@gmail.com> wrote:
>> Duy Nguyen wrote:
>>> We could put still ref aliases
>>> into the same ref namespace, with lower precedence that actual refs,
>>> so no new syntax required.
>>
>> Actually, ref-alises are the right way to solve the problem.
>> Recursive symref peeling is a bad idea: I can't take my aliases with
>> me, and they complicate unnecessarily.
>>
>> Any thoughts on how to implement it?  Should it go as deep as
>> resolve_ref_unsafe()?
>
> Depends on how you define ref alias. resolve_ref_unsafe allows you to
> substitute one ref with another. Thomas was talking about substituting
> part of extended sha-1 syntax (U -> @{u}) so it can't be done down
> there. I still think get_sha1_with_context_1() is the right place.
> Still not so sure how to handle when we have both alias "U" and
> refs/heads/U.

Well, I'm not sure about the semantics that I want. But so far I am *pretty* sure that I don't want it to be parameterized / part of another ref.

So I'm fine with looking at *just* the alias, and resolving that to a SHA1, and going from there. So assuming U = @{u} and H = HEAD, you'd be allowed to say U^ or U..H but not HU or H@U or whatever contortionate syntax that would need.

As for the collisions, not sure which one is better. Probably having the same semantics as command aliases would be less confusing, i.e., existing refs take precedence.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch
Ramkumar Ramachandra· Apr 30, 2013, 17:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Junio C Hamano wrote:
> We need to see if we can make the explanation we will give
> to end users is understandable.
I'll make an attempt.
@ can be used in two contexts:
1. When used without the {}, it means HEAD.
2. When used with a {}, it means what those three forms individually
mean.  @{} already implies HEAD: don't put an extra @.
I think it's the cleanest.
Ramkumar Ramachandra· Apr 30, 2013, 17:23 UTC · re: Junio C Hamano · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Junio C Hamano wrote:
> "git update-ref HEAD $commit" is accepted.  If @ is a synonym for
> HEAD, "git update-ref @ $commit" should work exactly the same way,
> but is it desirable?
>  Would we have $GIT_DIR/@ as the result?  How
> about "git symbolic-ref"?
Yes, it is a valid refname that can be overridden [*1*].
> Would @@{4} and HEAD@{4} be the same?
No.  Why should they?
[Footnotes]
*1*: In the current implementation, git update-ref fails.
Felipe Contreras· Apr 30, 2013, 17:28 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On Tue, Apr 30, 2013 at 12:23 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

> Junio C Hamano wrote:
>> Would @@{4} and HEAD@{4} be the same?
>
> No.  Why should they?
Why would HEAD^0^0~4 work? Because the syntax is recursive.
-- 
Felipe Contreras
Junio C Hamano· Apr 30, 2013, 18:08 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Ramkumar Ramachandra <artagnon@gmail.com> writes:
Show 5 quoted lines
> Felipe Contreras wrote:
>> Why would HEAD^0^0~4 work? Because the syntax is recursive.
>
> That's because you can compose with ^ and ^, while you can't with @
> and @.  Does @{0}@{0} resolve?

@{-1}@{0} does. That means @{0} is a revision and not a ref, but @{-1} is.

'"@" given alone' has to be a ref if we want @@{5.minutes.ago} to resolve.

Ramkumar Ramachandra· Apr 30, 2013, 18:22 UTC · re: Junio C Hamano · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Junio C Hamano wrote:
> @{-1}@{0} does.  That means @{0} is a revision and not a ref, but @{-1}
> is.
Right.  I missed that.
> '"@" given alone' has to be a ref if we want @@{5.minutes.ago} to
> resolve.

Yeah, I just realized that it's a bug in the @{u} implementation. @@{5.minutes.ago} and @@{1} work perfectly fine on git.git master. Therefore, @@{5.minutes.ago} should work with my implementation too. Yes, it's just the default value of .git/@.

Ramkumar Ramachandra· Apr 30, 2013, 18:40 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Ramkumar Ramachandra wrote:
> Therefore, @@{5.minutes.ago} should work with my implementation too.
Caveat: we won't keep reflogs for @ unless the manual override .git/@
is present (what's the point of keeping a duplicate of .git/logs/HEAD
in .git/logs/@?).  So, without the .git/@, @@{1} will complain that
the reflog only has 1 entry (in other words, it is "correct" but not
useful).
Ramkumar Ramachandra· Apr 30, 2013, 18:50 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

Ramkumar Ramachandra wrote:
> So, without the .git/@, @@{1} will complain that
> the reflog only has 1 entry (in other words, it is "correct" but not
> useful).

Even better idea. When not overridden, @@{<n>} will show the reflog for HEAD. To the user, it looks like @ is behaving like a true ref by keeping a reflog, but we're just cheating.

Felipe Contreras· Apr 30, 2013, 22:00 UTC · re: Ramkumar Ramachandra · lore

Re: [PATCH] refs.c: interpret @ as HEAD

On Tue, Apr 30, 2013 at 1:22 PM, Ramkumar Ramachandra <artagnon@gmail.com> wrote:

Show 10 quoted lines
> Junio C Hamano wrote:
>> @{-1}@{0} does.  That means @{0} is a revision and not a ref, but @{-1}
>> is.
>
> Right.  I missed that.
>
>> '"@" given alone' has to be a ref if we want @@{5.minutes.ago} to
>> resolve.
>
> Yeah, I just realized that it's a bug in the @{u} implementation.

I don't think so. You probably need to modify branch_get(), because it has a special case for "HEAD", and who knows if it's hard-coded in other places. It's not just the @{u} implementation.

If we do the magic at the rev-parsing phase, all these details become irrelevant.

FTR. @@{upstream} and @@{now} works just fine in v2 of my patch series.
-- 
Felipe Contreras

← back to recent threads