[PATCH] Remove obsolete comment from color.h

[PATCH] Remove obsolete comment from color.h

[Johannes Schindelin]

Originally, ANSI color sequences were supported on Windows only by
overriding the printf() and fprintf() functions, as mentioned in e7821d7
(Add a notice that only certain functions can print color escape codes,
2009-11-27).

As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),
however, this is no longer the case, as the ANSI color sequence support
code needed to be replaced with a thread-safe version, one side effect
being that stdout and stderr handled no matter which function is used to
write to it.

So let's just remove the comment that is now obsolete.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
Published-As: https://github.com/dscho/git/releases/tag/winansi-color-v1
 color.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/color.h b/color.h
index e155d13..b9ead16 100644
--- a/color.h
+++ b/color.h
@@ -18,11 +18,6 @@ struct strbuf;
  */
 #define COLOR_MAXLEN 70
 
-/*
- * IMPORTANT: Due to the way these color codes are emulated on Windows,
- * write them only using printf(), fprintf(), and fputs(). In particular,
- * do not use puts() or write().
- */
 #define GIT_COLOR_NORMAL	""
 #define GIT_COLOR_RESET		"\033[m"
 #define GIT_COLOR_BOLD		"\033[1m"
-- 
2.9.0.118.g0e1a633

base-commit: cf4c2cfe52be5bd973a4838f73a35d3959ce2f43

Re: [PATCH] Remove obsolete comment from color.h

[Junio C Hamano]

Johannes Schindelin <johannes.schindelin@gmx.de> writes:

> Originally, ANSI color sequences were supported on Windows only by
> overriding the printf() and fprintf() functions, as mentioned in e7821d7
> (Add a notice that only certain functions can print color escape codes,
> 2009-11-27).
>
> As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),
> however, this is no longer the case, as the ANSI color sequence support
> code needed to be replaced with a thread-safe version, one side effect
> being that stdout and stderr handled no matter which function is used to
> write to it.

So as long as we write via stdio to stdout/stderr, you can show
colors?  Or is it now stronger, in that as long as we do anything
that ends up writing to file descriptors 1 or 2, you can show
colors?

> So let's just remove the comment that is now obsolete.

Thanks.

> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
> Published-As: https://github.com/dscho/git/releases/tag/winansi-color-v1
>  color.h | 5 -----
>  1 file changed, 5 deletions(-)
>
> diff --git a/color.h b/color.h
> index e155d13..b9ead16 100644
> --- a/color.h
> +++ b/color.h
> @@ -18,11 +18,6 @@ struct strbuf;
>   */
>  #define COLOR_MAXLEN 70
>  
> -/*
> - * IMPORTANT: Due to the way these color codes are emulated on Windows,
> - * write them only using printf(), fprintf(), and fputs(). In particular,
> - * do not use puts() or write().
> - */
>  #define GIT_COLOR_NORMAL	""
>  #define GIT_COLOR_RESET		"\033[m"
>  #define GIT_COLOR_BOLD		"\033[1m"

Re: [PATCH] Remove obsolete comment from color.h

[Johannes Schindelin]

Hi Junio,

On Tue, 28 Jun 2016, Junio C Hamano wrote:

> Johannes Schindelin <johannes.schindelin@gmx.de> writes:
> 
> > Originally, ANSI color sequences were supported on Windows only by
> > overriding the printf() and fprintf() functions, as mentioned in e7821d7
> > (Add a notice that only certain functions can print color escape codes,
> > 2009-11-27).
> >
> > As of eac14f8 (Win32: Thread-safe windows console output, 2012-01-14),
> > however, this is no longer the case, as the ANSI color sequence support
> > code needed to be replaced with a thread-safe version, one side effect
> > being that stdout and stderr handled no matter which function is used to
> > write to it.
> 
> So as long as we write via stdio to stdout/stderr, you can show
> colors?  Or is it now stronger, in that as long as we do anything
> that ends up writing to file descriptors 1 or 2, you can show
> colors?

It means that we override stdout/stderr with a pipe that is parsed for the
color sequences. If stdout or stderr are reopened later, this color
handling will not apply to the new file descriptor/stream.

Essentially, the caveat in color.h that one should use fprintf() and
printf() when outputting color sequences to a terminal no longer holds
true. fwrite() or write() work just as well.

Ciao,
Dscho

Re: [PATCH] Remove obsolete comment from color.h

[Junio C Hamano]

On Tue, Jun 28, 2016 at 9:24 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
>
> On Tue, 28 Jun 2016, Junio C Hamano wrote:
>>
>> So as long as we write via stdio to stdout/stderr, you can show
>> colors?  Or is it now stronger, in that as long as we do anything
>> that ends up writing to file descriptors 1 or 2, you can show
>> colors?
>
> Essentially, the caveat in color.h that one should use fprintf() and
> printf() when outputting color sequences to a terminal no longer holds
> true. fwrite() or write() work just as well.

In short, the answer is "the latter"? That is a great news. It means that
Lukas can use write(2) in the recv_sideband() patch to ensure atomicity,
without having to rely on the observed behaviour of fprintf(stderr, "%s", ...)
that as long as the output is within reasonable size the call seems to
result in a single write(2).

Thanks for a clarification.

Re: [PATCH] Remove obsolete comment from color.h

[Jeff King]

On Tue, Jun 28, 2016 at 06:24:05PM +0200, Johannes Schindelin wrote:

> Essentially, the caveat in color.h that one should use fprintf() and
> printf() when outputting color sequences to a terminal no longer holds
> true. fwrite() or write() work just as well.

Cool. One less thing to worry about.

Thanks for digging into this.

-Peff