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

3 messages, 2 authors, 2017-12-07 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2017-12-07 15:43:32

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, but if that is why we have it
there, it probably should be done only when !is_terminal_dumb().

If the trailing SP is merely there by accident, then removal without
changing anything else is also OK.

I cannot tell which is the case, hence this comment.

Thanks.

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

From: Lars Schneider <hidden>
Date: 2017-12-07 16:16:30

On 07 Dec 2017, at 16:48, Lars Schneider [off-list ref] wrote:

quoted
On 07 Dec 2017, at 16:43, Junio C Hamano [off-list ref] wrote:

lars.schneider@autodesk.com writes:
...
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, " ")
I forgot the ";" ... switching between programming languages ;-)

			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

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

From: Lars Schneider <hidden>
Date: 2017-12-07 17:53:57

On 07 Dec 2017, at 16:43, Junio C Hamano [off-list ref] wrote:

lars.schneider@autodesk.com writes:
quoted
+		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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help