[PATCH][mingw] Enable colorized diagnostics

[PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

[-- Attachment #1: Type: text/plain, Size: 3217 bytes --]

Hello,

(I don't have SVN write access. So please apply this patch if you think 
it is OK. I have got FSF's copyright assignment paper for GCC. I will 
send you a copy of it when required.)

Colorized diagnostics used to be disabled for MinGW targets (on which 
the macro `_WIN32` is defined), and this patch enables it.

At the moment, GCC colorizes diagnostics using ANSI escape codes. Unlike 
most other terminal devices or simulators, Windows consoles did not 
recognize ANSI escape codes until Windows 10 "Threshold 2" in 2016, and 
this capability is not enabled by default thus has to be turned on 
explicitly sometime.

In reality, Windows consoles do support 
coloring/highlighting/underlining characters, moving the cursor or 
filling (erasing) screen areas. However the individual text attributes 
must be set or unset using functions provided by the system. Here comes 
my solution:

Normally, GCC outputs the diagnostic messages, infused with 
miscellaneous formatting, using the standard `fputs()` function. For 
Windows consoles we use a new function, `mingw_ansi_fputs()` to output 
such messages. This function strips ANSI escape codes, interprets them, 
orchestrates the modes as requested, then outputs the text remaining. 
Unimplemented codes are silently ignored.

I have successfully bootstrapped GCC for `i686-w64-mingw32` and 
`x86_64-w64-mingw32` with this patch enabled. Automated tests are not an 
option because all this patch enables are visual effects that are not 
unreadable by scripts.

`check_GNU_style.sh` spits some false positives about the comments and a 
line of non-conforming comment in the original file, which I think can 
be ignored.

* * * * * * * * * *

KNOWN ISSUES

1. Unlike `fputs()`, `mingw_ansi_fputs()` is no longer atomic. When 
multiple processes are outputting text to the same console, messages 
could interleave with each other. There is no simple solution to this 
issue. The atomicity of stdio functions is mandatory according to ISO 
C11, which was not the case in C99. The `fputs()` function from 
MinGW-w64 suffers from the same problem, but the one from MSVCRT does not.

2. `mingw_ansi_fputs()` is eventually compiled into `libcommon.a`. 
Personally, I think it is a bad idea to put target-specific code in it, 
notwithstanding `#if ... #endif` guards that has been went over 
carefully. I could have implemented this function in a separated 
target-specific file, which, actually, results in only undefined 
references, since all target-specific object files precede `libcommon.a` 
in the final linker command line.

* * * * * * * * * *

2017-09-28  Liu Hao  <lh_mouse@126.com>

	* gcc/pretty-print.c [_WIN32] (colorize_init): Remove.  Use
	the generic version below instead.
	(should_colorize): Recognize Windows consoles as terminals
	for MinGW targets.
	* gcc/pretty-print.c [__MINGW32__] (write_all): New function.
	[__MINGW32__] (find_esc_head): Likewise.
	[__MINGW32__] (find_esc_terminator): Likewise.
	[__MINGW32__] (eat_esc_sequence): Likewise.
	[__MINGW32__] (mingw_ansi_fputs): New function that handles
	ANSI escape codes.
	(pp_write_text_to_stream): Use mingw_ansi_fputs instead of fputs
	for MinGW targets.



-- 
Best regards,
LH_Mouse

[-- Attachment #2: 0001-Enable-a-native-GCC-to-color-diagnostic-messages-sen.patch --]
[-- Type: text/plain, Size: 19215 bytes --]

From 6a02e27b10a20ead725391804610bfca6d9c1e06 Mon Sep 17 00:00:00 2001
From: Liu Hao <lh_mouse@126.com>
Date: Thu, 7 Sep 2017 12:50:07 +0800
Subject: [PATCH] Enable a native GCC to color diagnostic messages sent over
 Windows consoles.

---
 gcc/diagnostic-color.c |  28 ++-
 gcc/pretty-print.c     | 664 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 682 insertions(+), 10 deletions(-)

diff --git a/gcc/diagnostic-color.c b/gcc/diagnostic-color.c
index 6adb872146b..b8cf6f2c045 100644
--- a/gcc/diagnostic-color.c
+++ b/gcc/diagnostic-color.c
@@ -20,6 +20,10 @@
 #include "system.h"
 #include "diagnostic-color.h"
 
+#ifdef __MINGW32__
+#  include <windows.h>
+#endif
+
 /* Select Graphic Rendition (SGR, "\33[...m") strings.  */
 /* Also Erase in Line (EL) to Right ("\33[K") by default.  */
 /*    Why have EL to Right after SGR?
@@ -275,23 +279,28 @@ parse_gcc_colors (void)
       return true;
 }
 
-#if defined(_WIN32)
-bool
-colorize_init (diagnostic_color_rule_t)
-{
-  return false;
-}
-#else
-
 /* Return true if we should use color when in auto mode, false otherwise. */
 static bool
 should_colorize (void)
 {
+#ifdef __MINGW32__
+  /* For consistency reasons, one should check the handle returned by
+     _get_osfhandle(_fileno(stderr)) because the function
+     pp_write_text_to_stream() in pretty-print.c calls fputs() on
+     that stream.  However, the code below for non-Windows doesn't seem
+     to care about it either...  */
+  HANDLE h;
+  DWORD m;
+
+  h = GetStdHandle (STD_ERROR_HANDLE);
+  return (h != INVALID_HANDLE_VALUE) && (h != NULL)
+	  && GetConsoleMode (h, &m);
+#else
   char const *t = getenv ("TERM");
   return t && strcmp (t, "dumb") != 0 && isatty (STDERR_FILENO);
+#endif
 }
 
-
 bool
 colorize_init (diagnostic_color_rule_t rule)
 {
@@ -310,4 +319,3 @@ colorize_init (diagnostic_color_rule_t rule)
       gcc_unreachable ();
     }
 }
-#endif
diff --git a/gcc/pretty-print.c b/gcc/pretty-print.c
index 7340cd4a565..f33d59370ae 100644
--- a/gcc/pretty-print.c
+++ b/gcc/pretty-print.c
@@ -30,6 +30,666 @@ along with GCC; see the file COPYING3.  If not see
 #include <iconv.h>
 #endif
 
+#ifdef __MINGW32__
+
+/* Replacement for fputs() that handles ANSI escape codes on Windows NT.
+   Contributed by: Liu Hao (lh_mouse at 126 dot com)
+
+   XXX: This file is compiled into libcommon.a that will be self-contained.
+	It looks like that these functions can be put nowhere else.  */
+
+#include <io.h>
+#define WIN32_LEAN_AND_MEAN 1
+#include <windows.h>
+
+/* Write all bytes in [s,s+n) into the specified stream.
+   Errors are ignored.  */
+static void
+write_all (HANDLE h, const char *s, size_t n)
+{
+  size_t rem = n;
+  DWORD step;
+
+  while (rem != 0)
+    {
+      if (rem <= UINT_MAX)
+	step = rem;
+      else
+	step = UINT_MAX;
+      if (!WriteFile (h, s + n - rem, step, &step, NULL))
+	break;
+      rem -= step;
+    }
+}
+
+/* Find the beginning of an escape sequence.
+   There are two cases:
+   1. If the sequence begins with an ESC character (0x1B) and a second
+      character X in [0x40,0x5F], returns X and stores a pointer to
+      the third character into *head.
+   2. If the sequence begins with a character X in [0x80,0x9F], returns
+      (X-0x40) and stores a pointer to the second character into *head.
+   Stores the number of ESC character(s) in *prefix_len.
+   Returns 0 if no such sequence can be found.  */
+static int
+find_esc_head (int *prefix_len, const char **head, const char *str)
+{
+  int c;
+  const char *r = str;
+  int escaped = 0;
+
+  for (;;)
+    {
+      c = (unsigned char) *r;
+      if (c == 0)
+	{
+	  /* Not found.  */
+	  return 0;
+	}
+      if (escaped && 0x40 <= c && c <= 0x5F)
+	{
+	  /* Found (case 1).  */
+	  *prefix_len = 2;
+	  *head = r + 1;
+	  return c;
+	}
+      if (0x80 <= c && c <= 0x9F)
+	{
+	  /* Found (case 2).  */
+	  *prefix_len = 1;
+	  *head = r + 1;
+	  return c - 0x40;
+	}
+      ++r;
+      escaped = c == 0x1B;
+    }
+}
+
+/* Find the terminator of an escape sequence.
+   str should be the value stored in *head by a previous successful
+   call to find_esc_head().
+   Returns -1 if no such sequence can be found.  */
+static int
+find_esc_terminator (const char **term, const char *str)
+{
+  int c;
+  const char *r = str;
+
+  for (;;)
+    {
+      c = (unsigned char) *r;
+      if (c == 0)
+	{
+	  /* Not found.  */
+	  return 0;
+	}
+      if (0x40 <= c && c <= 0x7E)
+	{
+	  /* Found.  */
+	  *term = r;
+	  return c;
+	}
+      ++r;
+    }
+}
+
+/* Handle a sequence of codes.  Sequences that are invalid, reserved,
+   unrecognized or unimplemented are ignored silently.
+   There isn't much we can do because of lameness of Windows consoles.  */
+static void
+eat_esc_sequence (HANDLE h, int esc_code,
+		  const char *esc_head, const char *esc_term)
+{
+  /* Numbers in an escape sequence cannot be negative, because
+     a minus sign in the middle of it would have terminated it.  */
+  long n1, n2;
+  char *eptr, *delim;
+  CONSOLE_SCREEN_BUFFER_INFO sb;
+  COORD cr;
+  /* ED and EL parameters.  */
+  DWORD cnt, step;
+  long rows;
+  /* SGR parameters.  */
+  WORD attrib_add, attrib_rm;
+  const char *param;
+
+  switch (MAKEWORD (esc_code, *esc_term))
+    {
+    /* ESC [ n1 'A'
+	 Move the cursor up by n1 characters.  */
+    case MAKEWORD ('[', 'A'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  /* Stop at the topmost boundary.  */
+	  if (cr.Y > n1)
+	    cr.Y -= n1;
+	  else
+	    cr.Y = 0;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'B'
+	 Move the cursor down by n1 characters.  */
+    case MAKEWORD ('[', 'B'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  /* Stop at the bottommost boundary.  */
+	  if (sb.dwSize.Y - cr.Y > n1)
+	    cr.Y += n1;
+	  else
+	    cr.Y = sb.dwSize.Y;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'C'
+	 Move the cursor right by n1 characters.  */
+    case MAKEWORD ('[', 'C'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  /* Stop at the rightmost boundary.  */
+	  if (sb.dwSize.X - cr.X > n1)
+	    cr.X += n1;
+	  else
+	    cr.X = sb.dwSize.X;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'D'
+	 Move the cursor left by n1 characters.  */
+    case MAKEWORD ('[', 'D'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  /* Stop at the leftmost boundary.  */
+	  if (cr.X > n1)
+	    cr.X -= n1;
+	  else
+	    cr.X = 0;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'E'
+	 Move the cursor to the beginning of the n1-th line downwards.  */
+    case MAKEWORD ('[', 'E'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  cr.X = 0;
+	  /* Stop at the bottommost boundary.  */
+	  if (sb.dwSize.Y - cr.Y > n1)
+	    cr.Y += n1;
+	  else
+	    cr.Y = sb.dwSize.Y;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'F'
+	 Move the cursor to the beginning of the n1-th line upwards.  */
+    case MAKEWORD ('[', 'F'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  cr.X = 0;
+	  /* Stop at the topmost boundary.  */
+	  if (cr.Y > n1)
+	    cr.Y -= n1;
+	  else
+	    cr.Y = 0;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'G'
+	 Move the cursor to the (1-based) n1-th column.  */
+    case MAKEWORD ('[', 'G'):
+      if (esc_head == esc_term)
+	n1 = 1;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  n1 -= 1;
+	  /* Stop at the leftmost or rightmost boundary.  */
+	  if (n1 < 0)
+	    cr.X = 0;
+	  else if (n1 > sb.dwSize.X)
+	    cr.X = sb.dwSize.X;
+	  else
+	    cr.X = n1;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 ';' n2 'H'
+       ESC [ n1 ';' n2 'f'
+	 Move the cursor to the (1-based) n1-th row and
+	 (also 1-based) n2-th column.  */
+    case MAKEWORD ('[', 'H'):
+    case MAKEWORD ('[', 'f'):
+      if (esc_head == esc_term)
+	{
+	  /* Both parameters are omitted and set to 1 by default.  */
+	  n1 = 1;
+	  n2 = 1;
+	}
+      else if (!(delim = (char *) memchr (esc_head, ';',
+					  esc_term - esc_head)))
+	{
+	  /* Only the first parameter is given.  The second one is
+	     set to 1 by default.  */
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	  n2 = 1;
+	}
+      else
+	{
+	  /* Both parameters are given.  The first one shall be
+	     terminated by the semicolon.  */
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != delim)
+	    break;
+	  n2 = strtol (delim + 1, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  cr = sb.dwCursorPosition;
+	  n1 -= 1;
+	  n2 -= 1;
+	  /* The cursor position shall be relative to the view coord of
+	     the console window, which is usually smaller than the actual
+	     buffer.  FWIW, the 'appropriate' solution will be shrinking
+	     the buffer to match the size of the console window,
+	     destroying scrollback in the process.  */
+	  n1 += sb.srWindow.Top;
+	  n2 += sb.srWindow.Left;
+	  /* Stop at the topmost or bottommost boundary.  */
+	  if (n1 < 0)
+	    cr.Y = 0;
+	  else if (n1 > sb.dwSize.Y)
+	    cr.Y = sb.dwSize.Y;
+	  else
+	    cr.Y = n1;
+	  /* Stop at the leftmost or rightmost boundary.  */
+	  if (n2 < 0)
+	    cr.X = 0;
+	  else if (n2 > sb.dwSize.X)
+	    cr.X = sb.dwSize.X;
+	  else
+	    cr.X = n2;
+	  SetConsoleCursorPosition (h, cr);
+	}
+      break;
+
+    /* ESC [ n1 'J'
+	 Erase display.  */
+    case MAKEWORD ('[', 'J'):
+      if (esc_head == esc_term)
+	/* This is one of the very few codes whose parameters have
+	   a default value of zero.  */
+	n1 = 0;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  /* The cursor is not necessarily in the console window, which
+	     makes the behavior of this code harder to define.  */
+	  switch (n1)
+	    {
+	    case 0:
+	      /* If the cursor is in or above the window, erase from
+		 it to the bottom of the window; otherwise, do nothing.  */
+	      cr = sb.dwCursorPosition;
+	      cnt = sb.dwSize.X - sb.dwCursorPosition.X;
+	      rows = sb.srWindow.Bottom - sb.dwCursorPosition.Y;
+	      break;
+	    case 1:
+	      /* If the cursor is in or under the window, erase from
+		 it to the top of the window; otherwise, do nothing.  */
+	      cr.X = 0;
+	      cr.Y = sb.srWindow.Top;
+	      cnt = sb.dwCursorPosition.X + 1;
+	      rows = sb.dwCursorPosition.Y - sb.srWindow.Top;
+	      break;
+	    case 2:
+	      /* Erase the entire window.  */
+	      cr.X = sb.srWindow.Left;
+	      cr.Y = sb.srWindow.Top;
+	      cnt = 0;
+	      rows = sb.srWindow.Bottom - sb.srWindow.Top + 1;
+	      break;
+	    default:
+	      /* Erase the entire buffer.  */
+	      cr.X = 0;
+	      cr.Y = 0;
+	      cnt = 0;
+	      rows = sb.dwSize.Y;
+	      break;
+	    }
+	  if (rows < 0)
+	    break;
+	  cnt += rows * sb.dwSize.X;
+	  FillConsoleOutputCharacterW (h, L' ', cnt, cr, &step);
+	  FillConsoleOutputAttribute (h, sb.wAttributes, cnt, cr, &step);
+	}
+      break;
+
+    /* ESC [ n1 'K'
+	 Erase line.  */
+    case MAKEWORD ('[', 'K'):
+      if (esc_head == esc_term)
+	/* This is one of the very few codes whose parameters have
+	   a default value of zero.  */
+	n1 = 0;
+      else
+	{
+	  n1 = strtol (esc_head, &eptr, 10);
+	  if (eptr != esc_term)
+	    break;
+	}
+
+      if (GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  switch (n1)
+	    {
+	    case 0:
+	      /* Erase from the cursor to the end.  */
+	      cr = sb.dwCursorPosition;
+	      cnt = sb.dwSize.X - sb.dwCursorPosition.X;
+	      break;
+	    case 1:
+	      /* Erase from the cursor to the beginning.  */
+	      cr = sb.dwCursorPosition;
+	      cr.X = 0;
+	      cnt = sb.dwCursorPosition.X + 1;
+	      break;
+	    default:
+	      /* Erase the entire line.  */
+	      cr = sb.dwCursorPosition;
+	      cr.X = 0;
+	      cnt = sb.dwSize.X;
+	      break;
+	    }
+	  FillConsoleOutputCharacterW (h, L' ', cnt, cr, &step);
+	  FillConsoleOutputAttribute (h, sb.wAttributes, cnt, cr, &step);
+	}
+      break;
+
+    /* ESC [ n1 ';' n2 'm'
+	 Set SGR parameters.  Zero or more parameters will follow.  */
+    case MAKEWORD ('[', 'm'):
+      attrib_add = 0;
+      attrib_rm = 0;
+      if (esc_head == esc_term)
+	{
+	  /* When no parameter is given, reset the console.  */
+	  attrib_add |= (FOREGROUND_RED | FOREGROUND_GREEN
+			 | FOREGROUND_BLUE);
+	  attrib_rm = -1; /* Removes everything.  */
+	  goto sgr_set_it;
+	}
+      param = esc_head;
+      do
+	{
+	  /* Parse a parameter.  */
+	  n1 = strtol (param, &eptr, 10);
+	  if (*eptr != ';' && eptr != esc_term)
+	    goto sgr_set_it;
+
+	  switch (n1)
+	    {
+	    case 0:
+	      /* Reset.  */
+	      attrib_add |= (FOREGROUND_RED | FOREGROUND_GREEN
+			     | FOREGROUND_BLUE);
+	      attrib_rm = -1; /* Removes everything.  */
+	      break;
+	    case 1:
+	      /* Bold.  */
+	      attrib_add |= FOREGROUND_INTENSITY;
+	      break;
+	    case 4:
+	      /* Underline.  */
+	      attrib_add |= COMMON_LVB_UNDERSCORE;
+	      break;
+	    case 5:
+	      /* Blink.  */
+	      /* XXX: It is not BLINKING at all! */
+	      attrib_add |= BACKGROUND_INTENSITY;
+	      break;
+	    case 7:
+	      /* Reverse.  */
+	      attrib_add |= COMMON_LVB_REVERSE_VIDEO;
+	      break;
+	    case 22:
+	      /* No bold.  */
+	      attrib_add &= ~FOREGROUND_INTENSITY;
+	      attrib_rm |= FOREGROUND_INTENSITY;
+	      break;
+	    case 24:
+	      /* No underline.  */
+	      attrib_add &= ~COMMON_LVB_UNDERSCORE;
+	      attrib_rm |= COMMON_LVB_UNDERSCORE;
+	      break;
+	    case 25:
+	      /* No blink.  */
+	      /* XXX: It is not BLINKING at all! */
+	      attrib_add &= ~BACKGROUND_INTENSITY;
+	      attrib_rm |= BACKGROUND_INTENSITY;
+	      break;
+	    case 27:
+	      /* No reverse.  */
+	      attrib_add &= ~COMMON_LVB_REVERSE_VIDEO;
+	      attrib_rm |= COMMON_LVB_REVERSE_VIDEO;
+	      break;
+	    case 30:
+	    case 31:
+	    case 32:
+	    case 33:
+	    case 34:
+	    case 35:
+	    case 36:
+	    case 37:
+	      /* Foreground color.  */
+	      attrib_add &= ~(FOREGROUND_RED | FOREGROUND_GREEN
+			      | FOREGROUND_BLUE);
+	      n1 -= 30;
+	      if (n1 & 1)
+		attrib_add |= FOREGROUND_RED;
+	      if (n1 & 2)
+		attrib_add |= FOREGROUND_GREEN;
+	      if (n1 & 4)
+		attrib_add |= FOREGROUND_BLUE;
+	      attrib_rm |= (FOREGROUND_RED | FOREGROUND_GREEN
+			    | FOREGROUND_BLUE);
+	      break;
+	    case 38:
+	      /* Reserved for extended foreground color.
+		 Don't know how to handle parameters remaining.
+		 Bail out.  */
+	      goto sgr_set_it;
+	    case 39:
+	      /* Reset foreground color.  */
+	      /* Set to grey.  */
+	      attrib_add |= (FOREGROUND_RED | FOREGROUND_GREEN
+			     | FOREGROUND_BLUE);
+	      attrib_rm |= (FOREGROUND_RED | FOREGROUND_GREEN
+			    | FOREGROUND_BLUE);
+	      break;
+	    case 40:
+	    case 41:
+	    case 42:
+	    case 43:
+	    case 44:
+	    case 45:
+	    case 46:
+	    case 47:
+	      /* Background color.  */
+	      attrib_add &= ~(BACKGROUND_RED | BACKGROUND_GREEN
+			      | BACKGROUND_BLUE);
+	      n1 -= 40;
+	      if (n1 & 1)
+		attrib_add |= BACKGROUND_RED;
+	      if (n1 & 2)
+		attrib_add |= BACKGROUND_GREEN;
+	      if (n1 & 4)
+		attrib_add |= BACKGROUND_BLUE;
+	      attrib_rm |= (BACKGROUND_RED | BACKGROUND_GREEN
+			    | BACKGROUND_BLUE);
+	      break;
+	    case 48:
+	      /* Reserved for extended background color.
+		 Don't know how to handle parameters remaining.
+		 Bail out.  */
+	      goto sgr_set_it;
+	    case 49:
+	      /* Reset background color.  */
+	      /* Set to black.  */
+	      attrib_add &= ~(BACKGROUND_RED | BACKGROUND_GREEN
+			      | BACKGROUND_BLUE);
+	      attrib_rm |= (BACKGROUND_RED | BACKGROUND_GREEN
+			    | BACKGROUND_BLUE);
+	      break;
+	    }
+
+	  /* Prepare the next parameter.  */
+	  param = eptr + 1;
+	}
+      while (param != esc_term);
+
+sgr_set_it:
+      /* 0xFFFF removes everything.  If it is not the case,
+	 care must be taken to preserve old attributes.  */
+      if (attrib_rm != 0xFFFF && GetConsoleScreenBufferInfo (h, &sb))
+	{
+	  attrib_add |= sb.wAttributes & ~attrib_rm;
+	}
+      SetConsoleTextAttribute (h, attrib_add);
+      break;
+    }
+}
+
+int
+mingw_ansi_fputs (const char *str, FILE *fp)
+{
+  const char *read = str;
+  HANDLE h;
+  DWORD mode;
+  int esc_code, prefix_len;
+  const char *esc_head, *esc_term;
+
+  h = (HANDLE) _get_osfhandle (_fileno (fp));
+  if (h == INVALID_HANDLE_VALUE)
+    return EOF;
+
+  /* Don't mess up stdio functions with Windows APIs.  */
+  fflush (fp);
+
+  if (GetConsoleMode (h, &mode))
+    /* If it is a console, translate ANSI escape codes as needed.  */
+    for (;;)
+      {
+	if ((esc_code = find_esc_head (&prefix_len, &esc_head, read)) == 0)
+	  {
+	    /* Write all remaining characters, then exit.  */
+	    write_all (h, read, strlen (read));
+	    break;
+	  }
+	if (find_esc_terminator (&esc_term, esc_head) == 0)
+	  /* Ignore incomplete escape sequences at the moment.
+	     FIXME: The escape state shall be cached for further calls
+		    to this function.  */
+	  break;
+	write_all (h, read, esc_head - prefix_len - read);
+	eat_esc_sequence (h, esc_code, esc_head, esc_term);
+	read = esc_term + 1;
+      }
+  else
+    /* If it is not a console, write everything as-is.  */
+    write_all (h, read, strlen (read));
+
+  _close ((intptr_t) h);
+  return 1;
+}
+
+#endif /* __MINGW32__ */
+
 static void pp_quoted_string (pretty_printer *, const char *, size_t = -1);
 
 /* Overwrite the given location/range within this text_info's rich_location.
@@ -140,7 +800,11 @@ void
 pp_write_text_to_stream (pretty_printer *pp)
 {
   const char *text = pp_formatted_text (pp);
+#ifdef __MINGW32__
+  mingw_ansi_fputs (text, pp_buffer (pp)->stream);
+#else
   fputs (text, pp_buffer (pp)->stream);
+#endif
   pp_clear_output_area (pp);
 }
 
-- 
2.14.1

Re: [PATCH][mingw] Enable colorized diagnostics

[Joseph Myers]

On Thu, 28 Sep 2017, Liu Hao wrote:

> Colorized diagnostics used to be disabled for MinGW targets (on which 
> the macro `_WIN32` is defined), and this patch enables it.

I'd hope this is all to do with MinGW host, and nothing to do with the 
target.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/9/28 4:09, Joseph Myers wrote:
> On Thu, 28 Sep 2017, Liu Hao wrote:
> 
>> Colorized diagnostics used to be disabled for MinGW targets (on which
>> the macro `_WIN32` is defined), and this patch enables it.
> 
> I'd hope this is all to do with MinGW host, and nothing to do with the
> target.
> 
Oh you are right. Since I build native compilers, host == target here. 
But strictly speaking the patch applies to the host, not the target.

-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[JonY]

[-- Attachment #1.1: Type: text/plain, Size: 576 bytes --]

On 09/27/2017 08:54 PM, Liu Hao wrote:
> On 2017/9/28 4:09, Joseph Myers wrote:
>> On Thu, 28 Sep 2017, Liu Hao wrote:
>>
>>> Colorized diagnostics used to be disabled for MinGW targets (on which
>>> the macro `_WIN32` is defined), and this patch enables it.
>>
>> I'd hope this is all to do with MinGW host, and nothing to do with the
>> target.
>>
> Oh you are right. Since I build native compilers, host == target here.
> But strictly speaking the patch applies to the host, not the target.
> 

Does it make sense to use a global lock in mingw_ansi_fputs?


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/9/28 7:37, JonY wrote:
> Does it make sense to use a global lock in mingw_ansi_fputs?
> 

I was thinking about a named Mutex object. Named Mutexes (as well as 
Events and Semaphores) can be shared across processes, but there are 
other considerations:

1. The name of the Mutex should base on the current console window which 
is shared by all child processes created by `make`, and must be unique. 
How can it be? Is it possible to create a string basing on the window 
handle or unique identifier whatsoever? Will the handle or unique id be 
reused after the window is destroyed? Is it unique after all?

2. This Mutex would only protect diagnostics from interleaving. 
Diagnostics can interleave with other messages written via stdio 
functions, including those written to `stdout` which is often output to 
the console as well. I don't think there are any solutions for this.

-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/9/28 4:09, Joseph Myers wrote:
> On Thu, 28 Sep 2017, Liu Hao wrote:
> 
>> Colorized diagnostics used to be disabled for MinGW targets (on which
>> the macro `_WIN32` is defined), and this patch enables it.
> 
> I'd hope this is all to do with MinGW host, and nothing to do with the
> target.
> 
Ping? Are there any more opinions about this?

-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/10/8 19:55, Hannes Domani wrote:
> So why not just enable it on Win10?

It is up to you, GCC maintainers. If dropping support for Windows prior 
to Windows 10 TH2 is an option, I may provide another patch, which I 
can't test because I primarily work on Windows 7. XD

-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/10/8 20:24, Hannes Domani wrote:
> Am Sonntag, 8. Oktober 2017, 14:02:48 MESZ hat Liu Hao 
> <lh_mouse@126.com> Folgendes geschrieben:
> 
>  > On 2017/10/8 19:55, Hannes Domani wrote:
>  >
>  > > So why not just enable it on Win10?
>  >
>  >
>  > It is up to you, GCC maintainers. If dropping support for Windows prior
>  > to Windows 10 TH2 is an option, I may provide another patch, which I
>  > can't test because I primarily work on Windows 7. XD
> 
> That wouldn't mean dropping support for earlier Windows.
> If enabling of ENABLE_VIRTUAL_TERMINAL_PROCESSING fails, just disable 
> the colored output.
It is exactly what I meant. I really, really want it on Windows 7, 
nevertheless.

-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[JonY]

[-- Attachment #1.1: Type: text/plain, Size: 652 bytes --]

On 10/08/2017 11:39 AM, Liu Hao wrote:
> On 2017/9/28 4:09, Joseph Myers wrote:
>> On Thu, 28 Sep 2017, Liu Hao wrote:
>>
>>> Colorized diagnostics used to be disabled for MinGW targets (on which
>>> the macro `_WIN32` is defined), and this patch enables it.
>>
>> I'd hope this is all to do with MinGW host, and nothing to do with the
>> target.
>>
> Ping? Are there any more opinions about this?
> 

I'm not sure if it should be enabled by default due to the interleaving
problem, but seeing as the user has to go out to set GCC_COLORS to use
this feature, I suppose it is OK.

I will commit soon if there are no more comments.



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/10/9 19:01, JonY wrote:
> On 10/08/2017 11:39 AM, Liu Hao wrote:
> 
> I'm not sure if it should be enabled by default due to the interleaving
> problem, but seeing as the user has to go out to set GCC_COLORS to use
> this feature, I suppose it is OK.
> 
> I will commit soon if there are no more comments.
> 
> 

Thank you. By the way I noticed a mistake in the comments above 
`find_esc_terminator()`. This function returns zero on failure like its 
`find_esc_head()` counterpart, while the comments mistakenly referred 
-1. Please correct it before committing.


-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[JonY]

[-- Attachment #1.1: Type: text/plain, Size: 709 bytes --]

On 10/09/2017 01:07 PM, Liu Hao wrote:
> On 2017/10/9 19:01, JonY wrote:
>> On 10/08/2017 11:39 AM, Liu Hao wrote:
>>
>> I'm not sure if it should be enabled by default due to the interleaving
>> problem, but seeing as the user has to go out to set GCC_COLORS to use
>> this feature, I suppose it is OK.
>>
>> I will commit soon if there are no more comments.
>>
>>
> 
> Thank you. By the way I noticed a mistake in the comments above
> `find_esc_terminator()`. This function returns zero on failure like its
> `find_esc_head()` counterpart, while the comments mistakenly referred
> -1. Please correct it before committing.
> 
> 

Committed to trunk r253645 with the appropriate change.


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 858 bytes --]

Re: [PATCH][mingw] Enable colorized diagnostics

[David Malcolm]

On Mon, 2017-10-09 at 11:01 +0000, JonY wrote:
> On 10/08/2017 11:39 AM, Liu Hao wrote:
> > On 2017/9/28 4:09, Joseph Myers wrote:
> > > On Thu, 28 Sep 2017, Liu Hao wrote:
> > > 
> > > > Colorized diagnostics used to be disabled for MinGW targets (on
> > > > which
> > > > the macro `_WIN32` is defined), and this patch enables it.
> > > 
> > > I'd hope this is all to do with MinGW host, and nothing to do
> > > with the
> > > target.
> > > 
> > 
> > Ping? Are there any more opinions about this?
> > 
> 
> I'm not sure if it should be enabled by default due to the
> interleaving
> problem, but seeing as the user has to go out to set GCC_COLORS to
> use
> this feature, I suppose it is OK.
> 
> I will commit soon if there are no more comments.

I have some concerns about adding non-trivial host-specific code to the
diagnostics subsystem.

I occasionally make changes to the files you're touching, but I don't
have access to the host in question, so I can't test that I don't break
things on MinGW.

Is it OK if this is the MinGW team's problem, and not mine?  (i.e. can
you please clean up after me if I break something on MinGW?).

Also, you might want to add some selftests to the code e.g. for
find_esc_head and find_esc_terminator.  I looked at the docs for the
Windows console API and unfortunately there doesn't seem to be a way to
set up a dummy console for unit-testing the parts of the code that call
the console API directly.  But find_esc_head and find_esc_terminator
don't directly call the API, and hence you can write some selftest
functions for them.  (there's probably a much more involved way to test
this using mocks/stubs for the console API, but that's probably
overkill).

Dave

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/10/9 22:16, David Malcolm wrote:
> I have some concerns about adding non-trivial host-specific code to the
> diagnostics subsystem.
> 
> I occasionally make changes to the files you're touching, but I don't
> have access to the host in question, so I can't test that I don't break
> things on MinGW.
> 
> Is it OK if this is the MinGW team's problem, and not mine?  (i.e. can
> you please clean up after me if I break something on MinGW?).
> 
I am tracing the branch for the latest stable major release 
(`gcc-7-branch` at the moment) closely. If anything is broken I will let 
you know.

> Also, you might want to add some selftests to the code e.g. for
> find_esc_head and find_esc_terminator.  
Those functions are static - they will not be visible elsewhere. They 
are formulated for clarification purposes and nothing else.

> I looked at the docs for the
> Windows console API and unfortunately there doesn't seem to be a way to
> set up a dummy console for unit-testing the parts of the code that call
> the console API directly.  But find_esc_head and find_esc_terminator
> don't directly call the API, and hence you can write some selftest
> functions for them.  (there's probably a much more involved way to test
> this using mocks/stubs for the console API, but that's probably
> overkill).
> 
> Dave
> 



-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[Manuel López-Ibáñez]

On 08/10/17 12:39, Liu Hao wrote:
> On 2017/9/28 4:09, Joseph Myers wrote:
>> On Thu, 28 Sep 2017, Liu Hao wrote:
>>
>>> Colorized diagnostics used to be disabled for MinGW targets (on which
>>> the macro `_WIN32` is defined), and this patch enables it.
>>
>> I'd hope this is all to do with MinGW host, and nothing to do with the
>> target.
>>
> Ping? Are there any more opinions about this?

For what is worth, the color output of GCC comes originally from grep, and grep 
does have code for colorizing in Windows: 
http://git.savannah.gnu.org/cgit/grep.git/tree/lib

and there are significant differences with this patch. For once,

   /* $TERM is not normally defined on DOS/Windows, so don't require
      it for highlighting.  But some programs, like Emacs, do define
      it when running Grep as a subprocess, so make sure they don't
      set TERM=dumb.  */
   char const *t = getenv ("TERM");
   return ! (t && strcmp (t, "dumb") == 0);

and they don't need a custom fputs() because their strategy is slightly 
different: They only override colorize_start (print_start_colorize) and 
colorize_stop (print_end_colorize) and convert ANSI sequences to W32 sequences 
on the fly. Thus, we wouldn't need to touch pretty-printer.c and all changes 
will be restricted to diagnostic-color.c (which could be split into -posix.c 
and -w32.c like grep does and be moved into host-specific config/ subdir).

Even if the host-specific part is not done, I honestly think it is a good idea 
to match grep's code as much as possible since we may want to merge bugfixes 
between the two and eventually this code may end up in gnulib. Moreover, if 
somebody else implemented color output for another OS in grep, it would be very 
easy to transplant it to GCC (or viceversa) if the API remains close.

Cheers,

	Manuel.

Re: [PATCH][mingw] Enable colorized diagnostics

[Manuel López-Ibáñez]

[-- Attachment #1: Type: text/plain, Size: 611 bytes --]

On 09/10/17 23:25, Manuel López-Ibáñez wrote:
> Even if the host-specific part is not done, I honestly think it is a good idea 
> to match grep's code as much as possible since we may want to merge bugfixes 
> between the two and eventually this code may end up in gnulib. Moreover, if 
> somebody else implemented color output for another OS in grep, it would be very 
> easy to transplant it to GCC (or viceversa) if the API remains close.

Something like the attached should do the trick (I didn't even try to compile 
it and completely untested, so it may need some adjustments).

Cheers,

Manuel.

[-- Attachment #2: diagnostic-color.diff --]
[-- Type: text/x-patch, Size: 7454 bytes --]

Index: diagnostic-color.c
===================================================================
--- diagnostic-color.c	(revision 253569)
+++ diagnostic-color.c	(working copy)
@@ -19,6 +19,12 @@
 #include "config.h"
 #include "system.h"
 #include "diagnostic-color.h"
+#ifdef __MINGW32__
+#  undef DATADIR /* conflicts with objidl.h, which is included by windows.h */
+#  include <windows.h>
+static HANDLE hstderr = INVALID_HANDLE_VALUE;
+static SHORT norm_attr;
+#endif
 
 /* Select Graphic Rendition (SGR, "\33[...m") strings.  */
 /* Also Erase in Line (EL) to Right ("\33[K") by default.  */
@@ -104,7 +110,125 @@
 #define SGR_SEQ(str)		SGR_START str SGR_END
 #define SGR_RESET		SGR_SEQ("")
 
+#ifdef __MINGW32__
+/* Convert a color spec, a semi-colon separated list of the form
+   SGR_START"NN;MM;KK;..."SGR_END, where each number is a value of the SGR
+   parameter, into the corresponding Windows console text attribute.
 
+   This function supports a subset of the SGR rendition aspects that
+   the Windows console can display.  */
+static int
+w32_sgr2attr (const char *sgr_seq)
+{
+  const char *s, *p;
+  int code, fg = norm_attr & 15, bg = norm_attr & (15 << 4);
+  int bright = 0, inverse = 0;
+  static const int fg_color[] = {
+    0,			/* black */
+    FOREGROUND_RED,	/* red */
+    FOREGROUND_GREEN,	/* green */
+    FOREGROUND_GREEN | FOREGROUND_RED, /* yellow */
+    FOREGROUND_BLUE,		       /* blue */
+    FOREGROUND_BLUE | FOREGROUND_RED,  /* magenta */
+    FOREGROUND_BLUE | FOREGROUND_GREEN, /* cyan */
+    FOREGROUND_RED | FOREGROUND_GREEN | FOREGROUND_BLUE /* gray */
+  };
+  static const int bg_color[] = {
+    0,			/* black */
+    BACKGROUND_RED,	/* red */
+    BACKGROUND_GREEN,	/* green */
+    BACKGROUND_GREEN | BACKGROUND_RED, /* yellow */
+    BACKGROUND_BLUE,		       /* blue */
+    BACKGROUND_BLUE | BACKGROUND_RED,  /* magenta */
+    BACKGROUND_BLUE | BACKGROUND_GREEN, /* cyan */
+    BACKGROUND_RED | BACKGROUND_GREEN | BACKGROUND_BLUE /* gray */
+  };
+
+  sgr_seq = sqr_seq + strlen(SGR_START);
+  
+  for (s = p = sgr_seq; strcmp(s, SGR_END) != 0; p++)
+    {
+      if (*p == ';' || strcmp(p, SGR_END) == 0)
+        {
+          code = strtol (s, NULL, 10);
+          s = p + (strcmp(p, SGR_END) != 0);
+
+          switch (code)
+            {
+            case 0:	/* all attributes off */
+              fg = norm_attr & 15;
+              bg = norm_attr & (15 << 4);
+              bright = 0;
+              inverse = 0;
+              break;
+            case 1:	/* intensity on */
+              bright = 1;
+              break;
+            case 7:	/* inverse video */
+              inverse = 1;
+              break;
+            case 22:	/* intensity off */
+              bright = 0;
+              break;
+            case 27:	/* inverse off */
+              inverse = 0;
+              break;
+            case 30: case 31: case 32: case 33: /* foreground color */
+            case 34: case 35: case 36: case 37:
+              fg = fg_color[code - 30];
+              break;
+            case 39:	/* default foreground */
+              fg = norm_attr & 15;
+              break;
+            case 40: case 41: case 42: case 43: /* background color */
+            case 44: case 45: case 46: case 47:
+              bg = bg_color[code - 40];
+              break;
+            case 49:	/* default background */
+              bg = norm_attr & (15 << 4);
+              break;
+            default:
+              break;
+            }
+        }
+    }
+  if (inverse)
+    {
+      int t = fg;
+      fg = (bg >> 4);
+      bg = (t << 4);
+    }
+  if (bright)
+    fg |= FOREGROUND_INTENSITY;
+
+  return (bg & (15 << 4)) | (fg & 15);
+}
+
+/* Clear to the end of the current line with the default attribute.
+   This is needed for reasons similar to those that require the "EL to
+   Right after SGR" operation on Posix platforms: if we don't do this,
+   setting the 'mt', 'ms', or 'mc' capabilities to use a non-default
+   background color spills that color to the empty space at the end of
+   the last screen line in a match whose line spans multiple screen
+   lines.  */
+static void
+w32_clreol (void)
+{
+  DWORD nchars;
+  COORD start_pos;
+  DWORD written;
+  CONSOLE_SCREEN_BUFFER_INFO csbi;
+
+  GetConsoleScreenBufferInfo (hstderr, &csbi);
+  start_pos = csbi.dwCursorPosition;
+  nchars = csbi.dwSize.X - start_pos.X;
+
+  FillConsoleOutputAttribute (hstderr, norm_attr, nchars, start_pos,
+                              &written);
+  FillConsoleOutputCharacter (hstderr, ' ', nchars, start_pos, &written);
+}
+#endif
+
 /* The context and logic for choosing default --color screen attributes
    (foreground and background colors, etc.) are the following.
       -- There are eight basic colors available, each with its own
@@ -193,6 +317,20 @@
   if (cap->name == NULL)
     return "";
 
+#ifdef __MINGW32__
+    /* If stdout is connected to a console, set the console text
+     attribute directly instead of using cap->val.  Otherwise, use
+     cap->val to emit the SGR escape sequence as on Posix platforms;
+     this is needed when GCC is invoked as a subprocess of another
+     program, such as Emacs, which will handle the display of the
+     matches.  */
+  if (hstderr != INVALID_HANDLE_VALUE)
+    {
+      SHORT attr = w32_sgr2attr (cap->val);
+      SetConsoleTextAttribute (hstderr, attr);
+      return "";
+    }
+#endif
   return cap->val;
 }
 
@@ -199,7 +337,19 @@
 const char *
 colorize_stop (bool show_color)
 {
-  return show_color ? SGR_RESET : "";
+  if (!show_color)
+    return "";
+
+#ifdef __MINGW32__
+  if (hstderr != INVALID_HANDLE_VALUE)
+    {
+      SetConsoleTextAttribute (hstderr, norm_attr);
+      w32_clreol ();
+      return "";
+    }
+#else
+  return SGR_RESET;
+#endif
 }
 
 /* Parse GCC_COLORS.  The default would look like:
@@ -275,13 +425,22 @@
       return true;
 }
 
-#if defined(_WIN32)
-bool
-colorize_init (diagnostic_color_rule_t)
+static bool
+host_color_init (void)
 {
-  return false;
+#ifdef __MINGW32__
+  CONSOLE_SCREEN_BUFFER_INFO csbi;
+
+  hstderr = GetStdHandle (STD_ERROR_HANDLE);
+  /* Initialize the normal text attribute used by the console.  */
+  if (hstderr != INVALID_HANDLE_VALUE
+      && GetConsoleScreenBufferInfo (hstderr, &csbi))
+     norm_attr = csbi.wAttributes;
+  else
+    hstderr = INVALID_HANDLE_VALUE;
+#endif
+  return true;
 }
-#else
 
 /* Return true if we should use color when in auto mode, false otherwise. */
 static bool
@@ -288,7 +447,15 @@
 should_colorize (void)
 {
   char const *t = getenv ("TERM");
+#ifdef __MINGW32__
+  /* $TERM is not normally defined on DOS/Windows, so don't require
+     it for highlighting.  But some programs, like Emacs, do define
+     it when running GCC as a subprocess, so make sure they don't
+     set TERM=dumb.  */
+  return !(t && strcmp (t, "dumb") == 0) && isatty (STDERR_FILENO);
+#else
   return t && strcmp (t, "dumb") != 0 && isatty (STDERR_FILENO);
+#endif
 }
 
 
@@ -300,10 +467,10 @@
     case DIAGNOSTICS_COLOR_NO:
       return false;
     case DIAGNOSTICS_COLOR_YES:
-      return parse_gcc_colors ();
+      return parse_gcc_colors () && host_color_init ();
     case DIAGNOSTICS_COLOR_AUTO:
       if (should_colorize ())
-	return parse_gcc_colors ();
+	return parse_gcc_colors () && host_color_init ();
       else
 	return false;
     default:
@@ -310,4 +477,3 @@
       gcc_unreachable ();
     }
 }
-#endif

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/10/10 6:25, Manuel López-Ibáñez wrote:
> For what is worth, the color output of GCC comes originally from grep, 
> and grep does have code for colorizing in Windows: 
> http://git.savannah.gnu.org/cgit/grep.git/tree/lib
> 
> and there are significant differences with this patch. For once,
> 
>    /* $TERM is not normally defined on DOS/Windows, so don't require
>       it for highlighting.  But some programs, like Emacs, do define
>       it when running Grep as a subprocess, so make sure they don't
>       set TERM=dumb.  */
>    char const *t = getenv ("TERM");
>    return ! (t && strcmp (t, "dumb") == 0);
> 
> and they don't need a custom fputs() because their strategy is slightly 
> different: They only override colorize_start (print_start_colorize) and 
> colorize_stop (print_end_colorize) and convert ANSI sequences to W32 
> sequences on the fly. Thus, we wouldn't need to touch pretty-printer.c
Since on *nix it is not when `colorize_start()` is called that the 
terminal color is changed (it is when those ANSI escape codes are 
delivered to the other peer which will translate them), and the string 
passed to `fputs()` is free to deliver multiple escape codes, it is not 
an option unless we output integral diagnostic messages using multiple 
fputs()` calls.

For example,
```
test.c:3:9: warning: 'a' is used uninitialized in this function 
[-Wuninitialized]
```
The words 'warning' and '-Wuninitialized' should be magenta, so there 
are four ANSI escape codes (two to set the color and another two to 
restore the color), and this line of text must be output using five 
individual calls to the `fputs()` function (one for each segment with 
the consistent color), which is not the case (this whole line of text is 
delivered using a single call), so all five segments have to be all in 
magenta or no color at all. This is not a solution.

> and all changes will be restricted to diagnostic-color.c (which could be 
> split into -posix.c and -w32.c like grep does and be moved into 
> host-specific config/ subdir).
> 
> Even if the host-specific part is not done, I honestly think it is a 
> good idea to match grep's code as much as possible since we may want to 
> merge bugfixes between the two and eventually this code may end up in 
> gnulib. Moreover, if somebody else implemented color output for another 
> OS in grep, it would be very easy to transplant it to GCC (or viceversa) 
> if the API remains close.
> 
> Cheers,
> 
>      Manuel.


-- 
Best regards,
LH_Mouse

Re: [PATCH][mingw] Enable colorized diagnostics

[Manuel López-Ibáñez]

On 10 Oct 2017 2:34 am, "Liu Hao" <lh_mouse@126.com> wrote:

Since on *nix it is not when `colorize_start()` is called that the terminal
color is changed (it is when those ANSI escape codes are delivered to the
other peer which will translate them), and the string passed to `fputs()`
is free to deliver multiple escape codes, it is not an option unless we
output integral diagnostic messages using multiple fputs()` calls.

For example,
```
test.c:3:9: warning: 'a' is used uninitialized in this function
[-Wuninitialized]
```
The words 'warning' and '-Wuninitialized' should be magenta, so there are
four ANSI escape codes (two to set the color and another two to restore the
color), and this line of text must be output using five individual calls to
the `fputs()` function (one for each segment with the consistent color),
which is not the case (this whole line of text is delivered using a single
call), so all five segments have to be all in magenta or no color at all.
This is not a solution.


Ops! You're obviously right. What was I thinking?

I still believe that pretty-printer.c is not the right place for all this
color-handling code (diagnostic-color.c or libiberty/ may be better
places). Also, your code handles a lot more ANSI codes than those needed
for color output. The code in grep seems much simpler. Could your fputs
replacement split the string as you suggest above, then if the chunk is an
ANSI code, use grep's conversion function to transform the codes, otherwise
use fputs to print text.

Cheers,

Manuel.

Re: [PATCH][mingw] Enable colorized diagnostics

[Liu Hao]

On 2017/10/11 5:04, Manuel López-Ibáñez wrote:
> Ops! You're obviously right. What was I thinking?
> 
> I still believe that pretty-printer.c is not the right place for all 
> this color-handling code (diagnostic-color.c or libiberty/ may be better 
> places).
No and yes. The colors emerge only when those messages are sent to a 
terminal. Plain arrays of characters don't have colors. So it is _the_ 
printer that does colorization.

Indeed I hope this host-specific code could go elsewhere. My initial 
attempt to move it to a separated .c file failed, as I mentioned in the 
very first mail.

> Also, your code handles a lot more ANSI codes than those needed 
> for color output. The code in grep seems much simpler. Could your fputs 
> replacement split the string as you suggest above, then if the chunk is 
> an ANSI code, use grep's conversion function to transform the codes, 
> otherwise use fputs to print text.
> 
At the moment I implemented it, I knew GCC was making of coloring and 
erase-line codes, while still had no idea whether more codes should be 
needed in the future. Hence almost all codes viable have been implemented.

I hope one day GREP people will be copying my code from GCC. Don't tell 
them. XD

> Cheers,
> 
> Manuel.
> 
> 


-- 
Best regards,
LH_Mouse