{"thread":{"id":"48122","subject":"[PATCH] branch: implement shortcut to delete last branch","startedAt":"2018-03-23T02:09:48Z","lastAt":"2018-03-26T16:49:53Z","messageCount":9,"participants":["Aaron Greenberg","Jeff King","git@matthieu-moy.fr","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"342553","messageId":"1521770966-18383-1-git-send-email-p@aaronjgreenberg.com","threadId":"48122","inReplyTo":null,"subject":"[PATCH] branch: implement shortcut to delete last branch","fromName":"Aaron Greenberg","fromEmail":"p@aaronjgreenberg.com","sentAt":"2018-03-23T02:09:25Z","receivedAt":"2018-03-23T02:09:48Z","isPatch":true,"sender":{"key":"p@aaronjgreenberg.com","avatar":null},"body":"This patch gives git-branch the ability to delete the previous\nchecked-out branch using the \"-\" shortcut. This shortcut already exists\nfor git-checkout, git-merge, and git-revert. One of my common workflows\nis to do some work on a local topic branch and push it to a remote,\nwhere it gets merged in to 'master'. Then, I switch back to my local\nmaster, fetch the remote master, and delete the previous topic branch.\n\n$ git checkout -b topic-a\n$ # Do some work...\n$ git commit -am \"Implement feature A\"\n$ git push origin topic-a\n\n# 'origin/topic-a' gets merged into 'origin/master'\n\n$ git checkout master\n$ git branch -d topic-a\n$ # With this patch, a user could simply type\n$ git branch -d -\n\nI think it's a useful shortcut for cleaning up a just-merged branch\n(or a just switched-from branch.)\n\n"},{"id":"342554","messageId":"1521770966-18383-2-git-send-email-p@aaronjgreenberg.com","threadId":"48122","inReplyTo":"1521770966-18383-1-git-send-email-p@aaronjgreenberg.com","subject":"[PATCH] branch: implement shortcut to delete last branch","fromName":"Aaron Greenberg","fromEmail":"p@aaronjgreenberg.com","sentAt":"2018-03-23T02:09:26Z","receivedAt":"2018-03-23T02:09:51Z","isPatch":true,"sender":{"key":"p@aaronjgreenberg.com","avatar":null},"body":"Add support for using the \"-\" shortcut to delete the last checked-out\nbranch. This functionality already exists for git-merge, git-checkout,\nand git-revert.\n\nSigned-off-by: Aaron Greenberg <p@aaronjgreenberg.com>\n---\n builtin/branch.c  | 3 +++\n t/t3200-branch.sh | 9 +++++++++\n 2 files changed, 12 insertions(+)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 6d0cea9..9e37078 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -221,6 +221,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tchar *target = NULL;\n \t\tint flags = 0;\n \n+\t\tif (!strcmp(argv[i], \"-\"))\n+\t\t\targv[i] = \"@{-1}\";\n+\n \t\tstrbuf_branchname(&bname, argv[i], allowed_interpret);\n \t\tfree(name);\n \t\tname = mkpathdup(fmt, bname.buf);\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 6c0b7ea..a3ffd54 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -776,6 +776,15 @@ test_expect_success 'deleting currently checked out branch fails' '\n \ttest_must_fail git branch -d my7\n '\n \n+test_expect_success 'test deleting last branch' '\n+\tgit checkout -b my7.2 &&\n+\tgit checkout  - &&\n+\tsha1=$(git rev-parse my7 | cut -c 1-7) &&\n+\techo \"Deleted branch my7.2 (was $sha1).\" >expect &&\n+\tgit branch -d - >actual 2>&1 &&\n+\ttest_i18ncmp expect actual\n+'\n+\n test_expect_success 'test --track without .fetch entries' '\n \tgit branch --track my8 &&\n \ttest \"$(git config branch.my8.remote)\" &&\n-- \n2.7.4\n\n"},{"id":"342568","messageId":"20180323085636.GA24416@sigill.intra.peff.net","threadId":"48122","inReplyTo":"1521770966-18383-1-git-send-email-p@aaronjgreenberg.com","subject":"Re: [PATCH] branch: implement shortcut to delete last branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-23T08:56:37Z","receivedAt":"2018-03-23T08:56:44Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 23, 2018 at 02:09:25AM +0000, Aaron Greenberg wrote:\n\n> This patch gives git-branch the ability to delete the previous\n> checked-out branch using the \"-\" shortcut. This shortcut already exists\n> for git-checkout, git-merge, and git-revert. One of my common workflows\n> is to do some work on a local topic branch and push it to a remote,\n> where it gets merged in to 'master'. Then, I switch back to my local\n> master, fetch the remote master, and delete the previous topic branch.\n> \n> $ git checkout -b topic-a\n> $ # Do some work...\n> $ git commit -am \"Implement feature A\"\n> $ git push origin topic-a\n> \n> # 'origin/topic-a' gets merged into 'origin/master'\n> \n> $ git checkout master\n> $ git branch -d topic-a\n> $ # With this patch, a user could simply type\n> $ git branch -d -\n> \n> I think it's a useful shortcut for cleaning up a just-merged branch\n> (or a just switched-from branch.)\n\nI don't use \"-\" myself, but I can see how this would be useful. Do note\nthat in a discussion last year there was some hesitation about allowing\n\"-\" for destructive commands:\n\n  https://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/\n\nI don't really have a strong opinion either way.\n\nThe details in this cover letter probably should go into the commit\nmessage. The diff itself looks OK (the assumption of a 7-char\nabbreviation in the test is a little gross, but I see you're just\nfollowing existing convention in the file).\n\n-Peff\n"},{"id":"342569","messageId":"20180323090006.GA25219@sigill.intra.peff.net","threadId":"48122","inReplyTo":"20180323085636.GA24416@sigill.intra.peff.net","subject":"Re: [PATCH] branch: implement shortcut to delete last branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-23T09:00:06Z","receivedAt":"2018-03-23T09:00:13Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"[resending; I cc'd Matthieu on his address from that old thread, but it\n bounced]\n\nOn Fri, Mar 23, 2018 at 04:56:36AM -0400, Jeff King wrote:\n\n> On Fri, Mar 23, 2018 at 02:09:25AM +0000, Aaron Greenberg wrote:\n> \n> > This patch gives git-branch the ability to delete the previous\n> > checked-out branch using the \"-\" shortcut. This shortcut already exists\n> > for git-checkout, git-merge, and git-revert. One of my common workflows\n> > is to do some work on a local topic branch and push it to a remote,\n> > where it gets merged in to 'master'. Then, I switch back to my local\n> > master, fetch the remote master, and delete the previous topic branch.\n> > \n> > $ git checkout -b topic-a\n> > $ # Do some work...\n> > $ git commit -am \"Implement feature A\"\n> > $ git push origin topic-a\n> > \n> > # 'origin/topic-a' gets merged into 'origin/master'\n> > \n> > $ git checkout master\n> > $ git branch -d topic-a\n> > $ # With this patch, a user could simply type\n> > $ git branch -d -\n> > \n> > I think it's a useful shortcut for cleaning up a just-merged branch\n> > (or a just switched-from branch.)\n> \n> I don't use \"-\" myself, but I can see how this would be useful. Do note\n> that in a discussion last year there was some hesitation about allowing\n> \"-\" for destructive commands:\n> \n>   https://public-inbox.org/git/vpqh944eof7.fsf@anie.imag.fr/\n> \n> I don't really have a strong opinion either way.\n> \n> The details in this cover letter probably should go into the commit\n> message. The diff itself looks OK (the assumption of a 7-char\n> abbreviation in the test is a little gross, but I see you're just\n> following existing convention in the file).\n> \n> -Peff\n"},{"id":"342705","messageId":"1521844835-23956-2-git-send-email-p@aaronjgreenberg.com","threadId":"48122","inReplyTo":"1521844835-23956-1-git-send-email-p@aaronjgreenberg.com","subject":"[PATCH] branch: implement shortcut to delete last branch","fromName":"Aaron Greenberg","fromEmail":"p@aaronjgreenberg.com","sentAt":"2018-03-23T22:40:35Z","receivedAt":"2018-03-23T22:41:27Z","isPatch":true,"sender":{"key":"p@aaronjgreenberg.com","avatar":null},"body":"This patch gives git-branch the ability to delete the previous\nchecked-out branch using the \"-\" shortcut. This shortcut already exists\nfor git-checkout, git-merge, and git-revert. A common workflow is\n\n1. Do some work on a local topic-branch and push it to a remote.\n2. 'remote/topic-branch' gets merged in to 'remote/master'.\n3. Switch back to local master and fetch 'remote/master'.\n4. Delete previously checked-out local topic-branch.\n\n$ git checkout -b topic-a\n$ # Do some work...\n$ git commit -am \"Implement feature A\"\n$ git push origin topic-a\n\n$ git checkout master\n$ git branch -d topic-a\n$ # With this patch, a user could simply type\n$ git branch -d -\n\n\"-\" is a useful shortcut for cleaning up a just-merged branch\n(or a just switched-from branch.)\n\nSigned-off-by: Aaron Greenberg <p@aaronjgreenberg.com>\n---\n builtin/branch.c  | 3 +++\n t/t3200-branch.sh | 8 ++++++++\n 2 files changed, 11 insertions(+)\n\ndiff --git a/builtin/branch.c b/builtin/branch.c\nindex 6d0cea9..9e37078 100644\n--- a/builtin/branch.c\n+++ b/builtin/branch.c\n@@ -221,6 +221,9 @@ static int delete_branches(int argc, const char **argv, int force, int kinds,\n \t\tchar *target = NULL;\n \t\tint flags = 0;\n \n+\t\tif (!strcmp(argv[i], \"-\"))\n+\t\t\targv[i] = \"@{-1}\";\n+\n \t\tstrbuf_branchname(&bname, argv[i], allowed_interpret);\n \t\tfree(name);\n \t\tname = mkpathdup(fmt, bname.buf);\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex 6c0b7ea..78c25aa 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -776,6 +776,14 @@ test_expect_success 'deleting currently checked out branch fails' '\n \ttest_must_fail git branch -d my7\n '\n \n+test_expect_success 'test deleting last branch' '\n+\tgit checkout -b my7.1 &&\n+\tgit checkout  - &&\n+\ttest_path_is_file .git/refs/heads/my7.1 &&\n+\tgit branch -d - &&\n+\ttest_path_is_missing .git/refs/heads/my7.1\n+'\n+\n test_expect_success 'test --track without .fetch entries' '\n \tgit branch --track my8 &&\n \ttest \"$(git config branch.my8.remote)\" &&\n-- \n2.7.4\n\n"},{"id":"342706","messageId":"1521844835-23956-1-git-send-email-p@aaronjgreenberg.com","threadId":"48122","inReplyTo":"20180323085636.GA24416@sigill.intra.peff.net","subject":"[PATCH v2] branch: implement shortcut to delete last branch","fromName":"Aaron Greenberg","fromEmail":"p@aaronjgreenberg.com","sentAt":"2018-03-23T22:40:34Z","receivedAt":"2018-03-23T22:41:35Z","isPatch":true,"sender":{"key":"p@aaronjgreenberg.com","avatar":null},"body":"\nI updated the commit message to include my first email's cover letter\nand cleaned up the test.\n\nCopying Junio, since he also had good comments in the conversation you\nlinked.\n\nI can appreciate Matthieu's points on the use of \"-\" in destructive\ncommands. As of this writing, git-merge supports the \"-\" shorthand,\nwhich while not destructive, is at least _mutative_. Also,\n\"git branch -d\" is not destructive in the same way that \"rm -rf\" is\ndestructive since you can recover the branch using the reflog.\n\nOne thing to consider is that approval of this patch extends the\nimplementation of the \"-\" shorthand in a piecemeal, rather than\nconsistent, way (implementing it in a consistent way was the goal of\nthe patch set you mentioned in your previous email.) Is that okay? Or\nis it better to pick up the consistent approach where it was left?\n"},{"id":"342977","messageId":"20180326081036.GA18714@sigill.intra.peff.net","threadId":"48122","inReplyTo":"1521844835-23956-1-git-send-email-p@aaronjgreenberg.com","subject":"Re: [PATCH v2] branch: implement shortcut to delete last branch","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2018-03-26T08:10:36Z","receivedAt":"2018-03-26T08:10:43Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Mar 23, 2018 at 10:40:34PM +0000, Aaron Greenberg wrote:\n\n> I updated the commit message to include my first email's cover letter\n> and cleaned up the test.\n\nThanks. This one looks good to me.\n\n> I can appreciate Matthieu's points on the use of \"-\" in destructive\n> commands. As of this writing, git-merge supports the \"-\" shorthand,\n> which while not destructive, is at least _mutative_. Also,\n> \"git branch -d\" is not destructive in the same way that \"rm -rf\" is\n> destructive since you can recover the branch using the reflog.\n\nThere's a slight subtlety there with the reflog, because \"branch -d\"\nactually _does_ delete the reflog for the branch. By definition if\nyou've found the branch with \"-\" then it was just checked out, so you at\nleast have the old tip. But the branch's whole reflog is gone for good.\n\nThat said, I'd still be OK with it.\n\n> One thing to consider is that approval of this patch extends the\n> implementation of the \"-\" shorthand in a piecemeal, rather than\n> consistent, way (implementing it in a consistent way was the goal of\n> the patch set you mentioned in your previous email.) Is that okay? Or\n> is it better to pick up the consistent approach where it was left?\n\nI don't have a real opinion on whether it should be implemented\neverywhere or not. But IMHO it's OK to do it piecemeal for now either\nway, unless we're really sure it's time to move to respecting it\neverywhere. Because we can always convert a\npiecemeal-but-covers-everything state to centralized parsing as a\ncleanup.\n\n-Peff\n"},{"id":"342992","messageId":"86r2o7nh4i.fsf@matthieu-moy.fr","threadId":"48122","inReplyTo":"20180326081036.GA18714@sigill.intra.peff.net","subject":"Re: [PATCH v2] branch: implement shortcut to delete last branch","fromName":"","fromEmail":"git@matthieu-moy.fr","sentAt":"2018-03-26T12:41:49Z","receivedAt":"2018-03-26T12:42:15Z","isPatch":true,"sender":{"key":"git@matthieu-moy.fr","avatar":"https://avatars.githubusercontent.com/u/14709?v=4"},"body":"Thanks for Cc-ing me, and sorry for not being very responsive these\ndays :-\\.\n\nJeff King writes:\n\n> On Fri, Mar 23, 2018 at 10:40:34PM +0000, Aaron Greenberg wrote:\n>\n>> I can appreciate Matthieu's points on the use of \"-\" in destructive\n>> commands. As of this writing, git-merge supports the \"-\" shorthand,\n>> which while not destructive, is at least _mutative_. Also,\n>> \"git branch -d\" is not destructive in the same way that \"rm -rf\" is\n>> destructive since you can recover the branch using the reflog.\n>\n> There's a slight subtlety there with the reflog, because \"branch -d\"\n> actually _does_ delete the reflog for the branch. By definition if\n> you've found the branch with \"-\" then it was just checked out, so you at\n> least have the old tip. But the branch's whole reflog is gone for good.\n>\n> That said, I'd still be OK with it.\n\nI don't have objection either.\n\nAnyway, we're supporting this \"-\" shortcut in more and more commands\n(partly because it's a nice microproject, but it probably makes sense),\nso the \"consistency\" argument becomes more and more important, and is\nprobably more important than the (relative) safety of not having the\nshortcut.\n\n>> One thing to consider is that approval of this patch extends the\n>> implementation of the \"-\" shorthand in a piecemeal, rather than\n>> consistent, way (implementing it in a consistent way was the goal of\n>> the patch set you mentioned in your previous email.) Is that okay? Or\n>> is it better to pick up the consistent approach where it was left?\n>\n> I don't have a real opinion on whether it should be implemented\n> everywhere or not. But IMHO it's OK to do it piecemeal for now either\n> way, unless we're really sure it's time to move to respecting it\n> everywhere. Because we can always convert a\n> piecemeal-but-covers-everything state to centralized parsing as a\n> cleanup.\n\nNot sure whether it's already been mentionned here, but a previous\nattempt is here:\n\n  https://public-inbox.org/git/1488007487-12965-1-git-send-email-kannan.siddharth12@gmail.com/\n\nMy understanding is that the actual code is quite straightforward, but\n1) it needs a few cleanup patches to be done correctly, and 2) there are\ncorner-cases to deal with like avoiding a commit message like \"merge\nbranch '-' into 'foo'\". Regarding 2), any piecemeal implementation with\nproper tests is a step in the right direction.\n\n--\nMatthieu Moy\nhttps://matthieu-moy.fr/\n"},{"id":"343017","messageId":"xmqqvadidbo7.fsf@gitster-ct.c.googlers.com","threadId":"48122","inReplyTo":"86r2o7nh4i.fsf@matthieu-moy.fr","subject":"Re: [PATCH v2] branch: implement shortcut to delete last branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-26T16:49:44Z","receivedAt":"2018-03-26T16:49:53Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"git@matthieu-moy.fr writes:\n\n>> That said, I'd still be OK with it.\n>\n> I don't have objection either.\n\nFWIW, I do not even buy the \"destructive commands should force\nspelling things out even more\" argument in the first place.\n\n    $ git checkout somelongtopicname\n    $ work work work\n    $ git checkout master && git merge -\n    $ git branch -d -\n\nwould be a lot less error-prone than the user being forced to write\nlast step in longhand\n\n    $ git branch -d someotherlongtopicname\n\nand destroying an unrelated but similarly named branch.\n\nSo obviously I am OK with it, too.\n\nAs long as we do not regress end-user experience, that is.  For\nexample, \"git merge @{-1}\" in the above sequence would record the\nfact that the resulting commit is a merge of 'somelongtopicname',\nnot literally \"@{-1}\", in its log message.  It would be a sad\nregression if it suddenly starts to say \"Merge branch '-'\" [*1*],\nfor example.\n\n\n[Reference]\n\n*1* https://public-inbox.org/git/xmqqinnsegxb.fsf@gitster.mtv.corp.google.com/\n\n\n"}]}