On 28/06/2019 23:08, Junio C Hamano wrote:
Phillip Wood <phillip.wood123@xxxxxxxxx> 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.
I'd really like to try and hide that as much as possible from users -
it's just confusing. (though sometimes we can't)
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.
It depends what else the alias includes, if it also includes
-i/-k/-r/--signoff then it wont have been dying but will if we start
requiring --exec and they don't set that.
Best Wishes
Phillip
