{"thread":{"id":"42732","subject":"[PATCH] Remove obsolete comment from color.h","startedAt":"2016-06-28T10:03:12Z","lastAt":"2016-06-28T16:46:08Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"290357","messageId":"16f3d3edb03c2cb9a6c11b745eda9fb2274af182.1467108142.git.johannes.schindelin@gmx.de","threadId":"42732","inReplyTo":null,"subject":"[PATCH] Remove obsolete comment from color.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-28T10:02:49Z","receivedAt":"2016-06-28T10:03:12Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Originally, ANSI color sequences were supported on Windows only by\noverriding the printf() and fprintf() functions, as mentioned in e7821d7\n(Add a notice that only certain functions can print color escape codes,\n2009-11-27).\n\nAs of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),\nhowever, this is no longer the case, as the ANSI color sequence support\ncode needed to be replaced with a thread-safe version, one side effect\nbeing that stdout and stderr handled no matter which function is used to\nwrite to it.\n\nSo let's just remove the comment that is now obsolete.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\nPublished-As: https://github.com/dscho/git/releases/tag/winansi-color-v1\n color.h | 5 -----\n 1 file changed, 5 deletions(-)\n\ndiff --git a/color.h b/color.h\nindex e155d13..b9ead16 100644\n--- a/color.h\n+++ b/color.h\n@@ -18,11 +18,6 @@ struct strbuf;\n  */\n #define COLOR_MAXLEN 70\n \n-/*\n- * IMPORTANT: Due to the way these color codes are emulated on Windows,\n- * write them only using printf(), fprintf(), and fputs(). In particular,\n- * do not use puts() or write().\n- */\n #define GIT_COLOR_NORMAL\t\"\"\n #define GIT_COLOR_RESET\t\t\"\\033[m\"\n #define GIT_COLOR_BOLD\t\t\"\\033[1m\"\n-- \n2.9.0.118.g0e1a633\n\nbase-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43\n"},{"id":"290385","messageId":"xmqqinwt9v6e.fsf@gitster.mtv.corp.google.com","threadId":"42732","inReplyTo":"16f3d3edb03c2cb9a6c11b745eda9fb2274af182.1467108142.git.johannes.schindelin@gmx.de","subject":"Re: [PATCH] Remove obsolete comment from color.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-28T15:42:33Z","receivedAt":"2016-06-28T15:43:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n\n> Originally, ANSI color sequences were supported on Windows only by\n> overriding the printf() and fprintf() functions, as mentioned in e7821d7\n> (Add a notice that only certain functions can print color escape codes,\n> 2009-11-27).\n>\n> As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),\n> however, this is no longer the case, as the ANSI color sequence support\n> code needed to be replaced with a thread-safe version, one side effect\n> being that stdout and stderr handled no matter which function is used to\n> write to it.\n\nSo as long as we write via stdio to stdout/stderr, you can show\ncolors?  Or is it now stronger, in that as long as we do anything\nthat ends up writing to file descriptors 1 or 2, you can show\ncolors?\n\n> So let's just remove the comment that is now obsolete.\n\nThanks.\n\n> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n> ---\n> Published-As: https://github.com/dscho/git/releases/tag/winansi-color-v1\n>  color.h | 5 -----\n>  1 file changed, 5 deletions(-)\n>\n> diff --git a/color.h b/color.h\n> index e155d13..b9ead16 100644\n> --- a/color.h\n> +++ b/color.h\n> @@ -18,11 +18,6 @@ struct strbuf;\n>   */\n>  #define COLOR_MAXLEN 70\n>  \n> -/*\n> - * IMPORTANT: Due to the way these color codes are emulated on Windows,\n> - * write them only using printf(), fprintf(), and fputs(). In particular,\n> - * do not use puts() or write().\n> - */\n>  #define GIT_COLOR_NORMAL\t\"\"\n>  #define GIT_COLOR_RESET\t\t\"\\033[m\"\n>  #define GIT_COLOR_BOLD\t\t\"\\033[1m\"\n"},{"id":"290390","messageId":"alpine.DEB.2.20.1606281822240.12947@virtualbox","threadId":"42732","inReplyTo":"xmqqinwt9v6e.fsf@gitster.mtv.corp.google.com","subject":"Re: [PATCH] Remove obsolete comment from color.h","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2016-06-28T16:24:05Z","receivedAt":"2016-06-28T16:24:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Tue, 28 Jun 2016, Junio C Hamano wrote:\n\n> Johannes Schindelin <johannes.schindelin@gmx.de> writes:\n> \n> > Originally, ANSI color sequences were supported on Windows only by\n> > overriding the printf() and fprintf() functions, as mentioned in e7821d7\n> > (Add a notice that only certain functions can print color escape codes,\n> > 2009-11-27).\n> >\n> > As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),\n> > however, this is no longer the case, as the ANSI color sequence support\n> > code needed to be replaced with a thread-safe version, one side effect\n> > being that stdout and stderr handled no matter which function is used to\n> > write to it.\n> \n> So as long as we write via stdio to stdout/stderr, you can show\n> colors?  Or is it now stronger, in that as long as we do anything\n> that ends up writing to file descriptors 1 or 2, you can show\n> colors?\n\nIt means that we override stdout/stderr with a pipe that is parsed for the\ncolor sequences. If stdout or stderr are reopened later, this color\nhandling will not apply to the new file descriptor/stream.\n\nEssentially, the caveat in color.h that one should use fprintf() and\nprintf() when outputting color sequences to a terminal no longer holds\ntrue. fwrite() or write() work just as well.\n\nCiao,\nDscho\n"},{"id":"290393","messageId":"CAPc5daXVxNtx1HTYBToSds92X37Ns4yd3w2hxH08ocJLre8LrA@mail.gmail.com","threadId":"42732","inReplyTo":"alpine.DEB.2.20.1606281822240.12947@virtualbox","subject":"Re: [PATCH] Remove obsolete comment from color.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2016-06-28T16:36:50Z","receivedAt":"2016-06-28T16:37:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"On Tue, Jun 28, 2016 at 9:24 AM, Johannes Schindelin\n<Johannes.Schindelin@gmx.de> wrote:\n>\n> On Tue, 28 Jun 2016, Junio C Hamano wrote:\n>>\n>> So as long as we write via stdio to stdout/stderr, you can show\n>> colors?  Or is it now stronger, in that as long as we do anything\n>> that ends up writing to file descriptors 1 or 2, you can show\n>> colors?\n>\n> Essentially, the caveat in color.h that one should use fprintf() and\n> printf() when outputting color sequences to a terminal no longer holds\n> true. fwrite() or write() work just as well.\n\nIn short, the answer is \"the latter\"? That is a great news. It means that\nLukas can use write(2) in the recv_sideband() patch to ensure atomicity,\nwithout having to rely on the observed behaviour of fprintf(stderr, \"%s\", ...)\nthat as long as the output is within reasonable size the call seems to\nresult in a single write(2).\n\nThanks for a clarification.\n"},{"id":"290394","messageId":"20160628164555.GA13821@sigill.intra.peff.net","threadId":"42732","inReplyTo":"alpine.DEB.2.20.1606281822240.12947@virtualbox","subject":"Re: [PATCH] Remove obsolete comment from color.h","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2016-06-28T16:45:55Z","receivedAt":"2016-06-28T16:46:08Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jun 28, 2016 at 06:24:05PM +0200, Johannes Schindelin wrote:\n\n> Essentially, the caveat in color.h that one should use fprintf() and\n> printf() when outputting color sequences to a terminal no longer holds\n> true. fwrite() or write() work just as well.\n\nCool. One less thing to worry about.\n\nThanks for digging into this.\n\n-Peff\n"}]}