Re: [PATCH v5 2/2] launch_editor(): indicate that Git waits for user input

[Lars Schneider <larsxschneider@gmail.com>]

> On 07 Dec 2017, at 17:30, Junio C Hamano <gitster@pobox.com> wrote:
> 
> Lars Schneider <larsxschneider@gmail.com> writes:
> 
>>> Can you squash that if you like it?
> 
> I thought you also had to update the log message that you forgot to?
> 
> Here is the replacement I queued tentatively.

Perfect. Thanks a lot for your additional fixes!

- Lars

> 
> -- >8 --
> From: Lars Schneider <larsxschneider@gmail.com>
> Date: Thu, 7 Dec 2017 16:16:41 +0100
> Subject: [PATCH] launch_editor(): indicate that Git waits for user input
> 
> When a graphical GIT_EDITOR is spawned by a Git command that opens
> and waits for user input (e.g. "git rebase -i"), then the editor window
> might be obscured by other windows. The user might be left staring at
> the original Git terminal window without even realizing that s/he needs
> to interact with another window before Git can proceed. To this user Git
> appears hanging.
> 
> Print a message that Git is waiting for editor input in the original
> terminal and get rid of it when the editor returns, if the terminal
> supports erasing the last line.  Also, make sure that our message is
> terminated with a whitespace so that any message the editor may show
> upon starting up will be kept separate from our message.
> 
> Power users might not want to see this message or their editor might
> already print such a message (e.g. emacsclient). Allow these users to
> suppress the message by disabling the "advice.waitingForEditor" config.
> 
> The standard advise() function is not used here as it would always add
> a newline which would make deleting the message harder.
> 
> Helped-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Lars Schneider <larsxschneider@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> Documentation/config.txt |  3 +++
> advice.c                 |  2 ++
> advice.h                 |  1 +
> editor.c                 | 24 ++++++++++++++++++++++++
> 4 files changed, 30 insertions(+)
> 
> diff --git a/Documentation/config.txt b/Documentation/config.txt
> index 9593bfabaa..6ebc50eea8 100644
> --- a/Documentation/config.txt
> +++ b/Documentation/config.txt
> @@ -351,6 +351,9 @@ advice.*::
> 	addEmbeddedRepo::
> 		Advice on what to do when you've accidentally added one
> 		git repo inside of another.
> +	waitingForEditor::
> +		Print a message to the terminal whenever Git is waiting for
> +		editor input from the user.
> --
> 
> core.fileMode::
> diff --git a/advice.c b/advice.c
> index d81e1cb742..af29d23e43 100644
> --- a/advice.c
> +++ b/advice.c
> @@ -17,6 +17,7 @@ int advice_set_upstream_failure = 1;
> int advice_object_name_warning = 1;
> int advice_rm_hints = 1;
> int advice_add_embedded_repo = 1;
> +int advice_waiting_for_editor = 1;
> 
> static struct {
> 	const char *name;
> @@ -38,6 +39,7 @@ static struct {
> 	{ "objectnamewarning", &advice_object_name_warning },
> 	{ "rmhints", &advice_rm_hints },
> 	{ "addembeddedrepo", &advice_add_embedded_repo },
> +	{ "waitingforeditor", &advice_waiting_for_editor },
> 
> 	/* make this an alias for backward compatibility */
> 	{ "pushnonfastforward", &advice_push_update_rejected }
> diff --git a/advice.h b/advice.h
> index c84a44531c..f7cbbd342f 100644
> --- a/advice.h
> +++ b/advice.h
> @@ -19,6 +19,7 @@ extern int advice_set_upstream_failure;
> extern int advice_object_name_warning;
> extern int advice_rm_hints;
> extern int advice_add_embedded_repo;
> +extern int advice_waiting_for_editor;
> 
> int git_default_advice_config(const char *var, const char *value);
> __attribute__((format (printf, 1, 2)))
> diff --git a/editor.c b/editor.c
> index c65ea698eb..8acce0dcd4 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -45,6 +45,23 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
> 		const char *args[] = { editor, real_path(path), NULL };
> 		struct child_process p = CHILD_PROCESS_INIT;
> 		int ret, sig;
> +		int print_waiting_for_editor = advice_waiting_for_editor && isatty(2);
> +
> +		if (print_waiting_for_editor) {
> +			/*
> +			 * A dumb terminal cannot erase the line later on. Add a
> +			 * newline to separate the hint from subsequent output.
> +			 *
> +			 * In case the editor emits further cruft after what
> +			 * we wrote above, separate it from our message with SP.
> +			 */
> +			const char term = is_terminal_dumb() ? '\n' : ' ';
> +
> +			fprintf(stderr,
> +				_("hint: Waiting for your editor to close the file...%c"),
> +				term);
> +			fflush(stderr);
> +		}
> 
> 		p.argv = args;
> 		p.env = env;
> @@ -63,6 +80,13 @@ int launch_editor(const char *path, struct strbuf *buffer, const char *const *en
> 		if (ret)
> 			return error("There was a problem with the editor '%s'.",
> 					editor);
> +
> +		if (print_waiting_for_editor && !is_terminal_dumb())
> +			/*
> +			 * Go back to the beginning and erase the entire line to
> +			 * avoid wasting the vertical space.
> +			 */
> +			fputs("\r\033[K", stderr);
> 	}
> 
> 	if (!buffer)
> -- 
> 2.15.1-480-gbc5668f98a
>