[PATCH] git-branch: display sha1 on branch deletion

[PATCH] git-branch: display sha1 on branch deletion

[Brandon Casey]

Make it easier to recover from a mistaken branch deletion by displaying the
sha1 of the branch's tip commit.

Update t3200.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin-branch.c  |    3 ++-
 t/t3200-branch.sh |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 494cbac..b3d0d20 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -165,7 +165,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 			ret = 1;
 		} else {
 			struct strbuf buf = STRBUF_INIT;
-			printf("Deleted %sbranch %s.\n", remote, argv[i]);
+			printf("Deleted %sbranch %s (%s).\n", remote, argv[i],
+                                sha1_to_hex(sha1));
 			strbuf_addf(&buf, "branch.%s", argv[i]);
 			if (git_config_rename_section(buf.buf, NULL) < 0)
 				warning("Update of config-file failed");
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 25e9971..e9bb6d5 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -194,7 +194,8 @@ test_expect_success 'test deleting branch deletes branch config' \
 
 test_expect_success 'test deleting branch without config' \
     'git branch my7 s &&
-     test "$(git branch -d my7 2>&1)" = "Deleted branch my7."'
+     sha1=$(git rev-parse my7) &&
+     test "$(git branch -d my7 2>&1)" = "Deleted branch my7 ($sha1)."'
 
 test_expect_success 'test --track without .fetch entries' \
     'git branch --track my8 &&
-- 
1.6.0.4.794.g35fad

Re: [PATCH] git-branch: display sha1 on branch deletion

[Jeff King]

On Fri, Dec 12, 2008 at 01:29:01PM -0600, Brandon Casey wrote:

> Make it easier to recover from a mistaken branch deletion by displaying the
> sha1 of the branch's tip commit.

I think this is reasonable behavior, but I have two comments:

> -			printf("Deleted %sbranch %s.\n", remote, argv[i]);
> +			printf("Deleted %sbranch %s (%s).\n", remote, argv[i],
> +                                sha1_to_hex(sha1));

1. Any reason not to use find_unique_abbrev(sha1, DEFAULT_ABBREV) here?
   The full 40-character sha1 kind of dominates the line, especially if
   you have short branch name. And this is not really for long-term
   usage, but rather "oops, I didn't mean to have just deleted that".

2. I wonder if it is confusing to new users to simply say "Delete branch
   $branch ($sha1)". We haven't deleted $sha1, just the branch pointer.
   $sha1 is probably still in the HEAD reflog, if not in another branch.
   Maybe something like "(was $sha1)" would be appropriate.

I don't know if '2' is a big deal. I haven't been a new user for a long
time, so I didn't personally find it confusing (especially with '1' so
that you actually notice the branch name rather than the gigantic sha1).

-Peff

Re: [PATCH] git-branch: display sha1 on branch deletion

[Brandon Casey]

Jeff King wrote:
> On Fri, Dec 12, 2008 at 01:29:01PM -0600, Brandon Casey wrote:
> 
>> Make it easier to recover from a mistaken branch deletion by displaying the
>> sha1 of the branch's tip commit.
> 
> I think this is reasonable behavior, but I have two comments:
> 
>> -			printf("Deleted %sbranch %s.\n", remote, argv[i]);
>> +			printf("Deleted %sbranch %s (%s).\n", remote, argv[i],
>> +                                sha1_to_hex(sha1));
> 
> 1. Any reason not to use find_unique_abbrev(sha1, DEFAULT_ABBREV) here?

I didn't consider using find_unique_abbrev(). I used the same format
that 'git stash drop' uses. There is enough room for a 20-char branch
name without overflowing an 80-char line (unless you're deleting a
remote branch). I kind of like the exactness of the full sha1 in this
case.

>    The full 40-character sha1 kind of dominates the line, especially if
>    you have short branch name. And this is not really for long-term
>    usage, but rather "oops, I didn't mean to have just deleted that".

Actually, I'm more worried about line wrapping with a long branch name
than a short branch name being dominated by the 40-char sha1. I'd even
consider dropping the the word "branch " from the "Deleted branch ..."
message since the user already knows that a branch is being deleted.
So, this word-wrapping argument holds weight with me.

We read left to right and the sha1 is delimited by parenthesis, so I'm
not sure if the sha1 is dominating the line to the extent that it would
cause confusion.

For comparison:

  $ git stash drop
  Dropped refs/stash@{0} (a8381dd3d3ea4e7bc83c2bfe7d1c27fa180632ee)

compared to:

  $ git branch -d my_branch_name
  Deleted branch my_branch_name (ca26e31d5442f3d1ef8ae1cb69970fb31ed4b841).

or

  $ git branch -d my_branch_name
  Deleted branch my_branch_name (ca26e31d).

or

  $ git branch -d my_branch_name
  Deleted my_branch_name (ca26e31d5442f3d1ef8ae1cb69970fb31ed4b841).

But since the longest branch that has been merged in the git history
was 40 characters, maybe it does make sense to abbreviate the sha1.

> 2. I wonder if it is confusing to new users to simply say "Delete branch
>    $branch ($sha1)". We haven't deleted $sha1, just the branch pointer.
>    $sha1 is probably still in the HEAD reflog, if not in another branch.
>    Maybe something like "(was $sha1)" would be appropriate.
>
> I don't know if '2' is a big deal. I haven't been a new user for a long
> time, so I didn't personally find it confusing (especially with '1' so
> that you actually notice the branch name rather than the gigantic sha1).

  Deleted branch my_branch_name (was ca26e31d).

I don't find it confusing (obviously) without the addition of "was ", but
I too haven't been a new user in a while.

-brandon

[PATCH v2] git-branch: display sha1 on branch deletion

[Brandon Casey]

Make it easier to recover from a mistaken branch deletion by displaying the
sha1 of the branch's tip commit.

Update t3200.

Signed-off-by: Brandon Casey <casey@nrlssc.navy.mil>
---
 builtin-branch.c  |    3 ++-
 t/t3200-branch.sh |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/builtin-branch.c b/builtin-branch.c
index 494cbac..02fa38f 100644
--- a/builtin-branch.c
+++ b/builtin-branch.c
@@ -165,7 +165,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)
 			ret = 1;
 		} else {
 			struct strbuf buf = STRBUF_INIT;
-			printf("Deleted %sbranch %s.\n", remote, argv[i]);
+			printf("Deleted %sbranch %s (%s).\n", remote, argv[i],
+				find_unique_abbrev(sha1, DEFAULT_ABBREV));
 			strbuf_addf(&buf, "branch.%s", argv[i]);
 			if (git_config_rename_section(buf.buf, NULL) < 0)
 				warning("Update of config-file failed");
diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 25e9971..61a2010 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -194,7 +194,8 @@ test_expect_success 'test deleting branch deletes branch config' \
 
 test_expect_success 'test deleting branch without config' \
     'git branch my7 s &&
-     test "$(git branch -d my7 2>&1)" = "Deleted branch my7."'
+     sha1=$(git rev-parse my7 | cut -c 1-7) &&
+     test "$(git branch -d my7 2>&1)" = "Deleted branch my7 ($sha1)."'
 
 test_expect_success 'test --track without .fetch entries' \
     'git branch --track my8 &&
-- 
1.6.0.4.794.g35fad

Re: [PATCH v2] git-branch: display sha1 on branch deletion

[Jeff King]

On Fri, Dec 12, 2008 at 05:20:07PM -0600, Brandon Casey wrote:

> Make it easier to recover from a mistaken branch deletion by displaying the
> sha1 of the branch's tip commit.

This version looks fine to me, but one nit:

> -     test "$(git branch -d my7 2>&1)" = "Deleted branch my7."'
> +     sha1=$(git rev-parse my7 | cut -c 1-7) &&
> +     test "$(git branch -d my7 2>&1)" = "Deleted branch my7 ($sha1)."'

There is a very very small chance that this sha1 might require more
than 7 characters to be unique (small because we have such a tiny number
of objects in the trash repo). Maybe:

  sha1=$(git log --pretty=format:%h -1 my7)

is better (though I have to admit, if I were writing the test originally
I would have tested the exit value of "git branch" instead of the
message).

-Peff

Re: [PATCH v2] git-branch: display sha1 on branch deletion

[Brandon Casey]

Jeff King wrote:
> On Fri, Dec 12, 2008 at 05:20:07PM -0600, Brandon Casey wrote:
> 
>> Make it easier to recover from a mistaken branch deletion by displaying the
>> sha1 of the branch's tip commit.
> 
> This version looks fine to me, but one nit:
> 
>> -     test "$(git branch -d my7 2>&1)" = "Deleted branch my7."'
>> +     sha1=$(git rev-parse my7 | cut -c 1-7) &&
>> +     test "$(git branch -d my7 2>&1)" = "Deleted branch my7 ($sha1)."'
> 
> There is a very very small chance that this sha1 might require more
> than 7 characters to be unique (small because we have such a tiny number
> of objects in the trash repo).

Yeah I was counting on the smallness of that chance to let me be lazy and
not think about how to get the proper abbreviated sha1.

> Maybe:
> 
>   sha1=$(git log --pretty=format:%h -1 my7)
> 
> is better (though I have to admit, if I were writing the test originally
> I would have tested the exit value of "git branch" instead of the
> message).

My feelings won't be hurt if you want to change it.

-brandon