| Message ID | c430a194-32ac-403c-a381-801556275f66@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | launch_editor: waiting message | expand |
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)
On Sat, Apr 13, 2024 at 04:09:27PM +0100, Phillip Wood wrote: > 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? Perhaps I am being too cautious. I'll follow Junio's suggestion and use term_clear_line() in all cases. Thanks.
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)
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. 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(-)