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

From: Junio C Hamano <gitster@pobox.com>
Date: Jun 28, 2019, 22:08 UTC
Message-ID: <xmqq4l498irq.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <e332eb5f-0818-763c-9de0-889644446a35@gmail.com>

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

Show 21 quoted lines

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