git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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

From
PWPhillip Wood <phillip.wood123@gmail.com>
Date
Jun 30, 2019, 10:03 UTC
Message-ID
<70648c5b-00ff-d7aa-f1f7-8bb7bf663829@gmail.com>
In-Reply-To
<xmqq4l498irq.fsf@gitster-ct.c.googlers.com>
On 28/06/2019 23:08, Junio C Hamano wrote:
Show 27 quoted lines
> 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.

I'd really like to try and hide that as much as possible from users - it's just confusing. (though sometimes we can't)

Show 11 quoted lines
> 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.
 >
Show 7 quoted lines
>> 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
Previous: Vas SudanaguntaNext: Johannes Schindelin
Message 8 of 13 in “Make rebase.reschedulefailedexec less overzealous”
  1. 0/1 Make rebase.reschedulefailedexec less overzealousJohannes Schindelin via GitGitGadget, Jun 27, 2019
  2. 1/1 Let rebase.reschedulefailedexec only affect interactive rebasesJohannes Schindelin via GitGitGadget, Jun 27, 2019
  3. Junio C HamanoJun 27, 2019
  4. Johannes SchindelinJun 28, 2019
  5. Phillip WoodJun 28, 2019
  6. Junio C HamanoJun 28, 2019
  7. Vas SudanaguntaJun 28, 2019
  8. Phillip WoodJun 30, 2019
  9. Johannes SchindelinJul 1, 2019
  10. Johannes SchindelinJul 1, 2019
  11. 0/1 Make rebase.reschedulefailedexec less overzealousJohannes Schindelin via GitGitGadget, Jul 1, 2019
  12. 1/1 rebase --am: ignore rebase.reschedulefailedexecJohannes Schindelin via GitGitGadget, Jul 1, 2019
  13. Johannes SchindelinJul 1, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.