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

From: Lars Schneider <larsxschneider@gmail.com>
Date: Dec 7, 2017, 15:48 UTC
Message-ID: <FCBDBD58-0593-4FFC-B574-61D67CAF13C6@gmail.com>
In-Reply-To: <xmqqr2s6ee7e.fsf@gitster.mtv.corp.google.com>

Show 20 quoted lines

> On 07 Dec 2017, at 16:43, Junio C Hamano <gitster@pobox.com> wrote:
> 
> lars.schneider@autodesk.com writes:
> 
>> +		if (print_waiting_for_editor) {
>> +			fprintf(stderr,
>> +				_("hint: Waiting for your editor to close the file... "));
>> +			if (is_terminal_dumb())
>> +				/*
>> +				 * A dumb terminal cannot erase the line later on. Add a
>> +				 * newline to separate the hint from subsequent output.
>> +				 */
>> +				fprintf(stderr, "\n");
>> +			fflush(stderr);
>> +		}
> 
> Was the trailing whitespace at the end of the hint message intended?
> 
> If we expect the editor to spit out additional garbage on the line,
> it would probably help to have that SP,

Argh. I forgot to mention that in the cover letter. Yes, I added the whitespace intentionally for exactly that reason.

> but if that is why we have it
> there, it probably should be done only when !is_terminal_dumb().

That, of course, is correct. My intention was to make the code simpler but I can see that people would be confused about the whitespace.

How about this?

			fprintf(stderr,
				_("hint: Waiting for your editor to close the file..."));
			if (is_terminal_dumb())
				/*
				 * A dumb terminal cannot erase the line later on. Add a
				 * newline to separate the hint from subsequent output.
				 */
				fprintf(stderr, "\n")
			else
				fprintf(stderr, " ")

Can you squash that if you like it?

Thanks, Lars