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

From: Rubén Justo <rjusto@gmail.com>
Date: Apr 12, 2024, 18:24 UTC
Message-ID: <701c23e2-ed88-4ad8-95b6-1ce73e671709@gmail.com>
In-Reply-To: <xmqqbk6emql1.fsf@gitster.g>

On Fri, Apr 12, 2024 at 10:35:38AM -0700, Junio C Hamano wrote:

Show 10 quoted lines

> > Yes, but I also see it from another perspective;  I don't want to worry
> > about a possible inconvenience.  And since it is perhaps an unexpected
> > precaution, for a future reviewer, hence the explicit comment in the
> > code.
> 
> But then the comment should say it only matters if the editor left
> its message incomplete, shouldn't it?  If the editor did the right
> thing and terminated its message before it exits with a newline, the
> extra LF we emit after it will only waste the vertical screen real
> estate.

Not sure if that needs to be noted in the comment.

This, and the other point raised by Randall [1], certainly makes me more in favor of using the advise_if_enabled().

Instead of "Waiting...", using a message such as "Started..." can be just as good for user guidance and less prone to error.

I think the v3 I posted is an improvement. But I believe we should consider moving towards using the advise API here, at some point.

 [1] https://lore.kernel.org/git/96bef5f9-1286-4938-99ec-6beed13ee68d@gmail.com/T/#m906fca9d24baf343326e134ac08370a77d69a603