From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sat, 13 Apr 2024 15:09:27 GMT
Subject: Re: [PATCH v3 2/2] launch_editor: waiting message on error
Message-ID: <782c7127-45d7-4bd6-930f-941a6f43088d@gmail.com>
In-Reply-To: <c430a194-32ac-403c-a381-801556275f66@gmail.com>

Hi Rubén

On 12/04/2024 18:15, Rubén Justo wrote:
> When advice.waitingForEditor configuration is not set to false, we show
> a hint telling that we are waiting for user's editor to close the file
> when we launch an editor and wait for it to return control back to us.
> We give the message on an incomplete line, expecting that we can go back
> to the line and clear the message when the editor returns successfully.
> 
> However, it is possible that the editor exits with an error status, in
> which case we show an error message and then return to our caller.  In
> such a case, the error message is given where the terminal cursor
> happens to be, which is most likely after the "we are waiting for your
> editor" message on the same line.

I think it is very likely that the editor has printed an error message 
if it exits with a non-zero exit code and if that message does not end 
with a newline that is a bug in the editor. Do you have a real-world 
example of the problem you are seeking to fix?

Best Wishes

Phillip

> Only clear the line when the editor returned cleanly, and otherwise,
> complete the message on the incomplete line with a newline before giving
> the error message.
> 
> While we're here, make the error message follow our CodingGuideLines.
> 
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
>   editor.c | 26 ++++++++++++++++++--------
>   1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/editor.c b/editor.c
> index 1da3a26f5d..eb0cfe4a28 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -104,16 +104,26 @@ static int launch_specified_editor(const char *editor, const char *path,
>   		sigchain_pop(SIGQUIT);
>   		if (sig == SIGINT || sig == SIGQUIT)
>   			raise(sig);
> +
> +		if (print_waiting_for_editor && !is_terminal_dumb()) {
> +			if (!ret)
> +				/*
> +				 * Erase the entire line to avoid wasting
> +				 * the vertical space.
> +				 */
> +				term_clear_line();
> +			else
> +				/*
> +				 * We don't want term_clear_line() here
> +				 * because the editor could have written
> +				 * some useful messages to the user.
> +				 */
> +				fprintf(stderr, "\n");
> +		}
> +
>   		if (ret)
> -			return error("There was a problem with the editor '%s'.",
> +			return error("there was a problem with the editor '%s'",
>   					editor);
> -
> -		if (print_waiting_for_editor && !is_terminal_dumb())
> -			/*
> -			 * Erase the entire line to avoid wasting the
> -			 * vertical space.
> -			 */
> -			term_clear_line();
>   	}
>   
>   	if (!buffer)