{"thread":{"id":"10631","subject":"[PATCH] fix display overlap between remote and local progress","startedAt":"2007-11-04T04:22:42Z","lastAt":"2012-03-30T14:56:03Z","messageCount":6,"participants":["Nicolas Pitre","Johannes Schindelin","Junio C Hamano","Marcin Cieslak"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"58207","messageId":"alpine.LFD.0.9999.0711032328490.21255@xanadu.home","threadId":"10631","inReplyTo":null,"subject":"[PATCH] fix display overlap between remote and local progress","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-11-04T04:22:42Z","receivedAt":"2007-11-04T04:22:42Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"It is possible for the remote summary line to be displayed over the \nlocal progress display line, and therefore that local progress gets \nbumped to the next line. However, if the progress line is long enough, \nit might not be entirely overwritten by the remote summary line. This \ncreates a messed up display such as:\n\n\tremote: Total 310 (delta 160), reused 178 (delta 112)iB/s\n\tReceiving objects: 100% (310/310), 379.98 KiB | 136 KiB/s, done.\n\nSo we have to clear the screen line before displaying the remote message \nto make sure the local progress is not visible anymore on the first \nline.\n\nYet some Git versions on the remote side might be sending updates to the \nsame line and terminate it with \\r, and a separate packet with a single \n\\n might be sent later when the progress display is done. This means \nthe screen line must *not* be cleared in that case.\n\nSince the sideband code already has to figure out line breaks in the \nreceived packet to properly prepend the \"remote:\" prefix, we can easily \ndetermine if the remote line about to be displayed is empty. Only when \nit is not then a proper suffix is inserted before the \\r or \\n to clear \nthe end of the screen line.\n\nAlso some magic constants related to the prefix length have been \nreplaced with a variable, making it similar to the suffix length \nhandling. Since gcc is smart enough to detect that the variable is \nconstant there is no impact on the generated code.\n\nSigned-off-by: Nicolas Pitre \n---\n\ndiff --git a/sideband.c b/sideband.c\nindex ab8a1e9..66385ad 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -11,13 +11,19 @@\n * stream, aka \"verbose\"). A message over band #3 is a signal that\n * the remote died unexpectedly. A flush() concludes the stream.\n */\n+\n+#define PREFIX \"remote:\"\n+#define SUFFIX \"\\e[K\" /* change to \" \" if ANSI sequences don't work */ \n+\n int recv_sideband(const char *me, int in_stream, int out, int err)\n {\n-\tchar buf[7 + LARGE_PACKET_MAX + 1];\n-\tstrcpy(buf, \"remote:\");\n+\tunsigned pf = strlen(PREFIX);\n+\tunsigned sf = strlen(SUFFIX);\n+\tchar buf[pf + LARGE_PACKET_MAX + sf + 1];\n+\tmemcpy(buf, PREFIX, pf);\n \twhile (1) {\n \t\tint band, len;\n-\t\tlen = packet_read_line(in_stream, buf+7, LARGE_PACKET_MAX);\n+\t\tlen = packet_read_line(in_stream, buf + pf, LARGE_PACKET_MAX);\n \t\tif (len == 0)\n \t\t\tbreak;\n \t\tif (len < 1) {\n@@ -25,35 +31,52 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\tsafe_write(err, buf, len);\n \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n \t\t}\n-\t\tband = buf[7] & 0xff;\n+\t\tband = buf[pf] & 0xff;\n \t\tlen--;\n \t\tswitch (band) {\n \t\tcase 3:\n-\t\t\tbuf[7] = ' ';\n-\t\t\tbuf[8+len] = '\\n';\n-\t\t\tsafe_write(err, buf, 8+len+1);\n+\t\t\tbuf[pf] = ' ';\n+\t\t\tbuf[pf+1+len] = '\\n';\n+\t\t\tsafe_write(err, buf, pf+1+len+1);\n \t\t\treturn SIDEBAND_REMOTE_ERROR;\n \t\tcase 2:\n-\t\t\tbuf[7] = ' ';\n-\t\t\tlen += 8;\n+\t\t\tbuf[pf] = ' ';\n+\t\t\tlen += pf+1;\n \t\t\twhile (1) {\n-\t\t\t\tint brk = 8;\n+\t\t\t\tint brk = pf+1;\n+\n+\t\t\t\t/* Break the buffer into separate lines. */\n \t\t\t\twhile (brk < len) {\n \t\t\t\t\tbrk++;\n \t\t\t\t\tif (buf[brk-1] == '\\n' ||\n \t\t\t\t\t buf[brk-1] == '\\r')\n \t\t\t\t\t\tbreak;\n \t\t\t\t}\n-\t\t\t\tsafe_write(err, buf, brk);\n+\n+\t\t\t\t/*\n+\t\t\t\t * Let's insert a suffix to clear the end\n+\t\t\t\t * of the screen line, but only if current\n+\t\t\t\t * line data actually contains something.\n+\t\t\t\t */\n+\t\t\t\tif (brk > pf+1 + 1) {\n+\t\t\t\t\tchar save[sf];\n+\t\t\t\t\tmemcpy(save, buf + brk, sf);\n+\t\t\t\t\tbuf[brk + sf - 1] = buf[brk - 1];\n+\t\t\t\t\tmemcpy(buf + brk - 1, SUFFIX, sf);\n+\t\t\t\t\tsafe_write(err, buf, brk + sf);\n+\t\t\t\t\tmemcpy(buf + brk, save, sf);\n+\t\t\t\t} else\n+\t\t\t\t\tsafe_write(err, buf, brk);\n+\n \t\t\t\tif (brk < len) {\n-\t\t\t\t\tmemmove(buf + 8, buf + brk, len - brk);\n-\t\t\t\t\tlen = len - brk + 8;\n+\t\t\t\t\tmemmove(buf + pf+1, buf + brk, len - brk);\n+\t\t\t\t\tlen = len - brk + pf+1;\n \t\t\t\t} else\n \t\t\t\t\tbreak;\n \t\t\t}\n \t\t\tcontinue;\n \t\tcase 1:\n-\t\t\tsafe_write(out, buf+8, len);\n+\t\t\tsafe_write(out, buf + pf+1, len);\n \t\t\tcontinue;\n \t\tdefault:\n \t\t\tlen = sprintf(buf,\n"},{"id":"58264","messageId":"Pine.LNX.4.64.0711041331520.4362@racer.site","threadId":"10631","inReplyTo":"alpine.LFD.0.9999.0711032328490.21255@xanadu.home","subject":"Re: [PATCH] fix display overlap between remote and local progress","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-04T13:33:34Z","receivedAt":"2007-11-04T13:33:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sun, 4 Nov 2007, Nicolas Pitre wrote:\n\n> +#define SUFFIX \"\\e[K\" /* change to \" \" if ANSI sequences don't work */ \n\nI am almost certain (without even testing) that cmd.exe has problems with \nthat. It does not even understand colorisation.\n\nMy vote is to let it be for the moment, and us msysGit people will \nprobably add something like NO_ANSI_TERM=YesPlease later.\n\nCiao,\nDscho\n"},{"id":"58307","messageId":"alpine.LFD.0.9999.0711041610520.21255@xanadu.home","threadId":"10631","inReplyTo":"Pine.LNX.4.64.0711041331520.4362@racer.site","subject":"Re: [PATCH] fix display overlap between remote and local progress","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-11-04T21:19:52Z","receivedAt":"2007-11-04T21:19:52Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 4 Nov 2007, Johannes Schindelin wrote:\n\n> Hi,\n> \n> On Sun, 4 Nov 2007, Nicolas Pitre wrote:\n> \n> > +#define SUFFIX \"\\e[K\" /* change to \" \" if ANSI sequences don't work */ \n> \n> I am almost certain (without even testing) that cmd.exe has problems with \n> that. It does not even understand colorisation.\n\nThat's what I was expecting. This is why I suggested an alternative in \nthe comment.\n\n\nNicolas\n"},{"id":"58318","messageId":"7v4pg1sj1r.fsf@gitster.siamese.dyndns.org","threadId":"10631","inReplyTo":"alpine.LFD.0.9999.0711041610520.21255@xanadu.home","subject":"Re: [PATCH] fix display overlap between remote and local progress","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-11-04T23:11:12Z","receivedAt":"2007-11-04T23:11:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nicolas Pitre writes:\n\n> On Sun, 4 Nov 2007, Johannes Schindelin wrote:\n>\n>> On Sun, 4 Nov 2007, Nicolas Pitre wrote:\n>> \n>> > +#define SUFFIX \"\\e[K\" /* change to \" \" if ANSI sequences don't work */ \n>> \n>> I am almost certain (without even testing) that cmd.exe has problems with \n>> that. It does not even understand colorisation.\n>\n> That's what I was expecting. This is why I suggested an alternative in \n> the comment.\n\nThat's fine --- cmd.exe weenies can patch it away ;-).\n\nThe compiler at k.org complains of \"\\e\" being non ISO-C, though.\n"},{"id":"58329","messageId":"alpine.LFD.0.9999.0711041955400.21255@xanadu.home","threadId":"10631","inReplyTo":"7v4pg1sj1r.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] fix display overlap between remote and local progress","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2007-11-05T01:07:29Z","receivedAt":"2007-11-05T01:07:29Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Sun, 4 Nov 2007, Junio C Hamano wrote:\n\n> The compiler at k.org complains of \"\\e\" being non ISO-C, though.\n\nBummer.\n\n...\n\nSigned-off-by: Nicolas Pitre \n---\ndiff --git a/sideband.c b/sideband.c\nindex 58edea6..756bbc2 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -13,7 +13,7 @@\n */\n \n #define PREFIX \"remote:\"\n-#define SUFFIX \"\\e[K\" /* change to \" \" if ANSI sequences don't work */\n+#define SUFFIX \"\\033[K\" /* change to \" \" if ANSI sequences don't work */\n \n int recv_sideband(const char *me, int in_stream, int out, int err)\n {\n"},{"id":"188164","messageId":"slrnjnbic3.30f0.saper@saper.info","threadId":"10631","inReplyTo":"Pine.LNX.4.64.0711041331520.4362@racer.site","subject":"Re: [PATCH] fix display overlap between remote and local progress","fromName":"Marcin Cieslak","fromEmail":"saper@saper.info","sentAt":"2012-03-30T14:56:03Z","receivedAt":"2012-03-30T14:56:03Z","isPatch":true,"sender":{"key":"saper@saper.info","avatar":"https://gravatar.com/avatar/8313a958c439a2d52eb2ee4cfcfa64018cefce5400d70dba51f38867f9ce7d36?d=mp&s=160"},"body":"(This is about ANSI clear-eol sequence introduced to sideline.c in commit\n ebe8fa738dcf6911fe520adce0cfa0cb26dee5e2)\n\n>> Johannes Schindelin wrote:\n> Hi,\n>\n> On Sun, 4 Nov 2007, Nicolas Pitre wrote:\n>\n>> +#define SUFFIX \"\\e[K\" /* change to \" \" if ANSI sequences don't work */ \n>\n> I am almost certain (without even testing) that cmd.exe has problems with \n> that. It does not even understand colorisation.\n>\n> My vote is to let it be for the moment, and us msysGit people will \n> probably add something like NO_ANSI_TERM=YesPlease later.\n\nHi,\n\nWe are using script(1) to record the workflow and troubleshoot problems.\nscript utility records all control codes as well (actually everything\nthat passes via pseudoterminal it creates.\n\nWe managed to get git quiet on most control codes with color.ui=false\nand pager set to nothing, but we still get ESC [ K codes from the\nremote output (we are using gerrit and it generates some). \n\nWhile I can live with git progress output lines in the log, it's\nquite annoying to edit those ANSI sequences out of the script.\nOne has to remember to set TERM=dumb before invoking _some_ git\noperations.\n\nIs there any better solution possible?\n\n//Marcin\n"}]}