Re: [PATCH 1/1] Let rebase.reschedulefailedexec only affect interactive rebases

[Vas Sudanagunta <vas@commonkarma.org>]

Hi, original reporter of the issue here. Some thoughts from a user perspective, from outside the black box of “the machinery":


It seems unintuitive that including a command to execute between each rebase merge implies an interactive rebase. It’s no more interactive than a sequence of rebase merges without intervening commands. A command failure interrupts a rebase just as a merge conflict does. If the fact that a rebase can be interrupts mid-sequence, then all rebases are interactive. 

The intuitive behavior for “rebase.reschedulefailedexec” is that it has no impact if nothing is scheduled.  It has nothing to do with interactivity. 

vas



On Jun 28, 2019, at 6:08 PM, Junio C Hamano <gitster@pobox.com> wrote:

Phillip Wood <phillip.wood123@gmail.com> writes:

>>> I wonder if users understand that '-x' is "an interctive rebase".
>>> The documentation can read both ways, and one of these may want to
>>> be clarified.
>>> 
>>> 	-x <cmd>, --exec <cmd>
>>> 	...
>>> 	This uses the --interactive machinery internally, but it can
>>> 	be run without an explicit --interactive.
>>> 
>>> Is it saying that use of interactive machinery is an impelementation
>>> detail the users should not concern themselves (in which case, the
>>> message given to "die()" above is misleading---not a new problem
>>> with this patch, though)?  Is it saying "-x" makes it plenty clear
>>> that the user wants interactive behaviour, so the users do not need
>>> to spell out --interactive in order to ask for it (in which case,
>>> "die()" message is fine, but "... internally, but ..." is
>>> misleading)?
>> 
>> Hmm. What would you think about:
>> 
>>   		die(_("--reschedule-failed-exec requires --exec or --interactive"));

I was leaning towards admitting that the use of the interactive
machinery in "-x" is not merely an implementation detail and fixing
the documentation, leaving the die() message in the patch as-is.

But ...

> I was wondering about requiring --exec with --reschedule-failed-exec
> rather than checking is_interactive() as that would be easier to
> understand.

... I find this a reasonable way to think about the issue.  The
option only matters when we are doing "--exec".  And the usual
convenience measure we'd use, i.e. with --reschedule-failed-exec we
consider that we are implicitly in --exec mode, would not work
because there is no default "command" to be executed.

> One potential problem is if someone has an alias that
> always sets --reschedule-failed-exec but does not always add --exec to
> the command line.

Such a use case would be hitting this die() already without this
topic, wouldn't it?  In which case we can say there is no "someone"
with such an alias.