threads / patch / 17786

patchteach the new "@{-1} syntax to "git branch"

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

## tl;dr

21 messages between Feb 14, 2009 and Feb 27, 2009. Diffs are folded; open one to read it.

replies: 20people: 5as markdown or json

Junio C Hamano· Feb 14, 2009, 07:08 UTC · lore

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(-)
Show changes to 2 files +33 −17

branch.c, builtin-branch.c

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
Jay Soffian· Feb 14, 2009, 07:11 UTC · re: Junio C Hamano · lore

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

On Sat, Feb 14, 2009 at 2:08 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 9 quoted lines
> 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.
Junio C Hamano· Feb 14, 2009, 07:21 UTC · re: Jay Soffian · lore

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

Jay Soffian <jaysoffian@gmail.com> writes:
Show 12 quoted lines
> 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.

Junio C Hamano· Feb 14, 2009, 09:37 UTC · re: Junio C Hamano · lore

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

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
Show changes to 3 files +59 −17

branch.c, builtin-branch.c, 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
Junio C Hamano· Feb 14, 2009, 09:39 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 2 files +35 −3

builtin-merge.c, t/t0100-previous.sh

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
Johannes Schindelin· Feb 14, 2009, 22:52 UTC · re: Junio C Hamano · lore

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

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

Junio C Hamano· Feb 15, 2009, 02:05 UTC · re: Johannes Schindelin · lore

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

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.

Junio C Hamano· Feb 15, 2009, 02:32 UTC · re: Junio C Hamano · lore

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

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.

Nanako Shiraishi· Feb 15, 2009, 09:24 UTC · re: Junio C Hamano · lore

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

Quoting Junio C Hamano <gitster@pobox.com>:
Show 13 quoted lines
> 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/
Junio C Hamano· Feb 15, 2009, 10:02 UTC · re: Nanako Shiraishi · lore

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

Nanako Shiraishi <nanako3@lavabit.com> writes:
Show 5 quoted lines
> 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.
Johannes Schindelin· Feb 15, 2009, 11:21 UTC · re: Junio C Hamano · lore

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

Hi,
On Sat, 14 Feb 2009, Junio C Hamano wrote:
Show 8 quoted lines
> 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.
Show 5 quoted lines
> 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.
Show 9 quoted lines
> > 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

Junio C Hamano· Feb 16, 2009, 04:16 UTC · re: Johannes Schindelin · lore

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

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?
Johannes Schindelin· Feb 16, 2009, 10:41 UTC · re: Junio C Hamano · lore

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

Hi,
On Sun, 15 Feb 2009, Junio C Hamano wrote:
Show 6 quoted lines
> 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

Jay Soffian· Feb 16, 2009, 16:40 UTC · re: Johannes Schindelin · lore

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

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.
Junio C Hamano· Feb 26, 2009, 00:11 UTC · re: Johannes Schindelin · lore

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

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
Show 15 quoted lines
> 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 ;-)
Johannes Schindelin· Feb 26, 2009, 10:18 UTC · re: Junio C Hamano · lore

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

Hi,
On Wed, 25 Feb 2009, Junio C Hamano wrote:
Show 20 quoted lines
> 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

Deskin Miller· Feb 27, 2009, 04:27 UTC · re: Johannes Schindelin · lore

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

On Sat, Feb 14, 2009 at 17:52, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 16 quoted lines
> 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
Junio C Hamano· Feb 27, 2009, 05:24 UTC · re: Deskin Miller · lore

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

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.
Junio C Hamano· Feb 27, 2009, 06:10 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to 2 files +6 −4

branch.c, builtin-branch.c

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);
Junio C Hamano· Feb 27, 2009, 06:12 UTC · re: Junio C Hamano · lore

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

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(-)
Show changes to builtin-branch.c +10 −4
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");
Junio C Hamano· Feb 27, 2009, 06:49 UTC · re: Junio C Hamano · lore

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

Junio C Hamano <gitster@pobox.com> writes:
Show 22 quoted lines
> 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)

← back to recent threads