Re: [PATCH v2] launch_editor: waiting message on error

[Junio C Hamano <gitster@pobox.com>]

Rubén Justo <rjusto@gmail.com> writes:

> On Mon, Apr 08, 2024 at 06:27:57PM -0700, Junio C Hamano wrote:
>
>> I wonder if we can just call term_clear_line()
>> regardless of the value of ret.  Either case, the waiting is already
>> over and in the error case, we show another message after it.
>
> My concern is that perhaps term_clear_line() might clear some useful
> information for the user.  Although I am not sure that this concern is
> sensible.

It happens ONLY when the error message the editor itself emits
(which comes later on the same line as "We are waiting for your
editor...") without terminating newline itself.  Otherwise, we'd
have

    We are waiting ... THE EDITOR SAYS I FAILED
    _

on the screen, and the cursor is at the _ position.  term_clear_line()
would not clear anything.

> Stepping back a bit, how painful it would be to drop the
> term_clear_line() and start using advice_if_enabled() here?
>
> This is what I'm thinking about now.
>
> 	$ GIT_EDITOR=false git commit -a
> 	hint: A-good-explanation-to-say-we-run-'editor'
> 	hint: Disable this message with "git config advice.waitingForEditor false"
> 	error: There was a problem with the editor 'false'.
> 	Please supply the message using either -m or -F option.

I do not think the problem is in the case where the editor
immediately exits with an error.  It is when the editor opens
elsewhere (or more likely, opens another tab to let you edit in an
existing GUI editor session, but the editor's window is buried under
other windows) and your "git commit" will stay silently without
giving you back a terminal prompt, leaving you wondering why it is
taking so much time.

So I am not sure if the advice mechanism is a good fit.

>> There is another error message when we fail to start the editor.
>> Doesn't that codepath have the same problem?
>
> Of course.
>
> My itch is:
>
> 	$ GIT_EDITOR=false git commit -a
> 	hint: Waiting for your editor to close the file... error: There was a problem with the editor 'false'.
> 	Please supply the message using either -m or -F option.

I do not think we want to encourage "-m" when the end user did not
say so.  Instead we should let them fix their editor to keep them
more productive.

Thanks.