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

From: Rubén Justo <rjusto@gmail.com>
Date: Apr 9, 2024, 23:38 UTC
Message-ID: <54dcd4f1-5a79-47ba-9650-7115b7170a84@gmail.com>
In-Reply-To: <xmqq4jcb495u.fsf@gitster.g>

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.

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.

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

But, yes, while we're here we can also fix:

	$ GIT_EDITOR=falso git commit -a
	hint: Waiting for your editor to close the file... error: cannot run falso: No such file or directory
	error: unable to start editor 'falso'
	Please supply the message using either -m or -F option.