{"thread":{"id":"18241","subject":"[PATCH] winansi: support ESC [ K (erase in line)","startedAt":"2009-03-10T00:41:53Z","lastAt":"2009-03-11T10:22:22Z","messageCount":26,"participants":["Johannes Schindelin","Johannes Sixt","Shawn O. Pearce","Jay Soffian","Nicolas Pitre","Junio C Hamano","Peter Harris"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"107532","messageId":"e2b19f6c7c50e5b0a652c40b0d8e4947134ed669.1236639280u.git.johannes.schindelin@gmx.de","threadId":"18241","inReplyTo":"cover.1236639280u.git.johannes.schindelin@gmx.de","subject":"[PATCH] winansi: support ESC [ K (erase in line)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T00:41:53Z","receivedAt":"2009-03-10T00:41:53Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"To make use of it during a fetch, write() needs to be overridden, too.\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tThis fixes the last bit of msysGit issue 124 for me:\n\n\t\thttp://code.google.com/p/msysgit/issues/detail?id=124\n\n\twhich annoyed me one time to many today.\n\n\tI had an earlier version which was smaller, but pretty hacky, in \n\tthat it checked if fprintf is #define'd in xwrite(), and had \n\tspecial handling for that case.\n\n\tThis patch is only slightly hacky, in that it assumes that you do \n\tnot try to output something that ends in an incomplete ESC [\n\tsequence.\n\n compat/mingw.h | 2 ++\n compat/winansi.c | 51 +++++++++++++++++++++++++++++++++++++++------------\n 2 files changed, 41 insertions(+), 12 deletions(-)\n\ndiff --git a/compat/mingw.h b/compat/mingw.h\nindex b82903c..9aac4e1 100644\n--- a/compat/mingw.h\n+++ b/compat/mingw.h\n@@ -192,9 +192,11 @@ sig_handler_t mingw_signal(int sig, sig_handler_t handler);\n int winansi_fputs(const char *str, FILE *stream);\n int winansi_printf(const char *format, ...) __attribute__((format (printf, 1, 2)));\n int winansi_fprintf(FILE *stream, const char *format, ...) __attribute__((format (printf, 2, 3)));\n+int winansi_write(int fd, const void *buf, size_t len);\n #define fputs winansi_fputs\n #define printf(...) winansi_printf(__VA_ARGS__)\n #define fprintf(...) winansi_fprintf(__VA_ARGS__)\n+#define write winansi_write\n \n /*\n * git specific compatibility\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex e2d96df..a9d981c 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -11,15 +11,13 @@\n #undef printf\n #undef fprintf\n #undef fputs\n-/* TODO: write */\n+#undef write\n \n /*\n ANSI codes used by git: m, K\n \n This file is git-specific. Therefore, this file does not attempt\n to implement any codes that are not used by git.\n-\n- TODO: K\n */\n \n static HANDLE console;\n@@ -79,6 +77,20 @@ static void set_console_attr(void)\n \tSetConsoleTextAttribute(console, attributes);\n }\n \n+static void erase_in_line(void)\n+{\n+\tCONSOLE_SCREEN_BUFFER_INFO sbi;\n+\n+\tif (!console)\n+\t\treturn;\n+\n+\tGetConsoleScreenBufferInfo(console, &sbi);\n+\tFillConsoleOutputCharacterA(console, ' ',\n+\t\tsbi.dwSize.X - sbi.dwCursorPosition.X, sbi.dwCursorPosition,\n+\t\tNULL);\n+}\n+\n+\n static const char *set_attr(const char *str)\n {\n \tconst char *func;\n@@ -218,7 +230,7 @@ static const char *set_attr(const char *str)\n \t\tset_console_attr();\n \t\tbreak;\n \tcase 'K':\n-\t\t/* TODO */\n+\t\terase_in_line();\n \t\tbreak;\n \tdefault:\n \t\t/* Unsupported code */\n@@ -228,13 +240,16 @@ static const char *set_attr(const char *str)\n \treturn func + 1;\n }\n \n-static int ansi_emulate(const char *str, FILE *stream)\n+static int ansi_emulate(const char *str, ssize_t len, FILE *stream)\n {\n \tint rv = 0;\n \tconst char *pos = str;\n \n-\twhile (*pos) {\n-\t\tpos = strstr(str, \"\\033[\");\n+\tif (len < 0)\n+\t\tlen = strlen(str);\n+\n+\twhile (rv < len) {\n+\t\tpos = memmem(str, len - rv, \"\\033[\", 2);\n \t\tif (pos) {\n \t\t\tsize_t len = pos - str;\n \n@@ -254,10 +269,11 @@ static int ansi_emulate(const char *str, FILE *stream)\n \t\t\trv += pos - str;\n \t\t\tstr = pos;\n \t\t} else {\n-\t\t\trv += strlen(str);\n-\t\t\tfputs(str, stream);\n-\t\t\treturn rv;\n+\t\t\tfwrite(str, len - rv, 1, stream);\n+\t\t\treturn len;\n \t\t}\n+\t\tif (rv > len)\n+\t\t\tdie (\"Invalid use of ESC [ sequence detected\");\n \t}\n \treturn rv;\n }\n@@ -274,7 +290,7 @@ int winansi_fputs(const char *str, FILE *stream)\n \tif (!console)\n \t\treturn fputs(str, stream);\n \n-\trv = ansi_emulate(str, stream);\n+\trv = ansi_emulate(str, -1, stream);\n \n \tif (rv >= 0)\n \t\treturn 0;\n@@ -309,7 +325,7 @@ static int winansi_vfprintf(FILE *stream, const char *format, va_list list)\n \t\tlen = vsnprintf(buf, len + 1, format, list);\n \t}\n \n-\trv = ansi_emulate(buf, stream);\n+\trv = ansi_emulate(buf, -1, stream);\n \n \tif (buf != small_buf)\n \t\tfree(buf);\n@@ -343,3 +359,14 @@ int winansi_printf(const char *format, ...)\n \n \treturn rv;\n }\n+\n+int winansi_write(int fd, const void *buf, size_t len)\n+{\n+\tif (isatty(fd)) {\n+\t\tinit();\n+\t\tif (console)\n+\t\t\treturn ansi_emulate((const char *)buf, len,\n+\t\t\t\t\tfd == 2 ? stderr : stdout);\n+\t}\n+\treturn write(fd, buf, len);\n+}\n-- \n1.6.2.240.g23c7\n"},{"id":"107547","messageId":"49B61377.90103@viscovery.net","threadId":"18241","inReplyTo":"e2b19f6c7c50e5b0a652c40b0d8e4947134ed669.1236639280u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] winansi: support ESC [ K (erase in line)","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T07:15:03Z","receivedAt":"2009-03-10T07:15:03Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> \tThis fixes the last bit of msysGit issue 124 for me:\n> \n> \t\thttp://code.google.com/p/msysgit/issues/detail?id=124\n> \n> \twhich annoyed me one time to many today.\n> \n> \tI had an earlier version which was smaller, but pretty hacky, in \n> \tthat it checked if fprintf is #define'd in xwrite(), and had \n> \tspecial handling for that case.\n> \n> \tThis patch is only slightly hacky, in that it assumes that you do \n> \tnot try to output something that ends in an incomplete ESC [\n> \tsequence.\n\nGood that I read mail before I start hacking. I was about to do something\nabout this in a moment. ;)\n\n> To make use of it during a fetch, write() needs to be overridden, too.\n\nNo, that's not necessary with the patch that I'm about to send in a\nmoment. To replace write() for ANSI emulation really goes too far.\n\n-- Hannes\n"},{"id":"107549","messageId":"49B61703.8030602@viscovery.net","threadId":"18241","inReplyTo":"49B61377.90103@viscovery.net","subject":"[PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T07:30:11Z","receivedAt":"2009-03-10T07:30:11Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Sixt \n\nThis removes the last parameter of recv_sideband, by which the callers\ntold which channel band #2 data should be written to. Since both callers\nof the function passed 2 for the parameter, we hereby remove the\nparameter and send band #2 to stderr explicitly using fprintf.\n\nThis has the nice side-effect that the band #2 data (most importantly\nprogress reports during a fetch operation) passes through our ANSI\nemulation layer on Windows.\n\nSigned-off-by: Johannes Sixt \n---\nJohannes Sixt schrieb:\n> Johannes Schindelin schrieb:\n>> To make use of it during a fetch, write() needs to be overridden, too.\n> \n> No, that's not necessary with the patch that I'm about to send in a\n> moment. To replace write() for ANSI emulation really goes too far.\n\nHere it is. The patch is still RFC because I didn't have a chance, yet,\nto test it in practice. It passes the test suite.\n\n-- Hannes\n\n builtin-archive.c | 2 +-\n builtin-fetch-pack.c | 2 +-\n sideband.c | 20 +++++++++-----------\n sideband.h | 2 +-\n 4 files changed, 12 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-archive.c b/builtin-archive.c\nindex 60adef9..ab50ceb 100644\n--- a/builtin-archive.c\n+++ b/builtin-archive.c\n@@ -52,7 +52,7 @@ static int run_remote_archiver(int argc, const char **argv,\n \t\tdie(\"git archive: expected a flush\");\n\n \t/* Now, start reading from fd[0] and spit it out to stdout */\n-\trv = recv_sideband(\"archive\", fd[0], 1, 2);\n+\trv = recv_sideband(\"archive\", fd[0], 1);\n \tclose(fd[0]);\n \tclose(fd[1]);\n \trv |= finish_connect(conn);\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex c2e5adc..2b36099 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -482,7 +482,7 @@ static int sideband_demux(int fd, void *data)\n {\n \tint *xd = data;\n\n-\treturn recv_sideband(\"fetch-pack\", xd[0], fd, 2);\n+\treturn recv_sideband(\"fetch-pack\", xd[0], fd);\n }\n\n static int get_pack(int xd[2], char **pack_lockfile)\ndiff --git a/sideband.c b/sideband.c\nindex cca3360..a706ac8 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -19,7 +19,7 @@\n\n #define FIX_SIZE 10 /* large enough for any of the above */\n\n-int recv_sideband(const char *me, int in_stream, int out, int err)\n+int recv_sideband(const char *me, int in_stream, int out)\n {\n \tunsigned pf = strlen(PREFIX);\n \tunsigned sf;\n@@ -41,8 +41,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\tif (len == 0)\n \t\t\tbreak;\n \t\tif (len < 1) {\n-\t\t\tlen = sprintf(buf, \"%s: protocol error: no band designator\\n\", me);\n-\t\t\tsafe_write(err, buf, len);\n+\t\t\tfprintf(stderr, \"%s: protocol error: no band designator\\n\", me);\n \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n \t\t}\n \t\tband = buf[pf] & 0xff;\n@@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\tswitch (band) {\n \t\tcase 3:\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\tbuf[pf+1+len] = '\\0';\n+\t\t\tfprintf(stderr, \"%s\\n\", buf);\n \t\t\treturn SIDEBAND_REMOTE_ERROR;\n \t\tcase 2:\n \t\t\tbuf[pf] = ' ';\n@@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\t\t\tmemcpy(save, b + brk, sf);\n \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n-\t\t\t\t\tsafe_write(err, b, brk + sf);\n+\t\t\t\t\tfprintf(stderr, \"%.*s\", brk + sf, b);\n \t\t\t\t\tmemcpy(b + brk, save, sf);\n \t\t\t\t\tlen -= brk;\n \t\t\t\t} else {\n \t\t\t\t\tint l = brk ? brk : len;\n-\t\t\t\t\tsafe_write(err, b, l);\n+\t\t\t\t\tif (l > 0)\n+\t\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n \t\t\t\t\tlen -= l;\n \t\t\t\t}\n\n@@ -112,10 +112,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\tsafe_write(out, buf + pf+1, len);\n \t\t\tcontinue;\n \t\tdefault:\n-\t\t\tlen = sprintf(buf,\n-\t\t\t\t \"%s: protocol error: bad band #%d\\n\",\n-\t\t\t\t me, band);\n-\t\t\tsafe_write(err, buf, len);\n+\t\t\tfprintf(stderr, \"%s: protocol error: bad band #%d\\n\",\n+\t\t\t\tme, band);\n \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n \t\t}\n \t}\ndiff --git a/sideband.h b/sideband.h\nindex a84b691..d72db35 100644\n--- a/sideband.h\n+++ b/sideband.h\n@@ -7,7 +7,7 @@\n #define DEFAULT_PACKET_MAX 1000\n #define LARGE_PACKET_MAX 65520\n\n-int recv_sideband(const char *me, int in_stream, int out, int err);\n+int recv_sideband(const char *me, int in_stream, int out);\n ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);\n\n #endif\n-- \n1.6.2.987.g90c1d\n"},{"id":"107563","messageId":"alpine.DEB.1.00.0903101153250.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"49B61703.8030602@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T10:56:09Z","receivedAt":"2009-03-10T10:56:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> From: Johannes Sixt \n> \n> This removes the last parameter of recv_sideband, by which the callers\n> told which channel band #2 data should be written to. Since both callers\n> of the function passed 2 for the parameter, we hereby remove the\n> parameter and send band #2 to stderr explicitly using fprintf.\n\nTo be honest, I considered this myself.\n\nBut I think it is wrong. Just because the current callers happen to \noutput to stderr does not mean that we would not like sidebands that \nexchange binary data for other uses in the future.\n\nI am thinking GitTorrent here.\n\nAnd clearly, sideband support was written with future uses like that in \nmind, as it goes out of its way to transmit packets instead of strings.\n\nCiao,\nDscho\n"},{"id":"107565","messageId":"49B64ADC.2090406@viscovery.net","threadId":"18241","inReplyTo":"alpine.DEB.1.00.0903101153250.14295@intel-tinevez-2-302","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T11:11:24Z","receivedAt":"2009-03-10T11:11:24Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> On Tue, 10 Mar 2009, Johannes Sixt wrote:\n>> This removes the last parameter of recv_sideband, by which the callers\n>> told which channel band #2 data should be written to. Since both callers\n>> of the function passed 2 for the parameter, we hereby remove the\n>> parameter and send band #2 to stderr explicitly using fprintf.\n> \n> To be honest, I considered this myself.\n> \n> But I think it is wrong. Just because the current callers happen to \n> output to stderr does not mean that we would not like sidebands that \n> exchange binary data for other uses in the future.\n> \n> I am thinking GitTorrent here.\n\nClearly, we are looking at git here, not GitTorrent. \"Because we could\" is\nIMNSHO not a good justification keep code unnecessarily complicated.\n\n> And clearly, sideband support was written with future uses like that in \n> mind, as it goes out of its way to transmit packets instead of strings.\n\nAll data producers and data consumers *in git* use band #2 to transport\nerror messages and progress report. GitTorrent cannot not talk to\nupload-pack or upload-archive and expect to get arbitrary binary data over\nband #2.\n\nFor use-cases that you have in mind in GitTorrent, the *protocol* may be a\ngood choice, but the current implementation is definitely a special case.\n\n-- Hannes\n"},{"id":"107566","messageId":"49B64C3A.50909@viscovery.net","threadId":"18241","inReplyTo":"49B64ADC.2090406@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T11:17:14Z","receivedAt":"2009-03-10T11:17:14Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Sixt schrieb:\n> All data producers and data consumers *in git* use band #2 to transport\n> error messages and progress report. GitTorrent cannot not talk to\n> upload-pack or upload-archive and expect to get arbitrary binary data over\n> band #2.\n> \n> For use-cases that you have in mind in GitTorrent, the *protocol* may be a\n> good choice, but the current implementation is definitely a special case.\n\nAnd it really is: Did you notice that stuff that recv_sideband sends over\nthe channel named 'err' (before my patch) has \"remote: \" prepended on\nevery line? That's certainly not an implementation that you want if you\nsend binary data over that band!\n\n-- Hannes\n"},{"id":"107568","messageId":"alpine.DEB.1.00.0903101231240.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"49B61377.90103@viscovery.net","subject":"Re: [PATCH] winansi: support ESC [ K (erase in line)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T11:31:44Z","receivedAt":"2009-03-10T11:31:44Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > \tThis fixes the last bit of msysGit issue 124 for me:\n> > \n> > \t\thttp://code.google.com/p/msysgit/issues/detail?id=124\n> > \n> > \twhich annoyed me one time to many today.\n> > \n> > \tI had an earlier version which was smaller, but pretty hacky, in \n> > \tthat it checked if fprintf is #define'd in xwrite(), and had \n> > \tspecial handling for that case.\n> > \n> > \tThis patch is only slightly hacky, in that it assumes that you do \n> > \tnot try to output something that ends in an incomplete ESC [\n> > \tsequence.\n> \n> Good that I read mail before I start hacking. I was about to do something\n> about this in a moment. ;)\n\nHeh...\n\n> > To make use of it during a fetch, write() needs to be overridden, too.\n> \n> No, that's not necessary with the patch that I'm about to send in a\n> moment. To replace write() for ANSI emulation really goes too far.\n\nSee my response to your patch...\n\nCiao,\nDscho\n"},{"id":"299097","messageId":"alpine.DEB.1.00.0903101236480.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"49B64C3A.50909@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T11:39:12Z","receivedAt":"2009-03-10T11:39:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nFWIW GitTorrent may be implemented as part of git-daemon, if Sam's ideas \nbecome reality. And then, sideband transport is _the_ means to do \nasyncrounous communication while pushing bytes.\n\nOn Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> Johannes Sixt schrieb:\n> > All data producers and data consumers *in git* use band #2 to \n> > transport error messages and progress report. GitTorrent cannot not \n> > talk to upload-pack or upload-archive and expect to get arbitrary \n> > binary data over band #2.\n> > \n> > For use-cases that you have in mind in GitTorrent, the *protocol* may \n> > be a good choice, but the current implementation is definitely a \n> > special case.\n> \n> And it really is: Did you notice that stuff that recv_sideband sends over\n> the channel named 'err' (before my patch) has \"remote: \" prepended on\n> every line? That's certainly not an implementation that you want if you\n> send binary data over that band!\n\nYes, that is unfortunate, but can be fixed easily.\n\nHowever, I disagree to \"fix\" something that is working, even if it might \nbe more complicated than currently necessary.\n\nCiao,\nDscho\n\n"},{"id":"107570","messageId":"49B659B4.5000705@viscovery.net","threadId":"18241","inReplyTo":"alpine.DEB.1.00.0903101236480.14295@intel-tinevez-2-302","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T12:14:44Z","receivedAt":"2009-03-10T12:14:44Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> FWIW GitTorrent may be implemented as part of git-daemon, if Sam's ideas \n> become reality. And then, sideband transport is _the_ means to do \n> asyncrounous communication while pushing bytes.\n\nI do not see how recv_sideband() in its current form could be helpful here\n(assuming that you really are thinking of sending binary data over band #2).\n\n> On Tue, 10 Mar 2009, Johannes Sixt wrote:\n>> Johannes Sixt schrieb:\n>>> For use-cases that you have in mind in GitTorrent, the *protocol* may \n>>> be a good choice, but the current implementation is definitely a \n>>> special case.\n>> And it really is: Did you notice that stuff that recv_sideband sends over\n>> the channel named 'err' (before my patch) has \"remote: \" prepended on\n>> every line? That's certainly not an implementation that you want if you\n>> send binary data over that band!\n> \n> Yes, that is unfortunate, but can be fixed easily.\n\nI don't believe this. Every treatment of \"remote: \" that you take away\nfrom recv_sideband() you must insert somewhere else. Perhaps easy, but\ncertainly not as trivial as my patch.\n\n\nJust a reminder: You proposed to override write() on Windows in a\nnon-trivial way, and we are discussing the topic above because I think\nthat is not a good idea. The reasons are:\n\n- write() is a fundamental operation, and we should not mess with it out\nof caution.\n\n- Your proposal is not a catch-all. For example, combine-diff.c uses\nputs() in dump_quoted_path(). If your goal was to not touch code outside\nof compat/ then you need to override at least puts(), too.\n\n- All code that writes ANSI escapes should use fprintf() anyway.\n(Currently that is not the case, but all cases I'm aware of can be fixed\ntrivially.)\n\n-- Hannes\n"},{"id":"299098","messageId":"eaa105840903100529lc29664hd6e6fde29e06844a@mail.gmail.com","threadId":"18241","inReplyTo":"e2b19f6c7c50e5b0a652c40b0d8e4947134ed669.1236639280u.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] winansi: support ESC [ K (erase in line)","fromName":"Peter Harris","fromEmail":"git@peter.is-a-geek.org","sentAt":"2009-03-10T12:29:58Z","receivedAt":"2009-03-10T12:29:58Z","isPatch":true,"sender":{"key":"git@peter.is-a-geek.org","avatar":null},"body":"On Mon, Mar 9, 2009 at 8:41 PM, Johannes Schindelin wrote:\n> To make use of it during a fetch, write() needs to be overridden, too.\n\nThanks for looking at this.\n\nSorry for leaving this part TODO for so long. Your version is\nconsiderably less hacky than the version I had in my head, too.\n\n> +int winansi_write(int fd, const void *buf, size_t len)\n> +{\n> +       if (isatty(fd)) {\n> +               init();\n> +               if (console)\n> +                       return ansi_emulate((const char *)buf, len,\n> +                                       fd == 2 ? stderr : stdout);\n> +       }\n> +       return write(fd, buf, len);\n> +}\n\nSwitching an unbuffered write to a buffered fwrite makes me a little\nnervous. In practice, all writes probably go through this function, so\nit doesn't matter. And if the write is going somewhere it matters, it\nlikely fails isatty anyway. But I would still be less nervous with an\nfflush() after ansi_emulate.\n\nPeter Harris\n"},{"id":"107572","messageId":"alpine.DEB.1.00.0903101343530.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"49B659B4.5000705@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T12:52:14Z","receivedAt":"2009-03-10T12:52:14Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > FWIW GitTorrent may be implemented as part of git-daemon, if Sam's \n> > ideas become reality. And then, sideband transport is _the_ means to \n> > do asyncrounous communication while pushing bytes.\n> \n> I do not see how recv_sideband() in its current form could be helpful \n> here (assuming that you really are thinking of sending binary data over \n> band #2).\n\nI think it is a safe bet that the side band would be a good way to \nexchange updates to the mirror list as well as the refs list.\n\n> > On Tue, 10 Mar 2009, Johannes Sixt wrote:\n> >> Johannes Sixt schrieb:\n> >>> For use-cases that you have in mind in GitTorrent, the *protocol* \n> >>> may be a good choice, but the current implementation is definitely a \n> >>> special case.\n> >>\n> >> And it really is: Did you notice that stuff that recv_sideband sends \n> >> over the channel named 'err' (before my patch) has \"remote: \" \n> >> prepended on every line? That's certainly not an implementation that \n> >> you want if you send binary data over that band!\n> > \n> > Yes, that is unfortunate, but can be fixed easily.\n> \n> I don't believe this. Every treatment of \"remote: \" that you take away\n> from recv_sideband() you must insert somewhere else. Perhaps easy, but\n> certainly not as trivial as my patch.\n\nAFAICT it would be a matter of\n\n\tunsigned pf = isatty(err) ? strlen(PREFIX) : 0;\n\n> Just a reminder: You proposed to override write() on Windows in a \n> non-trivial way, and we are discussing the topic above because I think \n> that is not a good idea. The reasons are:\n> \n> - write() is a fundamental operation, and we should not mess with it out \n> of caution.\n\nBut we do not mess with it! We ask explicitely if we are talking about a \ntty.\n\n> - Your proposal is not a catch-all. For example, combine-diff.c uses \n> puts() in dump_quoted_path(). If your goal was to not touch code \n> outside of compat/ then you need to override at least puts(), too.\n\n>From compat/mingw.h:\n\n-- snip --\n/*\n * ANSI emulation wrappers\n */\n\nint winansi_fputs(const char *str, FILE *stream);\n[...]\n#define fputs winansi_fputs\n-- snap --\n\n... added in c09df8a, SOBbed by yourself ;-)\n\n> - All code that writes ANSI escapes should use fprintf() anyway. \n> (Currently that is not the case, but all cases I'm aware of can be \n> fixed trivially.)\n\nI disagree that all ANSI escapes have to go through fprintf(). Sometimes \nyou have a buffer, and I do not like doing extra work with %.*s there.\n\nBTW I hope that you are not annoyed by the discussion; I think it is \nnecessary and important. I am certainly not married to my current POV; so \nfar, I am still in favor of it, though.\n\nCiao,\nDscho\n"},{"id":"107573","messageId":"alpine.DEB.1.00.0903101352240.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"eaa105840903100529lc29664hd6e6fde29e06844a@mail.gmail.com","subject":"Re: [PATCH] winansi: support ESC [ K (erase in line)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T12:54:34Z","receivedAt":"2009-03-10T12:54:34Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Peter Harris wrote:\n\n> On Mon, Mar 9, 2009 at 8:41 PM, Johannes Schindelin wrote:\n> \n> > +int winansi_write(int fd, const void *buf, size_t len)\n> > +{\n> > +       if (isatty(fd)) {\n> > +               init();\n> > +               if (console)\n> > +                       return ansi_emulate((const char *)buf, len,\n> > +                                       fd == 2 ? stderr : stdout);\n> > +       }\n> > +       return write(fd, buf, len);\n> > +}\n> \n> Switching an unbuffered write to a buffered fwrite makes me a little \n> nervous. In practice, all writes probably go through this function, so \n> it doesn't matter. And if the write is going somewhere it matters, it \n> likely fails isatty anyway. But I would still be less nervous with an \n> fflush() after ansi_emulate.\n\nThat would make the code substantially more messy, though.\n\nBesides, half of it is already there: when encountering an ANSI sequence, \nyour code already fflush()es the fd before setting the new attributes. \nAnd stderr does not need the fflush() anyway.\n\nCiao,\nDscho\n"},{"id":"107579","messageId":"49B6788F.2080609@viscovery.net","threadId":"18241","inReplyTo":"alpine.DEB.1.00.0903101343530.14295@intel-tinevez-2-302","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T14:26:23Z","receivedAt":"2009-03-10T14:26:23Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> Hi,\n> \n> On Tue, 10 Mar 2009, Johannes Sixt wrote:\n> \n>> Johannes Schindelin schrieb:\n>>> FWIW GitTorrent may be implemented as part of git-daemon, if Sam's \n>>> ideas become reality. And then, sideband transport is _the_ means to \n>>> do asyncrounous communication while pushing bytes.\n>> I do not see how recv_sideband() in its current form could be helpful \n>> here (assuming that you really are thinking of sending binary data over \n>> band #2).\n> \n> I think it is a safe bet that the side band would be a good way to \n> exchange updates to the mirror list as well as the refs list.\n\nBinary or not - the purpose and suitability of the sideband *protocol* for\nthis task are undisputed.\n\nBut you don't want to have \"remote:\" thrown in at seemingly random places\nin the demultiplexed stream that comes fromt he current implementation of\nrecv_sideband().\n\n>>> On Tue, 10 Mar 2009, Johannes Sixt wrote:\n>>>> And it really is: Did you notice that stuff that recv_sideband sends \n>>>> over the channel named 'err' (before my patch) has \"remote: \" \n>>>> prepended on every line? That's certainly not an implementation that \n>>>> you want if you send binary data over that band!\n>>> Yes, that is unfortunate, but can be fixed easily.\n>> I don't believe this. Every treatment of \"remote: \" that you take away\n>> from recv_sideband() you must insert somewhere else. Perhaps easy, but\n>> certainly not as trivial as my patch.\n> \n> AFAICT it would be a matter of\n> \n> \tunsigned pf = isatty(err) ? strlen(PREFIX) : 0;\n\nBut don't you see that are mixing a high-level concept of \"terminal\" into\nthe low-level function that you want it to be? In its current form,\nrecv_sideband() is *not* a low-level utility, it's already at a high level\nthat knows about the line-oriented nature of band #2. What you need for\nGitTorrent is a different function that *only* demultiplexes the sideband\nprotocol data into different streams without munging them. That's a\ntotally different function that *maybe* can share some code with the\ncurrent recv_sideband().\n\n>> Just a reminder: You proposed to override write() on Windows in a \n>> non-trivial way, and we are discussing the topic above because I think \n>> that is not a good idea. The reasons are:\n>>\n>> - write() is a fundamental operation, and we should not mess with it out \n>> of caution.\n> \n> But we do not mess with it! We ask explicitely if we are talking about a \n> tty.\n\nWith reference to Peter's reply, I'm not the only one who gets nervous if\nwrite() is replaced in a non-trivial way.\n\nAfter all, you are sneaking the high-level concept \"terminal emulation\"\ninto the low-level write() function.\n\n>> - Your proposal is not a catch-all. For example, combine-diff.c uses \n>> puts() in dump_quoted_path(). If your goal was to not touch code \n>> outside of compat/ then you need to override at least puts(), too.\n> \n>>From compat/mingw.h:\n> \n> -- snip --\n> /*\n> * ANSI emulation wrappers\n> */\n> \n> int winansi_fputs(const char *str, FILE *stream);\n> [...]\n> #define fputs winansi_fputs\n> -- snap --\n> \n> ... added in c09df8a, SOBbed by yourself ;-)\n\nMy point was that you cannot get away without modifying code outside of\ncompat/ (if that was your motivation to override write()). I don't care\nwhether we change this instance to fputs() or fprintf(). But we already\n*have* something, and don't need *yet another* override.\n\n>> - All code that writes ANSI escapes should use fprintf() anyway. \n>> (Currently that is not the case, but all cases I'm aware of can be \n>> fixed trivially.)\n> \n> I disagree that all ANSI escapes have to go through fprintf(). Sometimes \n> you have a buffer, and I do not like doing extra work with %.*s there.\n\nBut on the other hand you risk breaking write() semantics and give us a\ncolorful mix of concepts.\n\nI don't insist in that ANSI escapes must go through fprintf(), but they\nshould really not go through a level that is lower than stdio. Basic file\nIO should really not be muddied with terminal emulation.\n\n> BTW I hope that you are not annoyed by the discussion; I think it is \n> necessary and important. I am certainly not married to my current POV; so \n> far, I am still in favor of it, though.\n\nI'm absolutely not annoyed. And I am as married to my POV as you are to\nyours. ;) In this case we perhaps need a tie-breaker.\n\n-- Hannes\n"},{"id":"107580","messageId":"20090310143851.GP11989@spearce.org","threadId":"18241","inReplyTo":"49B6788F.2080609@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-10T14:38:51Z","receivedAt":"2009-03-10T14:38:51Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt wrote:\n> But don't you see that are mixing a high-level concept of \"terminal\" into\n> the low-level function that you want it to be? In its current form,\n> recv_sideband() is *not* a low-level utility, it's already at a high level\n> that knows about the line-oriented nature of band #2. What you need for\n> GitTorrent is a different function that *only* demultiplexes the sideband\n> protocol data into different streams without munging them. That's a\n> totally different function that *maybe* can share some code with the\n> current recv_sideband().\n\nACK.\n\nThe definition of the streams in the current sideband protocol\nare rather well defined for the one protocol that uses it,\nfetch-pack/receive-pack:\n\n stream #1: pack data\n stream #2: stderr messages, progress, meant for tty\n stream #3: oh-sh*t abort message, remote is dead, goodbye!\n\nThe stream number is encoded as a byte. Anyone trying to reuse the\nsideband protocol within the fetch-pack/receive-pack protocol to\ncarry *extra* data should use new channel numbers. We have another\n252 remaining. I don't think we're lacking on description space.\n \n> With reference to Peter's reply, I'm not the only one who gets nervous if\n> write() is replaced in a non-trivial way.\n> \n> After all, you are sneaking the high-level concept \"terminal emulation\"\n> into the low-level write() function.\n\nOh god. Please do not do this. I usually ignore the Win32 port\nstuff. But I agree with you Hannes, please do not do this.\n \n> I'm absolutely not annoyed. And I am as married to my POV as you are to\n> yours. ;) In this case we perhaps need a tie-breaker.\n\nI cast my vote in with you Hannes. Your original RFC patch made\nsense as a cleanup in the code.\n\nI agree with you that the way the code is currently structured\nand used, sideband #2 should always dump to the stderr channel\nof the process. We also assume in a lot of other places that\nfprintf(stderr, ...) is a good way to report progress to the\nend-user. This is no different, its just progress data from the\nremote system rather than the local system.\n\nHuge refactorings would be necessary to split sideband #2 into\nsomething other than stderr. You would also want to replace nearly\nall references to stderr. We're not going down that road.\n\nFWIW, JGit takes the meanings of the streams as I described them\nabove. Stream #1 goes to the pack processing, stream #2 gets parsed\nand converted into updates calls to our ProgressMonitor API (which in\nturn goes to GUI progress meters in the host IDE), stream #3, if it\never shows up, turns into the message of an exception being thrown\nup the stack. That is the definition of this particular protocol.\nAccept it, and move it. :-)\n\nThe sideband protocol is fairly simple. If we ever needed to use\nit for other data, we could probably refactor some of the header\nformatting/parsing out a bit and create a more generalized split\nfunction, under a different name. But that's all in the future\nand something we just don't have an immediate need to worry about.\nSo don't worry about it.\n\n-- \nShawn.\n"},{"id":"107582","messageId":"alpine.DEB.1.00.0903101545540.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"20090310143851.GP11989@spearce.org","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T14:46:23Z","receivedAt":"2009-03-10T14:46:23Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Shawn O. Pearce wrote:\n\n> Johannes Sixt wrote:\n> > But don't you see that are mixing a high-level concept of \"terminal\" \n> > into the low-level function that you want it to be? In its current \n> > form, recv_sideband() is *not* a low-level utility, it's already at a \n> > high level that knows about the line-oriented nature of band #2. What \n> > you need for GitTorrent is a different function that *only* \n> > demultiplexes the sideband protocol data into different streams \n> > without munging them. That's a totally different function that *maybe* \n> > can share some code with the current recv_sideband().\n> \n> ACK.\n> \n> The definition of the streams in the current sideband protocol are \n> rather well defined for the one protocol that uses it, \n> fetch-pack/receive-pack:\n> \n> stream #1: pack data\n> stream #2: stderr messages, progress, meant for tty\n> stream #3: oh-sh*t abort message, remote is dead, goodbye!\n> \n> The stream number is encoded as a byte. Anyone trying to reuse the \n> sideband protocol within the fetch-pack/receive-pack protocol to carry \n> *extra* data should use new channel numbers. We have another 252 \n> remaining. I don't think we're lacking on description space.\n\nFair enough, the tie-breaker hath spoken.\n\nCiao,\nDscho\n"},{"id":"107583","messageId":"20090310144646.GQ11989@spearce.org","threadId":"18241","inReplyTo":"49B61703.8030602@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Shawn O. Pearce","fromEmail":"spearce@spearce.org","sentAt":"2009-03-10T14:46:46Z","receivedAt":"2009-03-10T14:46:46Z","isPatch":true,"sender":{"key":"spearce@spearce.org","avatar":"https://avatars.githubusercontent.com/u/34844?v=4"},"body":"Johannes Sixt wrote:\n> From: Johannes Sixt \n> \n> This removes the last parameter of recv_sideband, by which the callers\n> told which channel band #2 data should be written to. Since both callers\n> of the function passed 2 for the parameter, we hereby remove the\n> parameter and send band #2 to stderr explicitly using fprintf.\n> \n> This has the nice side-effect that the band #2 data (most importantly\n> progress reports during a fetch operation) passes through our ANSI\n> emulation layer on Windows.\n> \n> Signed-off-by: Johannes Sixt \n\nLooks right to me.\n\n> diff --git a/sideband.c b/sideband.c\n> index cca3360..a706ac8 100644\n> --- a/sideband.c\n> +++ b/sideband.c\n> @@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> \t\tswitch (band) {\n> \t\tcase 3:\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\tbuf[pf+1+len] = '\\0';\n> +\t\t\tfprintf(stderr, \"%s\\n\", buf);\n\nCan't you instead do:\n\n\tfprintf(stderr, \"%.*s\\n\", buf, pf + len);\n\nlike you do...\n\n> @@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> \t\t\t\t\tmemcpy(save, b + brk, sf);\n> \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n> \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n> -\t\t\t\t\tsafe_write(err, b, brk + sf);\n> +\t\t\t\t\tfprintf(stderr, \"%.*s\", brk + sf, b);\n> \t\t\t\t\tmemcpy(b + brk, save, sf);\n> \t\t\t\t\tlen -= brk;\n> \t\t\t\t} else {\n> \t\t\t\t\tint l = brk ? brk : len;\n> -\t\t\t\t\tsafe_write(err, b, l);\n> +\t\t\t\t\tif (l > 0)\n> +\t\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n\nhere?\n\n-- \nShawn.\n"},{"id":"107584","messageId":"49B680F7.4040103@viscovery.net","threadId":"18241","inReplyTo":"20090310144646.GQ11989@spearce.org","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T15:02:15Z","receivedAt":"2009-03-10T15:02:15Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Shawn O. Pearce schrieb:\n> Johannes Sixt wrote:\n>> diff --git a/sideband.c b/sideband.c\n>> index cca3360..a706ac8 100644\n>> --- a/sideband.c\n>> +++ b/sideband.c\n>> @@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n>> \t\tswitch (band) {\n>> \t\tcase 3:\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\tbuf[pf+1+len] = '\\0';\n>> +\t\t\tfprintf(stderr, \"%s\\n\", buf);\n> \n> Can't you instead do:\n> \n> \tfprintf(stderr, \"%.*s\\n\", buf, pf + len);\n> \n> like you do...\n> \n>> @@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n>> \t\t\t\t\tmemcpy(save, b + brk, sf);\n>> \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n>> \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n>> -\t\t\t\t\tsafe_write(err, b, brk + sf);\n>> +\t\t\t\t\tfprintf(stderr, \"%.*s\", brk + sf, b);\n>> \t\t\t\t\tmemcpy(b + brk, save, sf);\n>> \t\t\t\t\tlen -= brk;\n>> \t\t\t\t} else {\n>> \t\t\t\t\tint l = brk ? brk : len;\n>> -\t\t\t\t\tsafe_write(err, b, l);\n>> +\t\t\t\t\tif (l > 0)\n>> +\t\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n> \n> here?\n\nI deliberatly avoided \"%.*s\" in the former hunk (1) because of the posts\nthat we had yesterday about potentially misbehaved fprintf in the case\nwhere the precision is 0; and (2) because it was so easy to avoid it. I\ndon't think we need ultimate performance in this case, and I also consider\nthe plain \"%s\\n\" more readable.\n\nThat said, the second hunk is really only the minimal change and I'd like\nto rewrite it to get rid of the memcpy stuff. It is really not needed once\nfprintf is in the game. But that's a separate patch.\n\n-- Hannes\n"},{"id":"107586","messageId":"alpine.DEB.1.00.0903101605460.14295@intel-tinevez-2-302","threadId":"18241","inReplyTo":"49B680F7.4040103@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-10T15:07:29Z","receivedAt":"2009-03-10T15:07:29Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> Shawn O. Pearce schrieb:\n> > Johannes Sixt wrote:\n> >> diff --git a/sideband.c b/sideband.c\n> >> index cca3360..a706ac8 100644\n> >> --- a/sideband.c\n> >> +++ b/sideband.c\n> >> @@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> >> \t\tswitch (band) {\n> >> \t\tcase 3:\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\tbuf[pf+1+len] = '\\0';\n> >> +\t\t\tfprintf(stderr, \"%s\\n\", buf);\n> > \n> > Can't you instead do:\n> > \n> > \tfprintf(stderr, \"%.*s\\n\", buf, pf + len);\n> > \n> > like you do...\n> > \n> >> @@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> >> \t\t\t\t\tmemcpy(save, b + brk, sf);\n> >> \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n> >> \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n> >> -\t\t\t\t\tsafe_write(err, b, brk + sf);\n> >> +\t\t\t\t\tfprintf(stderr, \"%.*s\", brk + sf, b);\n> >> \t\t\t\t\tmemcpy(b + brk, save, sf);\n> >> \t\t\t\t\tlen -= brk;\n> >> \t\t\t\t} else {\n> >> \t\t\t\t\tint l = brk ? brk : len;\n> >> -\t\t\t\t\tsafe_write(err, b, l);\n> >> +\t\t\t\t\tif (l > 0)\n> >> +\t\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n> > \n> > here?\n> \n> I deliberatly avoided \"%.*s\" in the former hunk (1) because of the posts\n> that we had yesterday about potentially misbehaved fprintf in the case\n> where the precision is 0;\n\nDidn't that turn out to be a false alarm?\n\n> and (2) because it was so easy to avoid it. I don't think we need \n> ultimate performance in this case, and I also consider the plain \"%s\\n\" \n> more readable.\n> \n> That said, the second hunk is really only the minimal change and I'd \n> like to rewrite it to get rid of the memcpy stuff. It is really not \n> needed once fprintf is in the game. But that's a separate patch.\n\nI think, indeed, that you can avoid the memcpy() by using %.*s. The \nprivate buffer is only used to make sure that the text is written in one \ngo anyway (i.e. that two sidebands messages are not written to the same \nline because they use multiple calls to fprintf()/fwrite() per line), \nright?\n\nCiao,\nDscho\n"},{"id":"107587","messageId":"49B683BC.3030104@viscovery.net","threadId":"18241","inReplyTo":"alpine.DEB.1.00.0903101605460.14295@intel-tinevez-2-302","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Johannes Sixt","fromEmail":"j.sixt@viscovery.net","sentAt":"2009-03-10T15:14:04Z","receivedAt":"2009-03-10T15:14:04Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"Johannes Schindelin schrieb:\n> On Tue, 10 Mar 2009, Johannes Sixt wrote:\n>> I deliberatly avoided \"%.*s\" in the former hunk (1) because of the posts\n>> that we had yesterday about potentially misbehaved fprintf in the case\n>> where the precision is 0;\n> \n> Didn't that turn out to be a false alarm?\n\nEven better. I didn't follow the thread very closely.\n\n> I think, indeed, that you can avoid the memcpy() by using %.*s. The \n> private buffer is only used to make sure that the text is written in one \n> go anyway (i.e. that two sidebands messages are not written to the same \n> line because they use multiple calls to fprintf()/fwrite() per line), \n> right?\n\nIt must be something like that, yes. ;)\n\n-- Hannes\n"},{"id":"107591","messageId":"76718490903100935see26eake42d328e64b1d09d@mail.gmail.com","threadId":"18241","inReplyTo":"alpine.DEB.1.00.0903101605460.14295@intel-tinevez-2-302","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Jay Soffian","fromEmail":"jaysoffian@gmail.com","sentAt":"2009-03-10T16:35:08Z","receivedAt":"2009-03-10T16:35:08Z","isPatch":true,"sender":{"key":"jaysoffian@gmail.com","avatar":"https://avatars.githubusercontent.com/u/155970?v=4"},"body":"On Tue, Mar 10, 2009 at 11:07 AM, Johannes Schindelin\n wrote:\n>> I deliberatly avoided \"%.*s\" in the former hunk (1) because of the posts\n>> that we had yesterday about potentially misbehaved fprintf in the case\n>> where the precision is 0;\n>\n> Didn't that turn out to be a false alarm?\n\nYes. A party who will remain nameless was reading the wrong man page\nsection (printf(1) vs printf(3)). :-)\n\nj.\n"},{"id":"107595","messageId":"alpine.LFD.2.00.0903101329370.30483@xanadu.home","threadId":"18241","inReplyTo":"49B683BC.3030104@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-03-10T17:35:30Z","receivedAt":"2009-03-10T17:35:30Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> Johannes Schindelin schrieb:\n> > I think, indeed, that you can avoid the memcpy() by using %.*s. The \n> > private buffer is only used to make sure that the text is written in one \n> > go anyway (i.e. that two sidebands messages are not written to the same \n> > line because they use multiple calls to fprintf()/fwrite() per line), \n> > right?\n> \n> It must be something like that, yes. ;)\n\nIt is a bit more complex than that.\n\nMessages received over sideband #2 may have two kinds of line break \ncharacters: \\r or \\n. We also have a string suffix which is used to \nclear the remaining of the screen line in those cases where a previous \nline is overwritten and the previous line was longer. That may happen \nif the previous line was terminated with \\r. In that case, the string \nsuffix _must_ be inserted before the line break character for obvious \nreasons.\n\n\nNicolas\n"},{"id":"107596","messageId":"alpine.LFD.2.00.0903101335470.30483@xanadu.home","threadId":"18241","inReplyTo":"49B61703.8030602@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2009-03-10T17:37:17Z","receivedAt":"2009-03-10T17:37:17Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> From: Johannes Sixt \n> \n> This removes the last parameter of recv_sideband, by which the callers\n> told which channel band #2 data should be written to. Since both callers\n> of the function passed 2 for the parameter, we hereby remove the\n> parameter and send band #2 to stderr explicitly using fprintf.\n> \n> This has the nice side-effect that the band #2 data (most importantly\n> progress reports during a fetch operation) passes through our ANSI\n> emulation layer on Windows.\n\nYou appear to modify band #3 as well. Better mention it in the commit \nlog.\n\n> Signed-off-by: Johannes Sixt \n\nAcked-by: Nicolas Pitre \n\n\n> ---\n> Johannes Sixt schrieb:\n> > Johannes Schindelin schrieb:\n> >> To make use of it during a fetch, write() needs to be overridden, too.\n> > \n> > No, that's not necessary with the patch that I'm about to send in a\n> > moment. To replace write() for ANSI emulation really goes too far.\n> \n> Here it is. The patch is still RFC because I didn't have a chance, yet,\n> to test it in practice. It passes the test suite.\n> \n> -- Hannes\n> \n> builtin-archive.c | 2 +-\n> builtin-fetch-pack.c | 2 +-\n> sideband.c | 20 +++++++++-----------\n> sideband.h | 2 +-\n> 4 files changed, 12 insertions(+), 14 deletions(-)\n> \n> diff --git a/builtin-archive.c b/builtin-archive.c\n> index 60adef9..ab50ceb 100644\n> --- a/builtin-archive.c\n> +++ b/builtin-archive.c\n> @@ -52,7 +52,7 @@ static int run_remote_archiver(int argc, const char **argv,\n> \t\tdie(\"git archive: expected a flush\");\n> \n> \t/* Now, start reading from fd[0] and spit it out to stdout */\n> -\trv = recv_sideband(\"archive\", fd[0], 1, 2);\n> +\trv = recv_sideband(\"archive\", fd[0], 1);\n> \tclose(fd[0]);\n> \tclose(fd[1]);\n> \trv |= finish_connect(conn);\n> diff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\n> index c2e5adc..2b36099 100644\n> --- a/builtin-fetch-pack.c\n> +++ b/builtin-fetch-pack.c\n> @@ -482,7 +482,7 @@ static int sideband_demux(int fd, void *data)\n> {\n> \tint *xd = data;\n> \n> -\treturn recv_sideband(\"fetch-pack\", xd[0], fd, 2);\n> +\treturn recv_sideband(\"fetch-pack\", xd[0], fd);\n> }\n> \n> static int get_pack(int xd[2], char **pack_lockfile)\n> diff --git a/sideband.c b/sideband.c\n> index cca3360..a706ac8 100644\n> --- a/sideband.c\n> +++ b/sideband.c\n> @@ -19,7 +19,7 @@\n> \n> #define FIX_SIZE 10 /* large enough for any of the above */\n> \n> -int recv_sideband(const char *me, int in_stream, int out, int err)\n> +int recv_sideband(const char *me, int in_stream, int out)\n> {\n> \tunsigned pf = strlen(PREFIX);\n> \tunsigned sf;\n> @@ -41,8 +41,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> \t\tif (len == 0)\n> \t\t\tbreak;\n> \t\tif (len < 1) {\n> -\t\t\tlen = sprintf(buf, \"%s: protocol error: no band designator\\n\", me);\n> -\t\t\tsafe_write(err, buf, len);\n> +\t\t\tfprintf(stderr, \"%s: protocol error: no band designator\\n\", me);\n> \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n> \t\t}\n> \t\tband = buf[pf] & 0xff;\n> @@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> \t\tswitch (band) {\n> \t\tcase 3:\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\tbuf[pf+1+len] = '\\0';\n> +\t\t\tfprintf(stderr, \"%s\\n\", buf);\n> \t\t\treturn SIDEBAND_REMOTE_ERROR;\n> \t\tcase 2:\n> \t\t\tbuf[pf] = ' ';\n> @@ -95,12 +94,13 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> \t\t\t\t\tmemcpy(save, b + brk, sf);\n> \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n> \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n> -\t\t\t\t\tsafe_write(err, b, brk + sf);\n> +\t\t\t\t\tfprintf(stderr, \"%.*s\", brk + sf, b);\n> \t\t\t\t\tmemcpy(b + brk, save, sf);\n> \t\t\t\t\tlen -= brk;\n> \t\t\t\t} else {\n> \t\t\t\t\tint l = brk ? brk : len;\n> -\t\t\t\t\tsafe_write(err, b, l);\n> +\t\t\t\t\tif (l > 0)\n> +\t\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n> \t\t\t\t\tlen -= l;\n> \t\t\t\t}\n> \n> @@ -112,10 +112,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n> \t\t\tsafe_write(out, buf + pf+1, len);\n> \t\t\tcontinue;\n> \t\tdefault:\n> -\t\t\tlen = sprintf(buf,\n> -\t\t\t\t \"%s: protocol error: bad band #%d\\n\",\n> -\t\t\t\t me, band);\n> -\t\t\tsafe_write(err, buf, len);\n> +\t\t\tfprintf(stderr, \"%s: protocol error: bad band #%d\\n\",\n> +\t\t\t\tme, band);\n> \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n> \t\t}\n> \t}\n> diff --git a/sideband.h b/sideband.h\n> index a84b691..d72db35 100644\n> --- a/sideband.h\n> +++ b/sideband.h\n> @@ -7,7 +7,7 @@\n> #define DEFAULT_PACKET_MAX 1000\n> #define LARGE_PACKET_MAX 65520\n> \n> -int recv_sideband(const char *me, int in_stream, int out, int err);\n> +int recv_sideband(const char *me, int in_stream, int out);\n> ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);\n> \n> #endif\n> -- \n> 1.6.2.987.g90c1d\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"},{"id":"107612","messageId":"200903102254.17517.j6t@kdbg.org","threadId":"18241","inReplyTo":"49B61703.8030602@viscovery.net","subject":"[PATCH 1/2] recv_sideband: Bands #2 and #3 always go to stderr","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-03-10T21:54:17Z","receivedAt":"2009-03-10T21:54:17Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"This removes the last parameter of recv_sideband, by which the callers\ntold which channel bands #2 and #3 should be written to.\n\nSayeth Shawn Pearce:\n\n The definition of the streams in the current sideband protocol\n are rather well defined for the one protocol that uses it,\n fetch-pack/receive-pack:\n\n stream #1: pack data\n stream #2: stderr messages, progress, meant for tty\n stream #3: abort message, remote is dead, goodbye!\n\nSince both callers of the function passed 2 for the parameter, we hereby\nremove it and send bands #2 and #3 to stderr explicitly using fprintf.\n\nThis has the nice side-effect that these two streams pass through our\nANSI emulation layer on Windows.\n\nSigned-off-by: Johannes Sixt \nAcked-by: Nicolas Pitre \n---\n This patch has an updated commit message and this small change:\n\n \t\t\t\t} else {\n \t\t\t\t\tint l = brk ? brk : len;\n -\t\t\t\t\tif (l > 0)\n -\t\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n +\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n \t\t\t\t\tlen -= l;\n \t\t\t\t}\n\n I also has undergone some practical tests in addition to the\n test suite.\n\n -- Hannes\n\n builtin-archive.c | 2 +-\n builtin-fetch-pack.c | 2 +-\n sideband.c | 19 ++++++++-----------\n sideband.h | 2 +-\n 4 files changed, 11 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin-archive.c b/builtin-archive.c\nindex 60adef9..ab50ceb 100644\n--- a/builtin-archive.c\n+++ b/builtin-archive.c\n@@ -52,7 +52,7 @@ static int run_remote_archiver(int argc, const char **argv,\n \t\tdie(\"git archive: expected a flush\");\n \n \t/* Now, start reading from fd[0] and spit it out to stdout */\n-\trv = recv_sideband(\"archive\", fd[0], 1, 2);\n+\trv = recv_sideband(\"archive\", fd[0], 1);\n \tclose(fd[0]);\n \tclose(fd[1]);\n \trv |= finish_connect(conn);\ndiff --git a/builtin-fetch-pack.c b/builtin-fetch-pack.c\nindex c2e5adc..2b36099 100644\n--- a/builtin-fetch-pack.c\n+++ b/builtin-fetch-pack.c\n@@ -482,7 +482,7 @@ static int sideband_demux(int fd, void *data)\n {\n \tint *xd = data;\n \n-\treturn recv_sideband(\"fetch-pack\", xd[0], fd, 2);\n+\treturn recv_sideband(\"fetch-pack\", xd[0], fd);\n }\n \n static int get_pack(int xd[2], char **pack_lockfile)\ndiff --git a/sideband.c b/sideband.c\nindex cca3360..899b1ff 100644\n--- a/sideband.c\n+++ b/sideband.c\n@@ -19,7 +19,7 @@\n \n #define FIX_SIZE 10 /* large enough for any of the above */\n \n-int recv_sideband(const char *me, int in_stream, int out, int err)\n+int recv_sideband(const char *me, int in_stream, int out)\n {\n \tunsigned pf = strlen(PREFIX);\n \tunsigned sf;\n@@ -41,8 +41,7 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\tif (len == 0)\n \t\t\tbreak;\n \t\tif (len < 1) {\n-\t\t\tlen = sprintf(buf, \"%s: protocol error: no band designator\\n\", me);\n-\t\t\tsafe_write(err, buf, len);\n+\t\t\tfprintf(stderr, \"%s: protocol error: no band designator\\n\", me);\n \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n \t\t}\n \t\tband = buf[pf] & 0xff;\n@@ -50,8 +49,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\tswitch (band) {\n \t\tcase 3:\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\tbuf[pf+1+len] = '\\0';\n+\t\t\tfprintf(stderr, \"%s\\n\", buf);\n \t\t\treturn SIDEBAND_REMOTE_ERROR;\n \t\tcase 2:\n \t\t\tbuf[pf] = ' ';\n@@ -95,12 +94,12 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\t\t\tmemcpy(save, b + brk, sf);\n \t\t\t\t\tb[brk + sf - 1] = b[brk - 1];\n \t\t\t\t\tmemcpy(b + brk - 1, suffix, sf);\n-\t\t\t\t\tsafe_write(err, b, brk + sf);\n+\t\t\t\t\tfprintf(stderr, \"%.*s\", brk + sf, b);\n \t\t\t\t\tmemcpy(b + brk, save, sf);\n \t\t\t\t\tlen -= brk;\n \t\t\t\t} else {\n \t\t\t\t\tint l = brk ? brk : len;\n-\t\t\t\t\tsafe_write(err, b, l);\n+\t\t\t\t\tfprintf(stderr, \"%.*s\", l, b);\n \t\t\t\t\tlen -= l;\n \t\t\t\t}\n \n@@ -112,10 +111,8 @@ int recv_sideband(const char *me, int in_stream, int out, int err)\n \t\t\tsafe_write(out, buf + pf+1, len);\n \t\t\tcontinue;\n \t\tdefault:\n-\t\t\tlen = sprintf(buf,\n-\t\t\t\t \"%s: protocol error: bad band #%d\\n\",\n-\t\t\t\t me, band);\n-\t\t\tsafe_write(err, buf, len);\n+\t\t\tfprintf(stderr, \"%s: protocol error: bad band #%d\\n\",\n+\t\t\t\tme, band);\n \t\t\treturn SIDEBAND_PROTOCOL_ERROR;\n \t\t}\n \t}\ndiff --git a/sideband.h b/sideband.h\nindex a84b691..d72db35 100644\n--- a/sideband.h\n+++ b/sideband.h\n@@ -7,7 +7,7 @@\n #define DEFAULT_PACKET_MAX 1000\n #define LARGE_PACKET_MAX 65520\n \n-int recv_sideband(const char *me, int in_stream, int out, int err);\n+int recv_sideband(const char *me, int in_stream, int out);\n ssize_t send_sideband(int fd, int band, const char *data, ssize_t sz, int packet_max);\n \n #endif\n-- \n1.6.2.103.gb3eac.dirty\n"},{"id":"107613","messageId":"200903102258.09521.j6t@kdbg.org","threadId":"18241","inReplyTo":"200903102254.17517.j6t@kdbg.org","subject":"[PATCH 2/2] winansi: support ESC [ K (erase in line)","fromName":"Johannes Sixt","fromEmail":"j6t@kdbg.org","sentAt":"2009-03-10T21:58:09Z","receivedAt":"2009-03-10T21:58:09Z","isPatch":true,"sender":{"key":"j6t@kdbg.org","avatar":"https://avatars.githubusercontent.com/u/14810926?v=4"},"body":"From: Johannes Schindelin \n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Johannes Sixt \n---\n This is your patch without the write() part.\n\n -- Hannes\n\n compat/winansi.c | 18 +++++++++++++++---\n 1 files changed, 15 insertions(+), 3 deletions(-)\n\ndiff --git a/compat/winansi.c b/compat/winansi.c\nindex e2d96df..44dc293 100644\n--- a/compat/winansi.c\n+++ b/compat/winansi.c\n@@ -18,8 +18,6 @@\n \n This file is git-specific. Therefore, this file does not attempt\n to implement any codes that are not used by git.\n-\n- TODO: K\n */\n \n static HANDLE console;\n@@ -79,6 +77,20 @@ static void set_console_attr(void)\n \tSetConsoleTextAttribute(console, attributes);\n }\n \n+static void erase_in_line(void)\n+{\n+\tCONSOLE_SCREEN_BUFFER_INFO sbi;\n+\n+\tif (!console)\n+\t\treturn;\n+\n+\tGetConsoleScreenBufferInfo(console, &sbi);\n+\tFillConsoleOutputCharacterA(console, ' ',\n+\t\tsbi.dwSize.X - sbi.dwCursorPosition.X, sbi.dwCursorPosition,\n+\t\tNULL);\n+}\n+\n+\n static const char *set_attr(const char *str)\n {\n \tconst char *func;\n@@ -218,7 +230,7 @@ static const char *set_attr(const char *str)\n \t\tset_console_attr();\n \t\tbreak;\n \tcase 'K':\n-\t\t/* TODO */\n+\t\terase_in_line();\n \t\tbreak;\n \tdefault:\n \t\t/* Unsupported code */\n-- \n1.6.2.103.gb3eac.dirty\n"},{"id":"107620","messageId":"7viqmh7wvg.fsf@gitster.siamese.dyndns.org","threadId":"18241","inReplyTo":"49B64ADC.2090406@viscovery.net","subject":"Re: [PATCH/RFC] recv_sideband: Band #2 always goes to stderr","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-03-10T23:59:15Z","receivedAt":"2009-03-10T23:59:15Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Sixt writes:\n\n> All data producers and data consumers *in git* use band #2 to transport\n> error messages and progress report.\n\nCorrect; I wrote that in the big comment at the beginning of sideband.c.\n\nOf course we could enhance the protocol to use more bands as needed. #2\nand #3 have their own meanings and they both map well to \"send to stderr\".\n"},{"id":"107665","messageId":"alpine.DEB.1.00.0903111122030.10279@pacific.mpi-cbg.de","threadId":"18241","inReplyTo":"200903102258.09521.j6t@kdbg.org","subject":"Re: [PATCH 2/2] winansi: support ESC [ K (erase in line)","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2009-03-11T10:22:22Z","receivedAt":"2009-03-11T10:22:22Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Tue, 10 Mar 2009, Johannes Sixt wrote:\n\n> From: Johannes Schindelin \n> \n> Signed-off-by: Johannes Schindelin \n> Signed-off-by: Johannes Sixt \n> ---\n> This is your patch without the write() part.\n\nThanks! All of a sudden, this patch does not look hacky anymore...\n\nCiao,\nDscho\n"}]}