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

[Lars Schneider <larsxschneider@gmail.com>]

> On 16 Nov 2017, at 07:04, Junio C Hamano <gitster@pobox.com> wrote:

Wow. Thanks for the quick patch :-)


> When a graphical GIT_EDITOR is spawned by a Git command that opens
> and waits for it for the user input (e.g. "git rebase -i") pops its
> window elsewhere that is obscure, the user may be left staring the
> original terminal window s/he invoked the Git command from, without
> even realizing that now s/he needs to interact with another window
> the editor is waiting in, before Git can proceed.

Maybe:

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


> Show a message to the original terminal, and get rid of it when the
s/to/in/ ?

> editor returns.



> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
> * I tried this with "emacsclient" but it turns out that Emacs folks
>   have already thought about this issue, and the program says
>   "Waiting for Emacs..." while it does its thing, so from that
>   point of view, perhaps as Stefan said originally, the editor Lars
>   had trouble with is what is at fault and needs fixing?  I dunno.

Based on my (not too extensive) testing, Emacs is probably the only 
editor with this clever behavior.


>   Also, emacsclient seems to conclude its message, once the editing
>   is done, by emitting LF _before_ we have a chance to do the "go
>   back to the beginning and clear" dance, so it probably is not a
>   very good example to emulate the situation Lars had trouble with.
>   You cannot observe the nuking of the "launched, waiting..." with
>   it.
> 
> editor.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
> 
> diff --git a/editor.c b/editor.c
> index 7519edecdc..1321944716 100644
> --- a/editor.c
> +++ b/editor.c
> @@ -40,6 +40,28 @@ 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;
> +		static const char *close_notice = NULL;
> +
> +		if (isatty(2) && !close_notice) {
> +			char *term = getenv("TERM");
> +
> +			if (term && strcmp(term, "dumb"))
> +				/*
> +				 * go back to the beginning and erase the
> +				 * entire line if the terminal is capable
> +				 * to do so, to avoid wasting the vertical
> +				 * space.
> +				 */
> +				close_notice = "\r\033[K";
> +			else
> +				/* otherwise, complete and waste the line */
> +				close_notice = "done.\n";
> +		}
> +
> +		if (close_notice) {
> +			fprintf(stderr, "Launched your editor, waiting...");

I also noticed that some people don't get that they need to save and close the
file to continue. Plus, we should tell them that Git is waiting for *them* and
not anything else. Maybe we should also tell them the editor name, but that might
be too verbose. I dunno. How about this?

	fprintf(stderr, "Launched your editor ('%s'). Adjust, save, and close the file to continue. Waiting for you input... ", editor);
		

- Lars