# [PATCH] Do not autosquash in case of an implied interactive rebase 6 messages from 2012-05-24 to 2012-05-29. Participants: Vincent van Ravesteijn, Junio C Hamano. Thread: https://gitlist.dev/t/30611 ## Vincent van Ravesteijn, 2012-05-24 13:52 Subject: [PATCH] Do not autosquash in case of an implied interactive rebase Message-ID: <1337867575-7212-1-git-send-email-vfr@lyx.org> URL: https://gitlist.dev/e/1337867575-7212-1-git-send-email-vfr%40lyx.org ``` The option to autosquash is only used in case of an interactive rebase. When merges are preserved, rebase uses an interactive rebase internally, but in this case autosquash should still be disabled. --- git-rebase.sh | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 24a2840..9148ec2 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -167,6 +167,7 @@ run_specific_rebase () { if [ "$interactive_rebase" = implied ]; then GIT_EDITOR=: export GIT_EDITOR + autosquash= fi . git-rebase--$type } -- 1.7.9.msysgit.0 ``` ## Vincent van Ravesteijn, 2012-05-24 13:57 Subject: [PATCHv2] Do not autosquash in case of an implied interactive rebase Message-ID: <1337867846-5336-1-git-send-email-vfr@lyx.org> URL: https://gitlist.dev/e/1337867846-5336-1-git-send-email-vfr%40lyx.org In-Reply-To: <1337867575-7212-1-git-send-email-vfr@lyx.org> ``` The option to autosquash is only used in case of an interactive rebase. When merges are preserved, rebase uses an interactive rebase internally, but in this case autosquash should still be disabled. Signed-off-by: Vincent van Ravesteijn --- git-rebase.sh | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/git-rebase.sh b/git-rebase.sh index 24a2840..9148ec2 100755 --- a/git-rebase.sh +++ b/git-rebase.sh @@ -167,6 +167,7 @@ run_specific_rebase () { if [ "$interactive_rebase" = implied ]; then GIT_EDITOR=: export GIT_EDITOR + autosquash= fi . git-rebase--$type } -- 1.7.9.msysgit.0 ``` ## Junio C Hamano, 2012-05-25 17:50 Subject: Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase Message-ID: <7vehq8tajh.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vehq8tajh.fsf%40alter.siamese.dyndns.org In-Reply-To: <1337867846-5336-1-git-send-email-vfr@lyx.org> ``` Vincent van Ravesteijn writes: > The option to autosquash is only used in case of an interactive rebase. > When merges are preserved, rebase uses an interactive rebase internally, > but in this case autosquash should still be disabled. > > Signed-off-by: Vincent van Ravesteijn Hrm, what if the end user said "git rebase --autosquash -p" explicitly from the command line? I _think_ you are addressing the case where rebase.autosquash is set to true in the configuration. The handling of that variable that was added in dd1e5b3 (add configuration variable for --autosquash option of interactive rebase, 2010-07-14) is not correct. The configuration should kick in only when the end user did not explicitly give either --autosquash or --no-autosquash, so the variable $autosquash is logically a tristate (unknown, set to yes, set to no), initialized to "unknown", set to either value when --[no-]autosquash option is seen, and fall back to the configured value _only_ after the option parsing loop exits and the variable is still set to "unknown". In other words, the current: autosquash=$(ask rebase.autosquash or default to no) for each option: if option == --autosquash: autosquash=yes if option == --no-autosquash: autosquash=no is wrong and I think this patch is trying to sweep that real problem under the rug. Shouldn't the fix be more like the following, learning from what was done to the "implied interactive rebase" case, to fix the option parsing loop? autosquash=unknown interactive_rebase=unknown for each option: if option == --autosquash: autosquash=yes if option == --no-autosquash: autosquash=no if option == --preserve-merges: preserve_merges=yes if interactive_rebase is unknown: interactive_rebase=implied if autosquash is unknown: autosquash=no ... other options ... if autosquash is unknown: autosquash=$(ask rebase.autosquash or default to no) ``` ## Vincent van Ravesteijn, 2012-05-28 09:17 Subject: Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase Message-ID: <4FC3428F.4060202@lyx.org> URL: https://gitlist.dev/e/4FC3428F.4060202%40lyx.org In-Reply-To: <7vehq8tajh.fsf@alter.siamese.dyndns.org> ``` Op 25-5-2012 19:50, Junio C Hamano schreef: > Vincent van Ravesteijn writes: > >> The option to autosquash is only used in case of an interactive rebase. >> When merges are preserved, rebase uses an interactive rebase internally, >> but in this case autosquash should still be disabled. >> >> Signed-off-by: Vincent van Ravesteijn > Hrm, what if the end user said "git rebase --autosquash -p" explicitly > from the command line? The option "--autosquash" is designed to have no effect whenever "--interactive" is not supplied. This is what you also argue in another thread (http://permalink.gmane.org/gmane.comp.version-control.git/198579): \"And that "I want rebase with --autosquash but I am not going to do any other editing" is where my suggestion to use "--no-edit" comes from.\" Or did you mean that you want to imply "--interactive" when "--autosquash" is explicitly given ? Vincent ``` ## Junio C Hamano, 2012-05-29 18:41 Subject: Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase Message-ID: <7vvcje4ypl.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vvcje4ypl.fsf%40alter.siamese.dyndns.org In-Reply-To: <4FC3428F.4060202@lyx.org> ``` Vincent van Ravesteijn writes: > Op 25-5-2012 19:50, Junio C Hamano schreef: >> Vincent van Ravesteijn writes: >> >>> The option to autosquash is only used in case of an interactive rebase. >>> When merges are preserved, rebase uses an interactive rebase internally, >>> but in this case autosquash should still be disabled. >>> >>> Signed-off-by: Vincent van Ravesteijn >> Hrm, what if the end user said "git rebase --autosquash -p" explicitly >> from the command line? > > The option "--autosquash" is designed to have no effect whenever > "--interactive" is not supplied. Ahh, Ok. I somehow thought "rebase --autosquash" implied "-i". ``` ## Junio C Hamano, 2012-05-29 18:43 Subject: Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase Message-ID: <7vr4u24ymc.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vr4u24ymc.fsf%40alter.siamese.dyndns.org In-Reply-To: <4FC3428F.4060202@lyx.org> ``` Vincent van Ravesteijn writes: > Or did you mean that you want to imply "--interactive" when > "--autosquash" is explicitly given ? Come to think of it, it might have been a saner UI design when f59baa5 (rebase -i --autosquash: auto-squash commits, 2009-12-08) added this option, but if we want to go that route, we do need to address the concerns I raised in my response. But as things stand, i.e. with --autosquash that does not imply -i, your patch is good. Thanks. ```