[PATCH] teach the new "@{-1} syntax to "git branch"

[PATCH] teach the new "@{-1} syntax to "git branch"

[Junio C Hamano]

This teaches the new "@{-1} syntax to refer to the previous branch to "git
branch".  After looking at somebody's faulty patch series on a topic
branch too long, if you decide it is not worth merging, you can just say:

    $ git checkout master
    $ git branch -D @{-1}

to get rid of it without having to type the name of the topic you now hate
so much for wasting a lot of your time.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 branch.c         |   19 ++++++++++++++-----
 builtin-branch.c |   31 +++++++++++++++++++------------
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/branch.c b/branch.c
index b1ac837..1f00e44 100644
--- a/branch.c
+++ b/branch.c
@@ -103,14 +103,22 @@ void create_branch(const char *head,
 	struct ref_lock *lock;
 	struct commit *commit;
 	unsigned char sha1[20];
-	char *real_ref, ref[PATH_MAX], msg[PATH_MAX + 20];
+	char *real_ref, msg[PATH_MAX + 20];
+	struct strbuf ref = STRBUF_INIT;
 	int forcing = 0;
+	int len;
 
-	snprintf(ref, sizeof ref, "refs/heads/%s", name);
-	if (check_ref_format(ref))
+	len = strlen(name);
+	if (interpret_nth_last_branch(name, &ref) != len) {
+		strbuf_reset(&ref);
+		strbuf_add(&ref, name, len);
+	}
+	strbuf_splice(&ref, 0, 0, "refs/heads/", 11);
+
+	if (check_ref_format(ref.buf))
 		die("'%s' is not a valid branch name.", name);
 
-	if (resolve_ref(ref, sha1, 1, NULL)) {
+	if (resolve_ref(ref.buf, sha1, 1, NULL)) {
 		if (!force)
 			die("A branch named '%s' already exists.", name);
 		else if (!is_bare_repository() && !strcmp(head, name))
@@ -142,7 +150,7 @@ void create_branch(const char *head,
 		die("Not a valid branch point: '%s'.", start_name);
 	hashcpy(sha1, commit->object.sha1);
 
-	lock = lock_any_ref_for_update(ref, NULL, 0);
+	lock = lock_any_ref_for_update(ref.buf, NULL, 0);
 	if (!lock)
 		die("Failed to lock ref for update: %s.", strerror(errno));
 
@@ -162,6 +170,7 @@ void create_branch(const char *head,
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
 
+	strbuf_release(&ref);
 	free(real_ref);
 }
 
diff --git a/builtin-branch.c b/builtin-branch.c
index 56a1971..504a981 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -99,6 +99,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 	const char *fmt, *remote;
 	int i;
 	int ret = 0;
+	struct strbuf bname = STRBUF_INIT;
 
 	switch (kinds) {
 	case REF_REMOTE_BRANCH:
@@ -119,20 +120,25 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 		if (!head_rev)
 			die("Couldn't look up commit object for HEAD");
 	}
-	for (i = 0; i < argc; i++) {
-		if (kinds == REF_LOCAL_BRANCH && !strcmp(head, argv[i])) {
+	for (i = 0; i < argc; i++, strbuf_release(&bname)) {
+		int len = strlen(argv[i]);
+
+		if (interpret_nth_last_branch(argv[i], &bname) != len)
+			strbuf_add(&bname, argv[i], len);
+
+		if (kinds == REF_LOCAL_BRANCH && !strcmp(head, bname.buf)) {
 			error("Cannot delete the branch '%s' "
-				"which you are currently on.", argv[i]);
+			      "which you are currently on.", bname.buf);
 			ret = 1;
 			continue;
 		}
 
 		free(name);
 
-		name = xstrdup(mkpath(fmt, argv[i]));
+		name = xstrdup(mkpath(fmt, bname.buf));
 		if (!resolve_ref(name, sha1, 1, NULL)) {
 			error("%sbranch '%s' not found.",
-					remote, argv[i]);
+					remote, bname.buf);
 			ret = 1;
 			continue;
 		}
@@ -152,22 +158,23 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 		if (!force &&
 		    !in_merge_bases(rev, &head_rev, 1)) {
 			error("The branch '%s' is not an ancestor of "
-				"your current HEAD.\n"
-				"If you are sure you want to delete it, "
-				"run 'git branch -D %s'.", argv[i], argv[i]);
+			      "your current HEAD.\n"
+			      "If you are sure you want to delete it, "
+			      "run 'git branch -D %s'.", bname.buf, bname.buf);
 			ret = 1;
 			continue;
 		}
 
 		if (delete_ref(name, sha1, 0)) {
 			error("Error deleting %sbranch '%s'", remote,
-			       argv[i]);
+			      bname.buf);
 			ret = 1;
 		} else {
 			struct strbuf buf = STRBUF_INIT;
-			printf("Deleted %sbranch %s (%s).\n", remote, argv[i],
-				find_unique_abbrev(sha1, DEFAULT_ABBREV));
-			strbuf_addf(&buf, "branch.%s", argv[i]);
+			printf("Deleted %sbranch %s (%s).\n", remote,
+			       bname.buf,
+			       find_unique_abbrev(sha1, DEFAULT_ABBREV));
+			strbuf_addf(&buf, "branch.%s", bname.buf);
 			if (git_config_rename_section(buf.buf, NULL) < 0)
 				warning("Update of config-file failed");
 			strbuf_release(&buf);
-- 
1.6.2.rc0.80.ge11dd

Re: [PATCH] teach the new "@{-1} syntax to "git branch"

[Jay Soffian]

On Sat, Feb 14, 2009 at 2:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
> This teaches the new "@{-1} syntax to refer to the previous branch to "git
> branch".  After looking at somebody's faulty patch series on a topic
> branch too long, if you decide it is not worth merging, you can just say:
>
>    $ git checkout master
>    $ git branch -D @{-1}
>
> to get rid of it without having to type the name of the topic you now hate
> so much for wasting a lot of your time.

I hope I'm not the person who motivated this new syntax. :-)

j.

Re: [PATCH] teach the new "@{-1} syntax to "git branch"

[Junio C Hamano]

Jay Soffian <jaysoffian@gmail.com> writes:

> On Sat, Feb 14, 2009 at 2:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> This teaches the new "@{-1} syntax to refer to the previous branch to "git
>> branch".  After looking at somebody's faulty patch series on a topic
>> branch too long, if you decide it is not worth merging, you can just say:
>>
>>    $ git checkout master
>>    $ git branch -D @{-1}
>>
>> to get rid of it without having to type the name of the topic you now hate
>> so much for wasting a lot of your time.
>
> I hope I'm not the person who motivated this new syntax. :-)

Don't worry.

The @{-1} syntax was added long before you started getting hyperactive
this round, but it will be in 1.6.2 and has been advertised as "usable
anywhere you can use a branch name", but in reality it is not.

I have been fixing up various places to match the reality with the claim.
I am making "git merge @{-1}" work now.

[PATCH v2 1/2] Teach the "@{-1} syntax to "git branch"

[Junio C Hamano]

This teaches the new "@{-1} syntax to refer to the previous branch to "git
branch".  After looking at somebody's faulty patch series on a topic
branch too long, if you decide it is not worth merging, you can just say:

    $ git checkout master
    $ git branch -D @{-1}

to get rid of it without having to type the name of the topic you now hate
so much for wasting a lot of your time.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * Now comes with test.
 branch.c            |   19 ++++++++++++++-----
 builtin-branch.c    |   31 +++++++++++++++++++------------
 t/t0100-previous.sh |   26 ++++++++++++++++++++++++++
 3 files changed, 59 insertions(+), 17 deletions(-)
 create mode 100755 t/t0100-previous.sh

diff --git a/branch.c b/branch.c
index b1ac837..1f00e44 100644
--- a/branch.c
+++ b/branch.c
@@ -103,14 +103,22 @@ void create_branch(const char *head,
 	struct ref_lock *lock;
 	struct commit *commit;
 	unsigned char sha1[20];
-	char *real_ref, ref[PATH_MAX], msg[PATH_MAX + 20];
+	char *real_ref, msg[PATH_MAX + 20];
+	struct strbuf ref = STRBUF_INIT;
 	int forcing = 0;
+	int len;
 
-	snprintf(ref, sizeof ref, "refs/heads/%s", name);
-	if (check_ref_format(ref))
+	len = strlen(name);
+	if (interpret_nth_last_branch(name, &ref) != len) {
+		strbuf_reset(&ref);
+		strbuf_add(&ref, name, len);
+	}
+	strbuf_splice(&ref, 0, 0, "refs/heads/", 11);
+
+	if (check_ref_format(ref.buf))
 		die("'%s' is not a valid branch name.", name);
 
-	if (resolve_ref(ref, sha1, 1, NULL)) {
+	if (resolve_ref(ref.buf, sha1, 1, NULL)) {
 		if (!force)
 			die("A branch named '%s' already exists.", name);
 		else if (!is_bare_repository() && !strcmp(head, name))
@@ -142,7 +150,7 @@ void create_branch(const char *head,
 		die("Not a valid branch point: '%s'.", start_name);
 	hashcpy(sha1, commit->object.sha1);
 
-	lock = lock_any_ref_for_update(ref, NULL, 0);
+	lock = lock_any_ref_for_update(ref.buf, NULL, 0);
 	if (!lock)
 		die("Failed to lock ref for update: %s.", strerror(errno));
 
@@ -162,6 +170,7 @@ void create_branch(const char *head,
 	if (write_ref_sha1(lock, sha1, msg) < 0)
 		die("Failed to write ref: %s.", strerror(errno));
 
+	strbuf_release(&ref);
 	free(real_ref);
 }
 
diff --git a/builtin-branch.c b/builtin-branch.c
index 56a1971..504a981 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -99,6 +99,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 	const char *fmt, *remote;
 	int i;
 	int ret = 0;
+	struct strbuf bname = STRBUF_INIT;
 
 	switch (kinds) {
 	case REF_REMOTE_BRANCH:
@@ -119,20 +120,25 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 		if (!head_rev)
 			die("Couldn't look up commit object for HEAD");
 	}
-	for (i = 0; i < argc; i++) {
-		if (kinds == REF_LOCAL_BRANCH && !strcmp(head, argv[i])) {
+	for (i = 0; i < argc; i++, strbuf_release(&bname)) {
+		int len = strlen(argv[i]);
+
+		if (interpret_nth_last_branch(argv[i], &bname) != len)
+			strbuf_add(&bname, argv[i], len);
+
+		if (kinds == REF_LOCAL_BRANCH && !strcmp(head, bname.buf)) {
 			error("Cannot delete the branch '%s' "
-				"which you are currently on.", argv[i]);
+			      "which you are currently on.", bname.buf);
 			ret = 1;
 			continue;
 		}
 
 		free(name);
 
-		name = xstrdup(mkpath(fmt, argv[i]));
+		name = xstrdup(mkpath(fmt, bname.buf));
 		if (!resolve_ref(name, sha1, 1, NULL)) {
 			error("%sbranch '%s' not found.",
-					remote, argv[i]);
+					remote, bname.buf);
 			ret = 1;
 			continue;
 		}
@@ -152,22 +158,23 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 		if (!force &&
 		    !in_merge_bases(rev, &head_rev, 1)) {
 			error("The branch '%s' is not an ancestor of "
-				"your current HEAD.\n"
-				"If you are sure you want to delete it, "
-				"run 'git branch -D %s'.", argv[i], argv[i]);
+			      "your current HEAD.\n"
+			      "If you are sure you want to delete it, "
+			      "run 'git branch -D %s'.", bname.buf, bname.buf);
 			ret = 1;
 			continue;
 		}
 
 		if (delete_ref(name, sha1, 0)) {
 			error("Error deleting %sbranch '%s'", remote,
-			       argv[i]);
+			      bname.buf);
 			ret = 1;
 		} else {
 			struct strbuf buf = STRBUF_INIT;
-			printf("Deleted %sbranch %s (%s).\n", remote, argv[i],
-				find_unique_abbrev(sha1, DEFAULT_ABBREV));
-			strbuf_addf(&buf, "branch.%s", argv[i]);
+			printf("Deleted %sbranch %s (%s).\n", remote,
+			       bname.buf,
+			       find_unique_abbrev(sha1, DEFAULT_ABBREV));
+			strbuf_addf(&buf, "branch.%s", bname.buf);
 			if (git_config_rename_section(buf.buf, NULL) < 0)
 				warning("Update of config-file failed");
 			strbuf_release(&buf);
diff --git a/t/t0100-previous.sh b/t/t0100-previous.sh
new file mode 100755
index 0000000..db51b42
--- /dev/null
+++ b/t/t0100-previous.sh
@@ -0,0 +1,26 @@
+#!/bin/sh
+
+test_description='previous branch syntax @{-n}'
+
+. ./test-lib.sh
+
+test_expect_success 'branch -d @{-1}' '
+	test_commit A &&
+	git checkout -b junk &&
+	git checkout - &&
+	test "$(git symbolic-ref HEAD)" = refs/heads/master &&
+	git branch -d @{-1} &&
+	test_must_fail git rev-parse --verify refs/heads/junk
+'
+
+test_expect_success 'branch -d @{-12} when there is not enough switches yet' '
+	git reflog expire --expire=now &&
+	git checkout -b junk2 &&
+	git checkout - &&
+	test "$(git symbolic-ref HEAD)" = refs/heads/master &&
+	test_must_fail git branch -d @{-12} &&
+	git rev-parse --verify refs/heads/master
+'
+
+test_done
+
-- 
1.6.2.rc0.80.ge11dd

[PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

1.6.2 will have @{-1} syntax advertised as "usable anywhere you can use
a branch name".  However, "git merge @{-1}" did not work.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * I know "show-branch @{-1} HEAD" does not dereference @{-1}, but I
   suspect there may be many others.

 builtin-merge.c     |   15 ++++++++++++---
 t/t0100-previous.sh |   23 +++++++++++++++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/builtin-merge.c b/builtin-merge.c
index 885fad9..6d2160d 100644
--- a/builtin-merge.c
+++ b/builtin-merge.c
@@ -356,9 +356,14 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	struct object *remote_head;
 	unsigned char branch_head[20], buf_sha[20];
 	struct strbuf buf = STRBUF_INIT;
+	struct strbuf bname = STRBUF_INIT;
 	const char *ptr;
 	int len, early;
 
+	len = strlen(remote);
+	if (interpret_nth_last_branch(remote, &bname) == len)
+		remote = bname.buf;
+
 	memset(branch_head, 0, sizeof(branch_head));
 	remote_head = peel_to_type(remote, 0, NULL, OBJ_COMMIT);
 	if (!remote_head)
@@ -371,7 +376,7 @@ static void merge_name(const char *remote, struct strbuf *msg)
 	if (!hashcmp(remote_head->sha1, branch_head)) {
 		strbuf_addf(msg, "%s\t\tbranch '%s' of .\n",
 			sha1_to_hex(branch_head), remote);
-		return;
+		goto cleanup;
 	}
 
 	/* See if remote matches <name>^^^.. or <name>~<number> */
@@ -411,7 +416,8 @@ static void merge_name(const char *remote, struct strbuf *msg)
 				    sha1_to_hex(remote_head->sha1),
 				    truname.buf + 11,
 				    (early ? " (early part)" : ""));
-			return;
+			strbuf_release(&truname);
+			goto cleanup;
 		}
 	}
 
@@ -432,10 +438,13 @@ static void merge_name(const char *remote, struct strbuf *msg)
 			strbuf_remove(&line, ptr-line.buf+1, 13);
 		strbuf_addbuf(msg, &line);
 		strbuf_release(&line);
-		return;
+		goto cleanup;
 	}
 	strbuf_addf(msg, "%s\t\tcommit '%s'\n",
 		sha1_to_hex(remote_head->sha1), remote);
+cleanup:
+	strbuf_release(&buf);
+	strbuf_release(&bname);
 }
 
 static int git_merge_config(const char *k, const char *v, void *cb)
diff --git a/t/t0100-previous.sh b/t/t0100-previous.sh
index db51b42..315b9b3 100755
--- a/t/t0100-previous.sh
+++ b/t/t0100-previous.sh
@@ -22,5 +22,28 @@ test_expect_success 'branch -d @{-12} when there is not enough switches yet' '
 	git rev-parse --verify refs/heads/master
 '
 
+test_expect_success 'merge @{-1}' '
+	git checkout A &&
+	test_commit B &&
+	git checkout A &&
+	test_commit C &&
+	git branch -f master B &&
+	git branch -f other &&
+	git checkout other &&
+	git checkout master &&
+	git merge @{-1} &&
+	git cat-file commit HEAD | grep "Merge branch '\''other'\''"
+'
+
+test_expect_success 'merge @{-1} when there is not enough switches yet' '
+	git reflog expire --expire=now &&
+	git checkout -f master &&
+	git reset --hard B &&
+	git branch -f other C &&
+	git checkout other &&
+	git checkout master &&
+	test_must_fail git merge @{-12}
+'
+
 test_done
 
-- 
1.6.2.rc0.80.ge11dd

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Johannes Schindelin]

Hi,

On Sat, 14 Feb 2009, Junio C Hamano wrote:

>  * I know "show-branch @{-1} HEAD" does not dereference @{-1}, but I
>    suspect there may be many others.

I scanned command-list.txt briefly, and could verify (by testing rather 
than code inspection) that

- bundle works as expected (thanks to dwim_ref()),

- fast-export (like bundle, dwim_ref() makes it work),

- so does filter-branch, thanks to rev-parse --symbolic-full-name 
  resolving @{-<n>} correctly,

- log and reflog also substitute @{-<n>} by the branch name,

- update-ref does _not_ interpret @{-<n>} as the branch name (which I 
  find okay, as update-ref is plumbing), but instead creates the file 
  '.git/@{-<n>}' (which I think is a bug).

Now, show-branch indeed does not substitute the branch _name_, but 
otherwise it works correctly, no?  And given the fact that show-branch 
does not substitute "HEAD" with the current branch name, either, I wonder 
if show-branch needs fixing at all.

Ciao,
Dscho

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> - update-ref does _not_ interpret @{-<n>} as the branch name (which I 
>   find okay, as update-ref is plumbing), but instead creates the file 
>   '.git/@{-<n>}' (which I think is a bug).

I do not think it make any sense for update-ref to expand @{-1} to the
name of the last branch, as you end up creating a not-a-ref ".git/next"
and not ".git/refs/heads/next".  It might be Ok to say that @{-1} expands
to refs/heads followed by the name of the last branch only for this
plumbing command, but such an inconsistency feels very wrong.

The particular plumbing command, following the example of recent loosening
the rule back to "HEAD is allowed to point outside refs/heads/", should be
allowed to create anything that passes check_ref_format(), and perhaps bit
more because we would need to allow ".git/HEAD".

> Now, show-branch indeed does not substitute the branch _name_, but 
> otherwise it works correctly, no?
> otherwise it works correctly, no?  And given the fact that show-branch 
> does not substitute "HEAD" with the current branch name, either,...

The tip commits come from get_sha1(), so obviously it would "work", but it
does not show what branch it talks about.  HEAD is shown only when the
user says HEAD (e.g. "git show-branch master HEAD") so I do not see much
similarity there.

The advertised new feature is that you can use the @{-1} syntax anywhere
you would use a branch name and it would work as if you typed the name of
the branch, so in that sense show-branch that shows @{-1} and does not
show the name of the branch is a bug.

It is the same thing for "git-merge @{-1}".  Sure, it would record a
correct tree in the resulting commit, but without the fix it would say
"Merge branch @{-1}" which is wrong and needs to be fixed.

And no, "update-ref @{-1}" should not create ".git/next" when you were on
'next' the last time.  When you say "update-ref next", you may have typed
the four characters and that may look the same as the name of a branch,
but obviously the argument to the command does not *mean* the name of a
branch, so it falls outside the "anywhere you would use a branch name"
rule.

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> The advertised new feature is that you can use the @{-1} syntax anywhere
> you would use a branch name and it would work as if you typed the name of
> the branch, so in that sense show-branch that shows @{-1} and does not
> show the name of the branch is a bug.

I've checked "checkout -b new -t @{-1}", too.  Just for completeness's
sake.

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Nanako Shiraishi]

Quoting Junio C Hamano <gitster@pobox.com>:

> The tip commits come from get_sha1(), so obviously it would "work", but it
> does not show what branch it talks about.  HEAD is shown only when the
> user says HEAD (e.g. "git show-branch master HEAD") so I do not see much
> similarity there.
>
> The advertised new feature is that you can use the @{-1} syntax anywhere
> you would use a branch name and it would work as if you typed the name of
> the branch, so in that sense show-branch that shows @{-1} and does not
> show the name of the branch is a bug.
>
> It is the same thing for "git-merge @{-1}".  Sure, it would record a
> correct tree in the resulting commit, but without the fix it would say
> "Merge branch @{-1}" which is wrong and needs to be fixed.

I am sorry, but I don't understand your logic.

'git-merge' can merge any commit. The argument doesn't have to be a branch name. But it behaves differently when the argument is a branch name. Therefore, @{-1} syntax must behave in the same way as any branch name. I can understand that part.

What I don't understand is your comments about 'git-show-branch'. It also takes any commits, not just branch names, and it's output doesn't seem to treat a branch name any specially from other commits.

I think Johannes'es analogy with HEAD is correct.

-- 
Nanako Shiraishi
http://ivory.ap.teacup.com/nanako3/

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

Nanako Shiraishi <nanako3@lavabit.com> writes:

> What I don't understand is your comments about 'git-show-branch'. It
> also takes any commits, not just branch names, and it's output doesn't
> seem to treat a branch name any specially from other commits.
>
> I think Johannes'es analogy with HEAD is correct.

I agree with you after thinking about it again; I stand corrected.

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Johannes Schindelin]

Hi,

On Sat, 14 Feb 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > - update-ref does _not_ interpret @{-<n>} as the branch name (which I 
> >   find okay, as update-ref is plumbing), but instead creates the file 
> >   '.git/@{-<n>}' (which I think is a bug).
> 
> I do not think it make any sense for update-ref to expand @{-1} to the
> name of the last branch,

As I said, I find the behavior okay.

> The particular plumbing command, following the example of recent 
> loosening the rule back to "HEAD is allowed to point outside 
> refs/heads/", should be allowed to create anything that passes 
> check_ref_format(), and perhaps bit more because we would need to allow 
> ".git/HEAD".

That was what I was referring to when talking about a bug.

> > Now, show-branch indeed does not substitute the branch _name_, but 
> > otherwise it works correctly, no?
> > otherwise it works correctly, no?  And given the fact that show-branch 
> > does not substitute "HEAD" with the current branch name, either,...
> 
> The tip commits come from get_sha1(), so obviously it would "work", but it
> does not show what branch it talks about.  HEAD is shown only when the
> user says HEAD (e.g. "git show-branch master HEAD") so I do not see much
> similarity there.

Well, it _is_ a substitution.  If you ask for "HEAD", you get "HEAD".  As 
a new user, if passing "@{-2}" to show-branch would not show "@{-2}", but 
DWIM the branch name, I would expect, really, that passing "HEAD" does the 
same sort of DWIMming for me.

In the name of consistency, I'd rather leave show-branch as it is.

However, IMO there is a much worse issue lurking: people might want to 
_know_ what "@{-2}" would expand to, _without_ switching branches.  At the 
moment, I can only think of

	git show -s --decorate --pretty=format:%d @{-2}

which is

- convoluted (I would not expect _any_ new user to think of this),

- wrong (it shows all the refs that point to the commit, not only the one 
  we wanted to find out).

And no, do not suggest using "git rev-parse --symbolic-full-name @{-2}", 
you know exactly what I think of teaching new users plumbing, and you know 
I am right in this respect.

Ciao,
Dscho

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> In the name of consistency, I'd rather leave show-branch as it is.

Yes, Nana explained why I was wrong on this one.

> However, IMO there is a much worse issue lurking: people might want to 
> _know_ what "@{-2}" would expand to, _without_ switching branches.

"git branch --history" anyone?

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Johannes Schindelin]

Hi,

On Sun, 15 Feb 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > However, IMO there is a much worse issue lurking: people might want to 
> > _know_ what "@{-2}" would expand to, _without_ switching branches.
> 
> "git branch --history" anyone?

For a moment, you had me convinced it already exists, and I only 
understood after "git grep history builtin-branch.c" came up empty.

;-)

Ciao,
Dscho

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Jay Soffian]

On Mon, Feb 16, 2009 at 5:41 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>> "git branch --history" anyone?
>
> For a moment, you had me convinced it already exists, and I only
> understood after "git grep history builtin-branch.c" came up empty.

And I've labeled that mail as TODO. :-)

j.

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi,
>
> On Sun, 15 Feb 2009, Junio C Hamano wrote:
>
>> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
>> 
>> > However, IMO there is a much worse issue lurking: people might want to 
>> > _know_ what "@{-2}" would expand to, _without_ switching branches.
>> 
>> "git branch --history" anyone?
>
> For a moment, you had me convinced it already exists, and I only 
> understood after "git grep history builtin-branch.c" came up empty.
>
> ;-)

$ git rev-parse --symbolic-full-name @{-24}

seem to work for this purpose ;-)

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Johannes Schindelin]

Hi,

On Wed, 25 Feb 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Sun, 15 Feb 2009, Junio C Hamano wrote:
> >
> >> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> >> 
> >> > However, IMO there is a much worse issue lurking: people might want 
> >> > to _know_ what "@{-2}" would expand to, _without_ switching 
> >> > branches.
> >> 
> >> "git branch --history" anyone?
> >
> > For a moment, you had me convinced it already exists, and I only 
> > understood after "git grep history builtin-branch.c" came up empty.
> >
> > ;-)
> 
> $ git rev-parse --symbolic-full-name @{-24}
> 
> seem to work for this purpose ;-)

AFAIR I mentioned that command, together with a reminder that teaching 
regular Git users plumbing commands is harmful.

Ciao,
Dscho

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Deskin Miller]

On Sat, Feb 14, 2009 at 17:52, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> I scanned command-list.txt briefly, and could verify (by testing rather
> than code inspection) that
>
> - bundle works as expected (thanks to dwim_ref()),
>
> - fast-export (like bundle, dwim_ref() makes it work),
>
> - so does filter-branch, thanks to rev-parse --symbolic-full-name
>  resolving @{-<n>} correctly,
>
> - log and reflog also substitute @{-<n>} by the branch name,
>
> - update-ref does _not_ interpret @{-<n>} as the branch name (which I
>  find okay, as update-ref is plumbing), but instead creates the file
>  '.git/@{-<n>}' (which I think is a bug).
>

I did 'git branch -M @{-1}' yesterday and ended up renaming my branch
to refs/heads/@{-1}.  Personally my usage felt like it should work
(but of course I think that); curious to know what others think, if
this is a bug.

Deskin Miller

Re: [PATCH v2 2/2] Teach @{-1} to git merge

[Junio C Hamano]

Deskin Miller <deskinm@gmail.com> writes:

> I did 'git branch -M @{-1}' yesterday and ended up renaming my branch
> to refs/heads/@{-1}.  Personally my usage felt like it should work
> (but of course I think that); curious to know what others think, if
> this is a bug.

The @{-n} notation is supposed to be usable anywhere you would write a
branch name and should behave as if you wrote the name itself.  And 'git
branch -M <name> [<name>]' does expect a branch name, not a random
extended SHA-1 expression.

I'd say it is a bug.

[PATCH] git-branch: a detached HEAD and branch called HEAD are different

[Junio C Hamano]

While on a detached HEAD, builtin-branch.c passed string "HEAD" around in
the varilable "head" that held the current branch name.  This did not
allow you to distinguish between a local branch whose name is "HEAD" and a
detached HEAD.

Allow the variable to contain NULL when the HEAD is detached.  The change
is primarily to protect !strcmp(head, name) to see if we are try to
manipulate the current branch with "head &&"; we won't be futzing with the
current branch when the HEAD is detached by definition.

Creating a local branch "HEAD" is not useful at all, but if you managed to
create one by mistake, "git branch -d HEAD" couldn't delete it while your
HEAD is detached, due to this bug.

One of the functions, rename_branch(), even expected NULL to signal this
situation, to prevent "git branch -m othername" while on detached HEAD,
and this change incidentally fixes this unrelated bug.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I wasn't personally very much interested in these "C-rewrites" of
   Porcelains, and I have never looked at the implementation very
   carefully, but some parts of them are disgustingly crappy, and I am
   finding these inconsistencies after looking at them only for 5
   minutes.  Oh well...

   Anyway, this is a preparatory clean-up.

 branch.c         |    2 +-
 builtin-branch.c |    8 +++++---
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/branch.c b/branch.c
index 1f00e44..e287eab 100644
--- a/branch.c
+++ b/branch.c
@@ -121,7 +121,7 @@ void create_branch(const char *head,
 	if (resolve_ref(ref.buf, sha1, 1, NULL)) {
 		if (!force)
 			die("A branch named '%s' already exists.", name);
-		else if (!is_bare_repository() && !strcmp(head, name))
+		else if (!is_bare_repository() && head && !strcmp(head, name))
 			die("Cannot force update the current branch.");
 		forcing = 1;
 	}
diff --git a/builtin-branch.c b/builtin-branch.c
index 504a981..c34af27 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -126,7 +126,7 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 		if (interpret_nth_last_branch(argv[i], &bname) != len)
 			strbuf_add(&bname, argv[i], len);
 
-		if (kinds == REF_LOCAL_BRANCH && !strcmp(head, bname.buf)) {
+		if (kinds == REF_LOCAL_BRANCH && head && !strcmp(head, bname.buf)) {
 			error("Cannot delete the branch '%s' "
 			      "which you are currently on.", bname.buf);
 			ret = 1;
@@ -408,7 +408,7 @@ static void print_ref_list(int kinds, int detached, int verbose, int abbrev, str
 	for (i = 0; i < ref_list.index; i++) {
 		int current = !detached &&
 			(ref_list.list[i].kind == REF_LOCAL_BRANCH) &&
-			!strcmp(ref_list.list[i].name, head);
+			head && !strcmp(ref_list.list[i].name, head);
 		print_ref_item(&ref_list.list[i], ref_list.maxwidth, verbose,
 			       abbrev, current);
 	}
@@ -541,6 +541,7 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	head = xstrdup(head);
 	if (!strcmp(head, "HEAD")) {
 		detached = 1;
+		head = NULL;
 	} else {
 		if (prefixcmp(head, "refs/heads/"))
 			die("HEAD not found below refs/heads!");
@@ -561,7 +562,8 @@ int cmd_branch(int argc, const char **argv, const char *prefix)
 	else if (rename && (argc == 2))
 		rename_branch(argv[0], argv[1], rename > 1);
 	else if (argc <= 2)
-		create_branch(head, argv[0], (argc == 2) ? argv[1] : head,
+		create_branch(head, argv[0],
+			      (argc == 2) ? argv[1] : head ? head : "HEAD",
 			      force_create, reflog, track);
 	else
 		usage_with_options(builtin_branch_usage, options);

[PATCH] "git branch -M @{-1}"

[Junio C Hamano]

Earlier, we've taught "git branch -d <name>" that <name> is supposed to be
a branch name and not a random extended SHA-1.  "git branch -m <name> [<name>]" 
needs to be taught the same.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 builtin-branch.c |   14 ++++++++++----
 1 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index c34af27..3f97838 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -425,21 +425,27 @@ static void rename_branch(const char *oldname, const char *newname, int force)
 	if (!oldname)
 		die("cannot rename the current branch while not on any.");
 
-	strbuf_addf(&oldref, "refs/heads/%s", oldname);
+	if (interpret_nth_last_branch(oldname, &oldref) == strlen(oldname))
+		strbuf_splice(&oldref, 0, 0, "refs/heads/", 11);
+	else
+		strbuf_addf(&oldref, "refs/heads/%s", oldname);
 
 	if (check_ref_format(oldref.buf))
 		die("Invalid branch name: %s", oldref.buf);
 
-	strbuf_addf(&newref, "refs/heads/%s", newname);
+	if (interpret_nth_last_branch(newname, &newref) == strlen(newname))
+		strbuf_splice(&newref, 0, 0, "refs/heads/", 11);
+	else
+		strbuf_addf(&newref, "refs/heads/%s", newname);
 
 	if (check_ref_format(newref.buf))
 		die("Invalid branch name: %s", newref.buf);
 
 	if (resolve_ref(newref.buf, sha1, 1, NULL) && !force)
-		die("A branch named '%s' already exists.", newname);
+		die("A branch named '%s' already exists.", newref.buf + 11);
 
 	strbuf_addf(&logmsg, "Branch: renamed %s to %s",
-		 oldref.buf, newref.buf);
+		    oldref.buf, newref.buf);
 
 	if (rename_ref(oldref.buf, newref.buf, logmsg.buf))
 		die("Branch rename failed");

Re: [PATCH] "git branch -M @{-1}"

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Earlier, we've taught "git branch -d <name>" that <name> is supposed to be
> a branch name and not a random extended SHA-1.  "git branch -m <name> [<name>]" 
> needs to be taught the same.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  builtin-branch.c |   14 ++++++++++----
>  1 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/builtin-branch.c b/builtin-branch.c
> index c34af27..3f97838 100644
> --- a/builtin-branch.c
> +++ b/builtin-branch.c
> @@ -425,21 +425,27 @@ static void rename_branch(const char *oldname, const char *newname, int force)
>  	if (!oldname)
>  		die("cannot rename the current branch while not on any.");
>  
> -	strbuf_addf(&oldref, "refs/heads/%s", oldname);
> +	if (interpret_nth_last_branch(oldname, &oldref) == strlen(oldname))
> +		strbuf_splice(&oldref, 0, 0, "refs/heads/", 11);
> +	else
> +		strbuf_addf(&oldref, "refs/heads/%s", oldname);

A few comments on this patch.

 (1) I am actually very reluctant to queue this, or any unproven "fix"
     that adds obscure codepath, for 1.6.2, this close to the final.  Most
     likely I won't be queueing any more @{-n] fixes until 1.6.2 final and
     defer them to 1.6.2.X series if necessary.

 (2) If we are to have full @{-n} support, and if we do not mind small
     leaks, it would make more sense to introduce a leaky helper function
     like this and use it everywhere:

	const char *handle_branch_arg(const char *name)
        {
        	struct strbuf buf = STRBUF_INIT;
		size_t len = strlen(name);
                if (interpret_nth_last_branch(name, &buf) == len)
			return strbuf_detach(&buf);
		return name;
	}

 (3) I think this "The argument could be previous branch name" conversion
     should be done only when the command is about local branch names
     (i.e. when run without -r option), and the above codepath needs to be
     protected with if (kinds == REF_LOCAL_BRANCH)