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

From: Kaartic Sivaraam <kaartic.sivaraam@gmail.com>
Date: Dec 7, 2017, 17:37 UTC
Message-ID: <dfb2cde5-a499-8da3-9695-8c8fd46a96ad@gmail.com>
In-Reply-To: <xmqqmv2uec0p.fsf@gitster.mtv.corp.google.com>

On Thursday 07 December 2017 10:00 PM, Junio C Hamano wrote:

Show 6 quoted lines

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

I guess this part of the comment could be improved a little. I currently interpret it as "See if the editor emits further cruft, print a space in that case". Though, it's not what we are doing. Something like the following, perhaps?

      In a non-dumb terminal, separate our message from further cruft
      that might be emitted by the editor with SP.

Show 26 quoted lines

> +			 */
> +			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)
>