Re: [PATCHv2] Do not autosquash in case of an implied interactive rebase

Vincent van Ravesteijn [off-list ref] 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 <redacted>

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)

Op 25-5-2012 19:50, Junio C Hamano schreef:

Vincent van Ravesteijn[off-list ref]  writes:

quoted

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<redacted>

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