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

Jeff King wrote:

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

quoted

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:

quoted

-			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