{"thread":{"id":"36800","subject":"[PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell","startedAt":"2014-05-30T23:10:51Z","lastAt":"2014-06-02T17:59:48Z","messageCount":3,"participants":["Naumov, Michael (North Sydney)","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"243065","messageId":"81E85F13A4BE084BAF4B1AA24173EDA0144E14B7@JWPKEXMBX03.corp.checkfree.com","threadId":"36800","inReplyTo":null,"subject":"[PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell","fromName":"Naumov, Michael (North Sydney)","fromEmail":"michael.naumov@fiserv.com","sentAt":"2014-05-30T23:10:51Z","receivedAt":"2014-05-30T23:10:51Z","isPatch":true,"sender":{"key":"michael.naumov@fiserv.com","avatar":"https://gravatar.com/avatar/9f0ba13d842c458d2e476a63746adc1325c81f3c25f19d241938acfd21549f50?d=mp&s=160"},"body":"Some git tools such as GitExtensions for Windows use environment\nvariable TERM=msys which causes the weird ANSI sequence shown for the\nmessages returned from server-side hooks\nWe add those ANSI sequences to help format sideband data on the user's\nterminal. However, GitExtensions is not using a terminal, and the ANSI\nsequences just confuses it. We can recognize this use by checking\nisatty().\nSee https://github.com/gitextensions/gitextensions/issues/1313 for\nmore details\n\nNOTE: I considered to cover the case that a pager has already been\nstarted. But decided that is probably not worth worrying about here,\nthough, as we shouldn't be using a pager for commands that do network\ncommunications (and if we do, omitting the magic line-clearing signal\nis probably a sane thing to do).\n\nSigned-off-by: Michael Naumov \nThanks-to: Erik Faye-Lund \nThanks-to: Jeff King \n---\n sideband.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/sideband.c b/sideband.c\nindex d1125f5..7f9dc22 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out)\n \n \tmemcpy(buf, PREFIX, pf);\n \tterm = getenv(\"TERM\");\n-\tif (term && strcmp(term, \"dumb\"))\n+\tif (isatty(2) && term && strcmp(term, \"dumb\"))\n \t\tsuffix = ANSI_SUFFIX;\n \telse\n \t\tsuffix = DUMB_SUFFIX;\n-- \n1.8.3.msysgit.0\n\nP.S. I gave up trying to send this letter from gmail, it eats my tab\ncharacter\nP.S 2. Sorry, my tab character has been eaten again!\nP.S 3. Wrapped the letter lines to fit on the terminal\n\nRegards,\nMichael\n"},{"id":"243118","messageId":"20140602062959.GA27254@sigill.intra.peff.net","threadId":"36800","inReplyTo":"81E85F13A4BE084BAF4B1AA24173EDA0144E14B7@JWPKEXMBX03.corp.checkfree.com","subject":"Re: [PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-06-02T06:30:00Z","receivedAt":"2014-06-02T06:30:00Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, May 30, 2014 at 11:10:51PM +0000, Naumov, Michael (North Sydney) wrote:\n\n> Some git tools such as GitExtensions for Windows use environment\n> variable TERM=msys which causes the weird ANSI sequence shown for the\n> messages returned from server-side hooks\n> We add those ANSI sequences to help format sideband data on the user's\n> terminal. However, GitExtensions is not using a terminal, and the ANSI\n> sequences just confuses it. We can recognize this use by checking\n> isatty().\n> See https://github.com/gitextensions/gitextensions/issues/1313 for\n> more details\n> \n> NOTE: I considered to cover the case that a pager has already been\n> started. But decided that is probably not worth worrying about here,\n> though, as we shouldn't be using a pager for commands that do network\n> communications (and if we do, omitting the magic line-clearing signal\n> is probably a sane thing to do).\n> \n> Signed-off-by: Michael Naumov \n> Thanks-to: Erik Faye-Lund \n> Thanks-to: Jeff King \n\nThis version looks fine to me.\n\n-Peff\n"},{"id":"243139","messageId":"xmqqk38z5h0r.fsf@gitster.dls.corp.google.com","threadId":"36800","inReplyTo":"81E85F13A4BE084BAF4B1AA24173EDA0144E14B7@JWPKEXMBX03.corp.checkfree.com","subject":"Re: [PATCH] sideband.c: Get rid of ANSI sequences for non-terminal shell","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2014-06-02T17:59:48Z","receivedAt":"2014-06-02T17:59:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Naumov, Michael (North Sydney)\" writes:\n\nYou either want to correct the \"From: \" header that appears in\ne-mails from you, or want to start your body of your patch message\nlike this:\n\n\tFrom: Michael Naumov \n\n\tSome git tools such as GitExtensions for Windows...\n\nif you want the author of the patch and the name you used to sign it\noff to match, which is almost always what you want.\n\n> Some git tools such as GitExtensions for Windows use environment\n> variable TERM=msys which causes the weird ANSI sequence shown for the\n> messages returned from server-side hooks\n\nThe above sentence, while it may be telling a truth, feels more or\nless irrelevant, especially the part that talks about TERM=msys.\nEven if GitExtensions used TERM=vt100, the end result would be the\nsame, wouldn't it?\n\nIf you are suggesting a fix to GitExtensions to make it export\nTERM=dumb like everybody else does, that would be a different\nstory. Mentioning \"TERM=msys causes the problem\" would be a very\nrelevant thing to do. But this patch is not that.\n\n> We add those ANSI sequences to help format sideband data on the user's\n> terminal. However, GitExtensions is not using a terminal, and the ANSI\n> sequences just confuses it. We can recognize this use by checking\n> isatty().\n\n This on the other hand is very readable. How about rephrasing these\n two like so:\n\n Diagnostic messages received on the sideband #2 from the server side\n are sent to the standard error with ANSI terminal control sequence\n \"\\033[K\" that erases to the end of line appended at the end of each\n line.\n\n However, some programs (e.g. GitExtensions for Windows) read and\n interpret and/or show the message without understanding the terminal\n control sequences, resulting them to be shown to their end users.\n To help these programs, squelch the control sequence when the\n standard error stream is not being sent to a tty.\n\nThere are programs that drive other programs (not limited to Git)\nthrough pty (hence satisfying isatty(2)) without interpreting the\nANSI terminal control sequences, and it is conventional for these\nprograms to export TERM=dumb, so and your patch still checks for\nTERM=dumb to help them, which is very good.\n\n> See https://github.com/gitextensions/gitextensions/issues/1313 for\n> more details\n\nAnd if you explain it like the above, I do not think this external\nreference is very useful.\n\n> NOTE: I considered to cover the case that a pager has already been\n> started. But decided that is probably not worth worrying about here,\n> though, as we shouldn't be using a pager for commands that do network\n> communications (and if we do, omitting the magic line-clearing signal\n> is probably a sane thing to do).\n\nSensible.\n\n> Signed-off-by: Michael Naumov \n> Thanks-to: Erik Faye-Lund \n> Thanks-to: Jeff King \n> ---\n> sideband.c | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n>\n> diff --git a/sideband.c b/sideband.c\n> index d1125f5..7f9dc22 100644\n> --- a/sideband.c\n> +++ b/sideband.c\n> @@ -30,7 +30,7 @@ int recv_sideband(const char *me, int in_stream, int out)\n> \n> \tmemcpy(buf, PREFIX, pf);\n> \tterm = getenv(\"TERM\");\n> -\tif (term && strcmp(term, \"dumb\"))\n> +\tif (isatty(2) && term && strcmp(term, \"dumb\"))\n> \t\tsuffix = ANSI_SUFFIX;\n> \telse\n> \t\tsuffix = DUMB_SUFFIX;\n"}]}