{"thread":{"id":"30611","subject":"[PATCH] Do not autosquash in case of an implied interactive rebase","startedAt":"2012-05-24T13:52:55Z","lastAt":"2012-05-29T18:43:07Z","messageCount":6,"participants":["Vincent van Ravesteijn","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"192066","messageId":"1337867575-7212-1-git-send-email-vfr@lyx.org","threadId":"30611","inReplyTo":null,"subject":"[PATCH] Do not autosquash in case of an implied interactive rebase","fromName":"Vincent van Ravesteijn","fromEmail":"vfr@lyx.org","sentAt":"2012-05-24T13:52:55Z","receivedAt":"2012-05-24T13:52:55Z","isPatch":true,"sender":{"key":"vfr@lyx.org","avatar":"https://avatars.githubusercontent.com/u/687868?v=4"},"body":"The option to autosquash is only used in case of an interactive rebase.\nWhen merges are preserved, rebase uses an interactive rebase internally,\nbut in this case autosquash should still be disabled.\n---\n git-rebase.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 24a2840..9148ec2 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -167,6 +167,7 @@ run_specific_rebase () {\n \tif [ \"$interactive_rebase\" = implied ]; then\n \t\tGIT_EDITOR=:\n \t\texport GIT_EDITOR\n+\t\tautosquash=\n \tfi\n \t. git-rebase--$type\n }\n-- \n1.7.9.msysgit.0\n"},{"id":"192067","messageId":"1337867846-5336-1-git-send-email-vfr@lyx.org","threadId":"30611","inReplyTo":"1337867575-7212-1-git-send-email-vfr@lyx.org","subject":"[PATCHv2] Do not autosquash in case of an implied interactive rebase","fromName":"Vincent van Ravesteijn","fromEmail":"vfr@lyx.org","sentAt":"2012-05-24T13:57:26Z","receivedAt":"2012-05-24T13:57:26Z","isPatch":false,"sender":{"key":"vfr@lyx.org","avatar":"https://avatars.githubusercontent.com/u/687868?v=4"},"body":"The option to autosquash is only used in case of an interactive rebase.\nWhen merges are preserved, rebase uses an interactive rebase internally,\nbut in this case autosquash should still be disabled.\n\nSigned-off-by: Vincent van Ravesteijn <vfr@lyx.org>\n---\n git-rebase.sh |    1 +\n 1 files changed, 1 insertions(+), 0 deletions(-)\n\ndiff --git a/git-rebase.sh b/git-rebase.sh\nindex 24a2840..9148ec2 100755\n--- a/git-rebase.sh\n+++ b/git-rebase.sh\n@@ -167,6 +167,7 @@ run_specific_rebase () {\n \tif [ \"$interactive_rebase\" = implied ]; then\n \t\tGIT_EDITOR=:\n \t\texport GIT_EDITOR\n+\t\tautosquash=\n \tfi\n \t. git-rebase--$type\n }\n-- \n1.7.9.msysgit.0\n"},{"id":"192179","messageId":"7vehq8tajh.fsf@alter.siamese.dyndns.org","threadId":"30611","inReplyTo":"1337867846-5336-1-git-send-email-vfr@lyx.org","subject":"Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-25T17:50:26Z","receivedAt":"2012-05-25T17:50:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vincent van Ravesteijn <vfr@lyx.org> writes:\n\n> The option to autosquash is only used in case of an interactive rebase.\n> When merges are preserved, rebase uses an interactive rebase internally,\n> but in this case autosquash should still be disabled.\n>\n> Signed-off-by: Vincent van Ravesteijn <vfr@lyx.org>\n\nHrm, what if the end user said \"git rebase --autosquash -p\" explicitly\nfrom the command line?\n\nI _think_ you are addressing the case where rebase.autosquash is set to\ntrue in the configuration.  The handling of that variable that was added\nin dd1e5b3 (add configuration variable for --autosquash option of\ninteractive rebase, 2010-07-14) is not correct.  The configuration should\nkick in only when the end user did not explicitly give either --autosquash\nor --no-autosquash, so the variable $autosquash is logically a tristate\n(unknown, set to yes, set to no), initialized to \"unknown\", set to either\nvalue when --[no-]autosquash option is seen, and fall back to the\nconfigured value _only_ after the option parsing loop exits and the\nvariable is still set to \"unknown\".  In other words, the current:\n\n\tautosquash=$(ask rebase.autosquash or default to no)\n        for each option:\n        \tif option == --autosquash: autosquash=yes\n                if option == --no-autosquash: autosquash=no\n\nis wrong and I think this patch is trying to sweep that real problem under\nthe rug.  Shouldn't the fix be more like the following, learning from what\nwas done to the \"implied interactive rebase\" case, to fix the option\nparsing loop?\n\n\tautosquash=unknown\n        interactive_rebase=unknown\n        for each option:\n        \tif option == --autosquash: autosquash=yes\n                if option == --no-autosquash: autosquash=no\n                if option == --preserve-merges:\n                \tpreserve_merges=yes\n                        if interactive_rebase is unknown:\n\t\t\t\tinteractive_rebase=implied\n\t\t\tif autosquash is unknown:\n                        \tautosquash=no\n\t\t... other options ...\n\tif autosquash is unknown:\n        \tautosquash=$(ask rebase.autosquash or default to no)\n"},{"id":"192316","messageId":"4FC3428F.4060202@lyx.org","threadId":"30611","inReplyTo":"7vehq8tajh.fsf@alter.siamese.dyndns.org","subject":"Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase","fromName":"Vincent van Ravesteijn","fromEmail":"vfr@lyx.org","sentAt":"2012-05-28T09:17:03Z","receivedAt":"2012-05-28T09:17:03Z","isPatch":false,"sender":{"key":"vfr@lyx.org","avatar":"https://avatars.githubusercontent.com/u/687868?v=4"},"body":"Op 25-5-2012 19:50, Junio C Hamano schreef:\n> Vincent van Ravesteijn<vfr@lyx.org>  writes:\n>\n>> The option to autosquash is only used in case of an interactive rebase.\n>> When merges are preserved, rebase uses an interactive rebase internally,\n>> but in this case autosquash should still be disabled.\n>>\n>> Signed-off-by: Vincent van Ravesteijn<vfr@lyx.org>\n> Hrm, what if the end user said \"git rebase --autosquash -p\" explicitly\n> from the command line?\n\nThe option \"--autosquash\" is designed to have no effect whenever \n\"--interactive\" is not supplied.\n\nThis is what you also argue in another thread \n(http://permalink.gmane.org/gmane.comp.version-control.git/198579):\n\n\\\"And that \"I want rebase with --autosquash but I am not going to do any \nother editing\" is where my suggestion to use \"--no-edit\" comes from.\\\"\n\nOr did you mean that you want to imply \"--interactive\" when \n\"--autosquash\" is explicitly given ?\n\nVincent\n"},{"id":"192424","messageId":"7vvcje4ypl.fsf@alter.siamese.dyndns.org","threadId":"30611","inReplyTo":"4FC3428F.4060202@lyx.org","subject":"Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-29T18:41:10Z","receivedAt":"2012-05-29T18:41:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vincent van Ravesteijn <vfr@lyx.org> writes:\n\n> Op 25-5-2012 19:50, Junio C Hamano schreef:\n>> Vincent van Ravesteijn<vfr@lyx.org>  writes:\n>>\n>>> The option to autosquash is only used in case of an interactive rebase.\n>>> When merges are preserved, rebase uses an interactive rebase internally,\n>>> but in this case autosquash should still be disabled.\n>>>\n>>> Signed-off-by: Vincent van Ravesteijn<vfr@lyx.org>\n>> Hrm, what if the end user said \"git rebase --autosquash -p\" explicitly\n>> from the command line?\n>\n> The option \"--autosquash\" is designed to have no effect whenever\n> \"--interactive\" is not supplied.\n\nAhh, Ok.  I somehow thought \"rebase --autosquash\" implied \"-i\".\n"},{"id":"192425","messageId":"7vr4u24ymc.fsf@alter.siamese.dyndns.org","threadId":"30611","inReplyTo":"4FC3428F.4060202@lyx.org","subject":"Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2012-05-29T18:43:07Z","receivedAt":"2012-05-29T18:43:07Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Vincent van Ravesteijn <vfr@lyx.org> writes:\n\n> Or did you mean that you want to imply \"--interactive\" when\n> \"--autosquash\" is explicitly given ?\n\nCome to think of it, it might have been a saner UI design when\nf59baa5 (rebase -i --autosquash: auto-squash commits, 2009-12-08)\nadded this option, but if we want to go that route, we do need to\naddress the concerns I raised in my response.\n\nBut as things stand, i.e. with --autosquash that does not imply -i,\nyour patch is good.\n\nThanks.\n"}]}