{"thread":{"id":"22264","subject":"[PATCH/RFC] Allow empty commits during rebase -i","startedAt":"2010-01-18T01:12:01Z","lastAt":"2010-01-18T10:01:50Z","messageCount":5,"participants":["Pete Harlan","Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"131998","messageId":"4B53B561.0@pcharlan.com","threadId":"22264","inReplyTo":null,"subject":"[PATCH/RFC] Allow empty commits during rebase -i","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2010-01-18T01:12:01Z","receivedAt":"2010-01-18T01:12:01Z","isPatch":true,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"If you squash two commits into a previous commit, where the first\nsquash reverts the previous commit and the second redoes the change\ncorrectly, rebase -i would fail during the first squash because it\ngenerates an empty commit. This patch allows the rebase to succeed.\n\nThis also introduces the possibility that you might accidentally\ncreate an empty commit with a squash, but I expect that will happen\nless often than the scenario this is intended to address.\n\nSigned-off-by: Pete Harlan \n---\n\nThis arose for me recently; I used \"git revert\" to undo a commit\nseveral changes back, and then reworked and committed anew. The first\ncommit that I was redoing had a thorough commit message, while my new\ncommit had a message like \"do it right this time\". I squashed the\nthree commits into one with rebase -i, but git choked on the\nintermediate empty commit.\n\nI could have simply removed the first two commits I was squashing (the\ninitial version and its revert), but then would have lost the\nwell-written commit message that went with the first version.\n\nI imagine an ideal version of this fix would make it so the use case I\npresented here would work, but rebase -i would still prevent\nintroducing a new empty commit, or at least warn when it was\nintroducing one. In the absence of that ideal fix, I think this\nbehavior is better than failing to handle this case.\n\n git-rebase--interactive.sh | 2 +-\n t/t3404-rebase-interactive.sh | 9 +++++++++\n 2 files changed, 10 insertions(+), 1 deletions(-)\n\ndiff --git a/git-rebase--interactive.sh b/git-rebase--interactive.sh\nindex 1560e84..81db5cf 100755\n--- a/git-rebase--interactive.sh\n+++ b/git-rebase--interactive.sh\n@@ -403,7 +403,7 @@ do_next () {\n \t\t\tGIT_AUTHOR_NAME=\"$GIT_AUTHOR_NAME\" \\\n \t\t\tGIT_AUTHOR_EMAIL=\"$GIT_AUTHOR_EMAIL\" \\\n \t\t\tGIT_AUTHOR_DATE=\"$GIT_AUTHOR_DATE\" \\\n-\t\t\t$USE_OUTPUT git commit --no-verify \\\n+\t\t\t$USE_OUTPUT git commit --no-verify --allow-empty \\\n \t\t\t\t$MSG_OPT \"$EDIT_OR_FILE\" || failed=t\n \t\tfi\n \t\tif test $failed = t\ndiff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh\nindex 3a37793..5eb9f7e 100755\n--- a/t/t3404-rebase-interactive.sh\n+++ b/t/t3404-rebase-interactive.sh\n@@ -484,4 +484,13 @@ test_expect_success 'reword' '\n \tgit show HEAD~2 | grep \"C changed\"\n '\n\n+test_expect_success 'squash including empty' '\n+\ttest_commit Initial_emptysquash emptysquash abc &&\n+\ttest_commit first_mod emptysquash abd &&\n+\ttest_tick &&\n+\tgit revert --no-edit HEAD &&\n+\ttest_commit second_mod emptysquash abe &&\n+\tFAKE_LINES=\"1 squash 2 squash 3\" git rebase -i Initial_emptysquash\n+'\n+\n test_done\n-- \n1.6.6.196.g1f735\n"},{"id":"132000","messageId":"7vljfww686.fsf@alter.siamese.dyndns.org","threadId":"22264","inReplyTo":"4B53B561.0@pcharlan.com","subject":"Re: [PATCH/RFC] Allow empty commits during rebase -i","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-18T01:29:29Z","receivedAt":"2010-01-18T01:29:29Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Harlan writes:\n\n> I imagine an ideal version of this fix would make it so the use case I\n> presented here would work, but rebase -i would still prevent\n> introducing a new empty commit, or at least warn when it was\n> introducing one. In the absence of that ideal fix, I think this\n> behavior is better than failing to handle this case.\n\nSorry, I actually tend to think that in the absense of that fix, your\nversion introduces risky behaviour that only a corner-case use case\nbenefits, and pros-and-cons doesn't look attractive enough.\n\nWhy not do something like:\n\n pick X a crap tree with a good message\n pick Y revert X\n pick Z a good tree with a crap message\n\n-->\n\n # drop X\n # drop Y\n edit Z\n\nand then run \"git commit --amend -C X\" when it is Z's turn to be\nprocessed?\n"},{"id":"132004","messageId":"4B53C355.1010109@pcharlan.com","threadId":"22264","inReplyTo":"7vljfww686.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCH/RFC] Allow empty commits during rebase -i","fromName":"Pete Harlan","fromEmail":"pgit@pcharlan.com","sentAt":"2010-01-18T02:11:33Z","receivedAt":"2010-01-18T02:11:33Z","isPatch":true,"sender":{"key":"pgit@pcharlan.com","avatar":null},"body":"On 01/17/2010 05:29 PM, Junio C Hamano wrote:\n> Pete Harlan writes:\n> \n>> I imagine an ideal version of this fix would make it so the use case I\n>> presented here would work, but rebase -i would still prevent\n>> introducing a new empty commit, or at least warn when it was\n>> introducing one. In the absence of that ideal fix, I think this\n>> behavior is better than failing to handle this case.\n> \n> Sorry, I actually tend to think that in the absense of that fix, your\n> version introduces risky behaviour that only a corner-case use case\n> benefits, and pros-and-cons doesn't look attractive enough.\n> \n> Why not do something like:\n> \n> pick X a crap tree with a good message\n> pick Y revert X\n> pick Z a good tree with a crap message\n> \n> -->\n> \n> # drop X\n> # drop Y\n> edit Z\n> \n> and then run \"git commit --amend -C X\" when it is Z's turn to be\n> processed?\n\nThat is another way to accomplish the same thing, but doesn't prevent\nthe current behavior from being confusing.\n\nPart of the problem is that with the current behavior the user is sent\nto the command line with:\n\n # Not currently on any branch.\n nothing to commit (working directory clean)\n\n Could not apply a0b17c5... Revert \"Crap tree good message\"\n\nwith HEAD pointed to X^. Unsure of how to proceed from here, I\n--aborted the rebase and copy/pasted the commit message I wanted and\nresolved to track this down and fix it when I got a chance.\n\nAs it happens, \"git rebase --continue\" does exactly what I would have\nwanted to happen, including putting me in an editor with all three\ncommit messages and succeeding when I exit the editor. But without a\nbetter message from git I don't expect a user to discover that. And,\nwhen rebase can continue just by being told so it would be nice if it\ndidn't require that user intervention.\n\nIf the introduction of empty commits that the user has asked for\n(perhaps inadvertently) is considered too undesirable, then perhaps my\nfix is too simple. I'll think about how to do something more\nsophisticated.\n\nThanks for your feedback,\n\n--Pete\n"},{"id":"132008","messageId":"7veilow1hc.fsf@alter.siamese.dyndns.org","threadId":"22264","inReplyTo":"4B53C355.1010109@pcharlan.com","subject":"Re: [PATCH/RFC] Allow empty commits during rebase -i","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-18T03:11:59Z","receivedAt":"2010-01-18T03:11:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pete Harlan writes:\n\n> ..., \"git rebase --continue\" does exactly what I would have\n> wanted to happen, including putting me in an editor with all three\n> commit messages and succeeding when I exit the editor. But without a\n> better message from git I don't expect a user to discover that.\n\nThere seems to be an idea for a good improvement ;-) CC'ing Michael as he\nhas been most active in this area for the past few weeks.\n"},{"id":"132026","messageId":"alpine.DEB.1.00.1001181059510.4985@pacific.mpi-cbg.de","threadId":"22264","inReplyTo":"4B53C355.1010109@pcharlan.com","subject":"Re: [PATCH/RFC] Allow empty commits during rebase -i","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2010-01-18T10:01:50Z","receivedAt":"2010-01-18T10:01:50Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 17 Jan 2010, Pete Harlan wrote:\n\n> If the introduction of empty commits that the user has asked for \n> (perhaps inadvertently) is considered too undesirable, then perhaps my \n> fix is too simple. I'll think about how to do something more \n> sophisticated.\n\nHow about something less sophisticated instead? Namely, check for the \ncondition that nothing was changed, and tell the user that the commit \nblablabla seems to introduce changes that are already present in HEAD. \nMaybe even mention that this may be due to the commit being applied \nalready and saying that --continue is safe in that case, but please check.\n\nHmm?\n\nCiao,\nDscho\n"}]}