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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 10, 2024, 15:44 UTC
Message-ID: <xmqqle5lxlwm.fsf@gitster.g>
In-Reply-To: <54dcd4f1-5a79-47ba-9650-7115b7170a84@gmail.com>

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

Show 9 quoted lines

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

Show 10 quoted lines

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

Show 10 quoted lines

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