{"thread":{"id":"23018","subject":"[PATCH] rebase--interactive: don't enforce valid branch","startedAt":"2010-03-15T04:48:22Z","lastAt":"2010-03-15T17:14:28Z","messageCount":8,"participants":["Dave Olszewski","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"136790","messageId":"1268628502-29696-1-git-send-email-cxreg@pobox.com","threadId":"23018","inReplyTo":null,"subject":"[PATCH] rebase--interactive: don't enforce valid branch","fromName":"Dave Olszewski","fromEmail":"cxreg@pobox.com","sentAt":"2010-03-15T04:48:22Z","receivedAt":"2010-03-15T04:48:22Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"git rebase allows you to specify a non-branch commit-ish as the \"branch\"\nargument, which leaves HEAD detached when it's finished. This is\noccasionally useful, and this patch brings the same functionality to git\nrebase ---interactive.\n\nSigned-off-by: Dave Olszewski \n---\n git-rebase--interactive.sh | 2 --\n t/t3404-rebase-interactive.sh | 7 +++++++\n 2 files changed, 7 insertions(+), 2 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 3e4fd14..d047dcb 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -783,8 +783,6 @@ first and then run 'git rebase --continue' again.\"\n \n \t\tif test ! -z \"$1\"\n \t\tthen\n-\t\t\toutput git show-ref --verify --quiet \"refs/heads/$1\" ||\n-\t\t\t\tdie \"Invalid branchname: $1\"\n \t\t\toutput git checkout \"$1\" ||\n \t\t\t\tdie \"Could not checkout $1\"\n \t\tfi\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 4e35137..32ffa15 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -553,4 +553,11 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n \n+test_expect_success 'rebase while detaching HEAD' '\n+\tgrandparent=$(git rev-parse HEAD~2) &&\n+\ttest_tick &&\n+\tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n+\ttest $grandparent = $(git rev-parse HEAD~2)\n+'\n+\n test_done\n-- \n1.7.0.2.213.gd6898.dirty\n"},{"id":"136791","messageId":"7vsk82i2kd.fsf@alter.siamese.dyndns.org","threadId":"23018","inReplyTo":"1268628502-29696-1-git-send-email-cxreg@pobox.com","subject":"Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-15T05:20:02Z","receivedAt":"2010-03-15T05:20:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Olszewski writes:\n\n> git rebase allows you to specify a non-branch commit-ish as the \"branch\"\n> argument, which leaves HEAD detached when it's finished. This is\n> occasionally useful, and this patch brings the same functionality to git\n> rebase ---interactive.\n\nThree dashes?\n\n> +test_expect_success 'rebase while detaching HEAD' '\n> +\tgrandparent=$(git rev-parse HEAD~2) &&\n> +\ttest_tick &&\n> +\tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n\nWhat's the point of saying this? You could instead say:\n\n\tgit rebase -i HEAD~2\n\nno?\n"},{"id":"136792","messageId":"alpine.DEB.2.00.1003142227100.796@narbuckle.genericorp.net","threadId":"23018","inReplyTo":"7vsk82i2kd.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Dave Olszewski","fromEmail":"cxreg@pobox.com","sentAt":"2010-03-15T05:28:43Z","receivedAt":"2010-03-15T05:28:43Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Sun, 14 Mar 2010, Junio C Hamano wrote:\n\n> Dave Olszewski writes:\n>\n>> git rebase allows you to specify a non-branch commit-ish as the \"branch\"\n>> argument, which leaves HEAD detached when it's finished. This is\n>> occasionally useful, and this patch brings the same functionality to git\n>> rebase ---interactive.\n>\n> Three dashes?\n\nOops, good catch\n\n>> +test_expect_success 'rebase while detaching HEAD' '\n>> +\tgrandparent=$(git rev-parse HEAD~2) &&\n>> +\ttest_tick &&\n>> +\tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n>\n> What's the point of saying this? You could instead say:\n>\n> \tgit rebase -i HEAD~2\n>\n> no?\n\nThere's already a test for rebasing on a previously detached HEAD. The\nform \"git rebase -i HEAD~2\" specifies a non-branch upstream, but doesn't\ntake the branch argument which is the point of the change.\n"},{"id":"136793","messageId":"7vvdcygmz8.fsf@alter.siamese.dyndns.org","threadId":"23018","inReplyTo":"alpine.DEB.2.00.1003142227100.796@narbuckle.genericorp.net","subject":"Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-15T05:42:03Z","receivedAt":"2010-03-15T05:42:03Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Olszewski writes:\n\n>>> +test_expect_success 'rebase while detaching HEAD' '\n>>> +\tgrandparent=$(git rev-parse HEAD~2) &&\n>>> +\ttest_tick &&\n>>> +\tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n>>\n>> What's the point of saying this? You could instead say:\n>>\n>> \tgit rebase -i HEAD~2\n>>\n>> no?\n>\n> There's already a test for rebasing on a previously detached HEAD. The\n> form \"git rebase -i HEAD~2\" specifies a non-branch upstream, but doesn't\n> take the branch argument which is the point of the change.\n\nWhat I meant was that if you prefer to work on a detached HEAD (and I\nsometimes do), then your HEAD would likely to be detached already when you\nrun rebase. IOW, I would expect that \n\n\tgit checkout HEAD^0\n ... perhaps do something, perhaps do nothing, here ...\n git rebase -i HEAD~2\n\nwould be a lot more natural thing to do, and in that case you do not need\nto say HEAD^0 there.\n"},{"id":"136794","messageId":"alpine.DEB.2.00.1003142242510.796@narbuckle.genericorp.net","threadId":"23018","inReplyTo":"7vvdcygmz8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Dave Olszewski","fromEmail":"cxreg@pobox.com","sentAt":"2010-03-15T05:46:49Z","receivedAt":"2010-03-15T05:46:49Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Sun, 14 Mar 2010, Junio C Hamano wrote:\n\n>> There's already a test for rebasing on a previously detached HEAD. The\n>> form \"git rebase -i HEAD~2\" specifies a non-branch upstream, but doesn't\n>> take the branch argument which is the point of the change.\n>\n> What I meant was that if you prefer to work on a detached HEAD (and I\n> sometimes do), then your HEAD would likely to be detached already when you\n> run rebase. IOW, I would expect that\n>\n> \tgit checkout HEAD^0\n> ... perhaps do something, perhaps do nothing, here ...\n> git rebase -i HEAD~2\n>\n> would be a lot more natural thing to do, and in that case you do not need\n> to say HEAD^0 there.\n\nThat functionality already works. It's certainly possible to do it as\nyou describe, but why require the extra step? git-rebase already\nsupports detaching as part of its behavior, but rebase -i does not.\nIt may seem nitpicky, but it's asymmetric and occasionally it's handy\nsyntax.\n"},{"id":"136795","messageId":"7vd3z6f6wt.fsf@alter.siamese.dyndns.org","threadId":"23018","inReplyTo":"7vvdcygmz8.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-15T06:14:26Z","receivedAt":"2010-03-15T06:14:26Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> Dave Olszewski writes:\n>\n>>>> +test_expect_success 'rebase while detaching HEAD' '\n>>>> +\tgrandparent=$(git rev-parse HEAD~2) &&\n>>>> +\ttest_tick &&\n>>>> +\tFAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n>>>\n>>> What's the point of saying this? You could instead say:\n>>>\n>>> \tgit rebase -i HEAD~2\n>>>\n>>> no?\n>> ...\n\nAhh, Ok, the point is that when we start this sequence we are on a branch,\nand then you want to end up on a detached HEAD that points at the result\nof the branch.\n\nI'll queue it in 'pu', but with a little tweak to the test to make it\nclear what is going on, perhaps like this.\n\n test_expect_success 'rebase while detaching HEAD' '\n git symbolic-ref HEAD &&\n grandparent=$(git rev-parse HEAD~2) &&\n test_tick &&\n FAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n test $grandparent = $(git rev-parse HEAD~2) &&\n test_must_fail git symbolic-ref HEAD\n '\n\nWe may need to document this behaviour, by the way, if we make it official\nthat the extra \"branch to be rewritten\" parameter can be a non-branch.\nTwo points are that you can give arbitrary commit, and that you will end\nup with a detached HEAD that points at the result if you did so.\n\nAlso I did't followed the code, but does it behave sanely when you say\n\"rebase --abort\"?\n"},{"id":"136817","messageId":"alpine.DEB.2.00.1003150132060.4362@narbuckle.genericorp.net","threadId":"23018","inReplyTo":"7vd3z6f6wt.fsf@alter.siamese.dyndns.org","subject":"Re: Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Dave Olszewski","fromEmail":"cxreg@pobox.com","sentAt":"2010-03-15T08:41:30Z","receivedAt":"2010-03-15T08:41:30Z","isPatch":true,"sender":{"key":"cxreg@pobox.com","avatar":"https://avatars.githubusercontent.com/u/55474?v=4"},"body":"On Sun, 14 Mar 2010, Junio C Hamano wrote:\n\n> Ahh, Ok, the point is that when we start this sequence we are on a branch,\n> and then you want to end up on a detached HEAD that points at the result\n> of the branch.\n\nYep, you got it\n\n\n> I'll queue it in 'pu', but with a little tweak to the test to make it\n> clear what is going on, perhaps like this.\n>\n> test_expect_success 'rebase while detaching HEAD' '\n> git symbolic-ref HEAD &&\n> grandparent=$(git rev-parse HEAD~2) &&\n> test_tick &&\n> FAKE_LINES=\"2 1\" git rebase -i HEAD~2 HEAD^0 &&\n> test $grandparent = $(git rev-parse HEAD~2) &&\n> test_must_fail git symbolic-ref HEAD\n> '\n\nGood idea. Thanks.\n\n\n> We may need to document this behaviour, by the way, if we make it official\n> that the extra \"branch to be rewritten\" parameter can be a non-branch.\n> Two points are that you can give arbitrary commit, and that you will end\n> up with a detached HEAD that points at the result if you did so.\n>\n> Also I did't followed the code, but does it behave sanely when you say\n> \"rebase --abort\"?\n\nGood question. It turns out that both rebase and rebase -i will end\nup on the commit specified by , whether it's a branch or not.\nThat might be the expected and desired behavior, though:\n\n [Starting on branch A]\n git rebase origin/B B\n git rebase --abort\n [HEAD is now a symref to B]\n\n [Starting on branch A]\n git rebase origin/B B^0\n git rebase --abort\n [HEAD is now detached at B^0]\n"},{"id":"136849","messageId":"7vbpepse17.fsf@alter.siamese.dyndns.org","threadId":"23018","inReplyTo":"alpine.DEB.2.00.1003150132060.4362@narbuckle.genericorp.net","subject":"Re: [PATCH] rebase--interactive: don't enforce valid branch","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-03-15T17:14:28Z","receivedAt":"2010-03-15T17:14:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Dave Olszewski writes:\n\n>> Also I did't follow the code, but does it behave sanely when you say\n>> \"rebase --abort\"?\n>\n> Good question. It turns out that both rebase and rebase -i will end\n> up on the commit specified by , whether it's a branch or not.\n> That might be the expected and desired behavior, though:\n>\n> [Starting on branch A]\n> git rebase origin/B B\n> git rebase --abort\n> [HEAD is now a symref to B]\n>\n> [Starting on branch A]\n> git rebase origin/B B^0\n> git rebase --abort\n> [HEAD is now detached at B^0]\n\nYup, that is what I would call \"behave sanely\". Thanks for clarification.\n"}]}