{"thread":{"id":"16701","subject":"[PATCH] git-branch: display sha1 on branch deletion","startedAt":"2008-12-12T19:29:01Z","lastAt":"2008-12-15T17:23:06Z","messageCount":6,"participants":["Brandon Casey","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"97726","messageId":"uTIFnEi0iyLKcAungf7u1Gu2xl50j9i-AMiZaQp-QTs1q-ppgyHZoelGLgvK7BFKpYE03BLRHJ4@cipher.nrlssc.navy.mil","threadId":"16701","inReplyTo":null,"subject":"[PATCH] git-branch: display sha1 on branch deletion","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-12-12T19:29:01Z","receivedAt":"2008-12-12T19:29:01Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Make it easier to recover from a mistaken branch deletion by displaying the\nsha1 of the branch's tip commit.\n\nUpdate t3200.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n builtin-branch.c  |    3 ++-\n t/t3200-branch.sh |    3 ++-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 494cbac..b3d0d20 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -165,7 +165,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t\tret = 1;\n \t\t} else {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\t\tprintf(\"Deleted %sbranch %s.\\n\", remote, argv[i]);\n+\t\t\tprintf(\"Deleted %sbranch %s (%s).\\n\", remote, argv[i],\n+                                sha1_to_hex(sha1));\n \t\t\tstrbuf_addf(&buf, \"branch.%s\", argv[i]);\n \t\t\tif (git_config_rename_section(buf.buf, NULL) < 0)\n \t\t\t\twarning(\"Update of config-file failed\");\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 25e9971..e9bb6d5 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -194,7 +194,8 @@ test_expect_success 'test deleting branch deletes branch config' \\\n \n test_expect_success 'test deleting branch without config' \\\n     'git branch my7 s &&\n-     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7.\"'\n+     sha1=$(git rev-parse my7) &&\n+     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7 ($sha1).\"'\n \n test_expect_success 'test --track without .fetch entries' \\\n     'git branch --track my8 &&\n-- \n1.6.0.4.794.g35fad\n"},{"id":"97729","messageId":"20081212194349.GA5486@sigill.intra.peff.net","threadId":"16701","inReplyTo":"uTIFnEi0iyLKcAungf7u1Gu2xl50j9i-AMiZaQp-QTs1q-ppgyHZoelGLgvK7BFKpYE03BLRHJ4@cipher.nrlssc.navy.mil","subject":"Re: [PATCH] git-branch: display sha1 on branch deletion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-12T19:43:51Z","receivedAt":"2008-12-12T19:43:51Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 12, 2008 at 01:29:01PM -0600, Brandon Casey wrote:\n\n> Make it easier to recover from a mistaken branch deletion by displaying the\n> sha1 of the branch's tip commit.\n\nI think this is reasonable behavior, but I have two comments:\n\n> -\t\t\tprintf(\"Deleted %sbranch %s.\\n\", remote, argv[i]);\n> +\t\t\tprintf(\"Deleted %sbranch %s (%s).\\n\", remote, argv[i],\n> +                                sha1_to_hex(sha1));\n\n1. Any reason not to use find_unique_abbrev(sha1, DEFAULT_ABBREV) here?\n   The full 40-character sha1 kind of dominates the line, especially if\n   you have short branch name. And this is not really for long-term\n   usage, but rather \"oops, I didn't mean to have just deleted that\".\n\n2. I wonder if it is confusing to new users to simply say \"Delete branch\n   $branch ($sha1)\". We haven't deleted $sha1, just the branch pointer.\n   $sha1 is probably still in the HEAD reflog, if not in another branch.\n   Maybe something like \"(was $sha1)\" would be appropriate.\n\nI don't know if '2' is a big deal. I haven't been a new user for a long\ntime, so I didn't personally find it confusing (especially with '1' so\nthat you actually notice the branch name rather than the gigantic sha1).\n\n-Peff\n"},{"id":"97760","messageId":"AeIdfC6dcUaLaDHDaAdSq8Mxz6QoUdeypSUI24erUEyAHVz4HeTSCw@cipher.nrlssc.navy.mil","threadId":"16701","inReplyTo":"20081212194349.GA5486@sigill.intra.peff.net","subject":"Re: [PATCH] git-branch: display sha1 on branch deletion","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-12-12T23:17:44Z","receivedAt":"2008-12-12T23:17:44Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Jeff King wrote:\n> On Fri, Dec 12, 2008 at 01:29:01PM -0600, Brandon Casey wrote:\n> \n>> Make it easier to recover from a mistaken branch deletion by displaying the\n>> sha1 of the branch's tip commit.\n> \n> I think this is reasonable behavior, but I have two comments:\n> \n>> -\t\t\tprintf(\"Deleted %sbranch %s.\\n\", remote, argv[i]);\n>> +\t\t\tprintf(\"Deleted %sbranch %s (%s).\\n\", remote, argv[i],\n>> +                                sha1_to_hex(sha1));\n> \n> 1. Any reason not to use find_unique_abbrev(sha1, DEFAULT_ABBREV) here?\n\nI didn't consider using find_unique_abbrev(). I used the same format\nthat 'git stash drop' uses. There is enough room for a 20-char branch\nname without overflowing an 80-char line (unless you're deleting a\nremote branch). I kind of like the exactness of the full sha1 in this\ncase.\n\n>    The full 40-character sha1 kind of dominates the line, especially if\n>    you have short branch name. And this is not really for long-term\n>    usage, but rather \"oops, I didn't mean to have just deleted that\".\n\nActually, I'm more worried about line wrapping with a long branch name\nthan a short branch name being dominated by the 40-char sha1. I'd even\nconsider dropping the the word \"branch \" from the \"Deleted branch ...\"\nmessage since the user already knows that a branch is being deleted.\nSo, this word-wrapping argument holds weight with me.\n\nWe read left to right and the sha1 is delimited by parenthesis, so I'm\nnot sure if the sha1 is dominating the line to the extent that it would\ncause confusion.\n\nFor comparison:\n\n  $ git stash drop\n  Dropped refs/stash@{0} (a8381dd3d3ea4e7bc83c2bfe7d1c27fa180632ee)\n\ncompared to:\n\n  $ git branch -d my_branch_name\n  Deleted branch my_branch_name (ca26e31d5442f3d1ef8ae1cb69970fb31ed4b841).\n\nor\n\n  $ git branch -d my_branch_name\n  Deleted branch my_branch_name (ca26e31d).\n\nor\n\n  $ git branch -d my_branch_name\n  Deleted my_branch_name (ca26e31d5442f3d1ef8ae1cb69970fb31ed4b841).\n\nBut since the longest branch that has been merged in the git history\nwas 40 characters, maybe it does make sense to abbreviate the sha1.\n\n> 2. I wonder if it is confusing to new users to simply say \"Delete branch\n>    $branch ($sha1)\". We haven't deleted $sha1, just the branch pointer.\n>    $sha1 is probably still in the HEAD reflog, if not in another branch.\n>    Maybe something like \"(was $sha1)\" would be appropriate.\n>\n> I don't know if '2' is a big deal. I haven't been a new user for a long\n> time, so I didn't personally find it confusing (especially with '1' so\n> that you actually notice the branch name rather than the gigantic sha1).\n\n  Deleted branch my_branch_name (was ca26e31d).\n\nI don't find it confusing (obviously) without the addition of \"was \", but\nI too haven't been a new user in a while.\n\n-brandon\n"},{"id":"97763","messageId":"UfCPFu_boLV0ycLKLmOno8GTqmtC4RSZZ9O6aRLGYjmSZOdKv6ywhCjApnplTHLxUIzO8KJ5Mww@cipher.nrlssc.navy.mil","threadId":"16701","inReplyTo":"AeIdfC6dcUaLaDHDaAdSq8Mxz6QoUdeypSUI24erUEyAHVz4HeTSCw@cipher.nrlssc.navy.mil","subject":"[PATCH v2] git-branch: display sha1 on branch deletion","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-12-12T23:20:07Z","receivedAt":"2008-12-12T23:20:07Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Make it easier to recover from a mistaken branch deletion by displaying the\nsha1 of the branch's tip commit.\n\nUpdate t3200.\n\nSigned-off-by: Brandon Casey <casey@nrlssc.navy.mil>\n---\n builtin-branch.c  |    3 ++-\n t/t3200-branch.sh |    3 ++-\n 2 files changed, 4 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-branch.c b/builtin-branch.c\nindex 494cbac..02fa38f 100644\n--- a/builtin-branch.c\n+++ b/builtin-branch.c\n@@ -165,7 +165,8 @@ static int delete_branches(int argc, const char **argv, int force, int kinds)\n \t\t\tret = 1;\n \t\t} else {\n \t\t\tstruct strbuf buf = STRBUF_INIT;\n-\t\t\tprintf(\"Deleted %sbranch %s.\\n\", remote, argv[i]);\n+\t\t\tprintf(\"Deleted %sbranch %s (%s).\\n\", remote, argv[i],\n+\t\t\t\tfind_unique_abbrev(sha1, DEFAULT_ABBREV));\n \t\t\tstrbuf_addf(&buf, \"branch.%s\", argv[i]);\n \t\t\tif (git_config_rename_section(buf.buf, NULL) < 0)\n \t\t\t\twarning(\"Update of config-file failed\");\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 25e9971..61a2010 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -194,7 +194,8 @@ test_expect_success 'test deleting branch deletes branch config' \\\n \n test_expect_success 'test deleting branch without config' \\\n     'git branch my7 s &&\n-     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7.\"'\n+     sha1=$(git rev-parse my7 | cut -c 1-7) &&\n+     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7 ($sha1).\"'\n \n test_expect_success 'test --track without .fetch entries' \\\n     'git branch --track my8 &&\n-- \n1.6.0.4.794.g35fad\n"},{"id":"97786","messageId":"20081213063104.GA8480@coredump.intra.peff.net","threadId":"16701","inReplyTo":"UfCPFu_boLV0ycLKLmOno8GTqmtC4RSZZ9O6aRLGYjmSZOdKv6ywhCjApnplTHLxUIzO8KJ5Mww@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2] git-branch: display sha1 on branch deletion","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2008-12-13T06:31:05Z","receivedAt":"2008-12-13T06:31:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Dec 12, 2008 at 05:20:07PM -0600, Brandon Casey wrote:\n\n> Make it easier to recover from a mistaken branch deletion by displaying the\n> sha1 of the branch's tip commit.\n\nThis version looks fine to me, but one nit:\n\n> -     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7.\"'\n> +     sha1=$(git rev-parse my7 | cut -c 1-7) &&\n> +     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7 ($sha1).\"'\n\nThere is a very very small chance that this sha1 might require more\nthan 7 characters to be unique (small because we have such a tiny number\nof objects in the trash repo). Maybe:\n\n  sha1=$(git log --pretty=format:%h -1 my7)\n\nis better (though I have to admit, if I were writing the test originally\nI would have tested the exit value of \"git branch\" instead of the\nmessage).\n\n-Peff\n"},{"id":"97981","messageId":"urlVZPqgW2MvLyyMWmI_xoDCxnKMd-kyAs1vihhFHBiFmQ3aPtF7Pw@cipher.nrlssc.navy.mil","threadId":"16701","inReplyTo":"20081213063104.GA8480@coredump.intra.peff.net","subject":"Re: [PATCH v2] git-branch: display sha1 on branch deletion","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2008-12-15T17:23:06Z","receivedAt":"2008-12-15T17:23:06Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"Jeff King wrote:\n> On Fri, Dec 12, 2008 at 05:20:07PM -0600, Brandon Casey wrote:\n> \n>> Make it easier to recover from a mistaken branch deletion by displaying the\n>> sha1 of the branch's tip commit.\n> \n> This version looks fine to me, but one nit:\n> \n>> -     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7.\"'\n>> +     sha1=$(git rev-parse my7 | cut -c 1-7) &&\n>> +     test \"$(git branch -d my7 2>&1)\" = \"Deleted branch my7 ($sha1).\"'\n> \n> There is a very very small chance that this sha1 might require more\n> than 7 characters to be unique (small because we have such a tiny number\n> of objects in the trash repo).\n\nYeah I was counting on the smallness of that chance to let me be lazy and\nnot think about how to get the proper abbreviated sha1.\n\n> Maybe:\n> \n>   sha1=$(git log --pretty=format:%h -1 my7)\n> \n> is better (though I have to admit, if I were writing the test originally\n> I would have tested the exit value of \"git branch\" instead of the\n> message).\n\nMy feelings won't be hurt if you want to change it.\n\n-brandon\n"}]}