[PATCH v2 1/3] diagnostics: Enable escape sequence processing on windows consoles

[PATCH v2 1/3] diagnostics: Enable escape sequence processing on windows consoles

[Peter Damianov]

Since windows 10 release v1511, the windows console has had support for VT100
escape sequences. We should try to enable this, and utilize it where possible.

gcc/ChangeLog:
	* diagnostic-color.cc (should_colorize): Enable processing of VT100
	escape sequences on windows consoles

Signed-off-by: Peter Damianov <peter0x44@disroot.org>
---

Forgot to add -v2 to git send-email the first time I sent. Sorry for the spam.

 gcc/diagnostic-color.cc | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

diff --git a/gcc/diagnostic-color.cc b/gcc/diagnostic-color.cc
index f01a0fc2e37..3af198654af 100644
--- a/gcc/diagnostic-color.cc
+++ b/gcc/diagnostic-color.cc
@@ -213,12 +213,23 @@ should_colorize (void)
      pp_write_text_to_stream() in pretty-print.cc calls fputs() on
      that stream.  However, the code below for non-Windows doesn't seem
      to care about it either...  */
-  HANDLE h;
-  DWORD m;
+  HANDLE handle;
+  DWORD mode;
+  BOOL isconsole = false;
 
-  h = GetStdHandle (STD_ERROR_HANDLE);
-  return (h != INVALID_HANDLE_VALUE) && (h != NULL)
-	  && GetConsoleMode (h, &m);
+  handle = GetStdHandle (STD_ERROR_HANDLE);
+
+  if ((handle != INVALID_HANDLE_VALUE) && (handle != NULL))
+    isconsole = GetConsoleMode (handle, &mode);
+
+  if (isconsole)
+    {
+      /* Try to enable processing of VT100 escape sequences */
+      mode |= ENABLE_PROCESSED_OUTPUT | ENABLE_VIRTUAL_TERMINAL_PROCESSING;
+      SetConsoleMode (handle, mode);
+    }
+
+  return isconsole;
 #else
   char const *t = getenv ("TERM");
   /* emacs M-x shell sets TERM="dumb".  */
-- 
2.39.2

[PATCH v2 2/3] diagnostics: Don't hardcode auto_enable_urls to false for mingw hosts

[Peter Damianov]

Windows terminal and mintty both have support for link escape sequences, and so
auto_enable_urls shouldn't be hardcoded to false. For older versions of the
windows console, mingw_ansi_fputs's console API translation logic does mangle
these sequences, but there's nothing useful it could do even if this weren't
the case, so check if the ansi escape sequences are supported at all.

conhost.exe doesn't support link escape sequences, but printing them does not
cause any problems.

gcc/ChangeLog:
	* diagnostic-color.cc (auto_enable_urls): Don't hardcode to return
	false on mingw hosts.
	* diagnostic-color.cc (auto_enable_urls): Return true if console
	supports ansi escape sequences.

Signed-off-by: Peter Damianov <peter0x44@disroot.org>
---

v2: auto_enable_urls should check if the console supports ansi escape sequences

 gcc/diagnostic-color.cc | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/gcc/diagnostic-color.cc b/gcc/diagnostic-color.cc
index 3af198654af..03fe35b2dc8 100644
--- a/gcc/diagnostic-color.cc
+++ b/gcc/diagnostic-color.cc
@@ -293,9 +293,6 @@ parse_env_vars_for_urls ()
 static bool
 auto_enable_urls ()
 {
-#ifdef __MINGW32__
-  return false;
-#else
   const char *term, *colorterm;
 
   /* First check the terminal is capable of printing color escapes,
@@ -303,6 +300,21 @@ auto_enable_urls ()
   if (!should_colorize ())
     return false;
 
+#ifdef __MINGW32__
+  HANDLE handle;
+  DWORD mode;
+
+  handle = GetStdHandle (STD_ERROR_HANDLE);
+  if ((handle == INVALID_HANDLE_VALUE) || (handle == NULL))
+    return false;
+
+  /* If ansi escape sequences aren't supported by the console, then URLs will
+     print mangled from mingw_ansi_fputs's console API translation. It wouldn't
+     be useful even if this weren't the case.  */
+  if (GetConsoleMode (handle, &mode) && !(mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
+    return false;
+#endif
+
   /* xfce4-terminal is known to not implement URLs at this time.
      Recently new installations (0.8) will safely ignore the URL escape
      sequences, but a large number of legacy installations (0.6.3) print
@@ -337,7 +349,6 @@ auto_enable_urls ()
     return false;
 
   return true;
-#endif
 }
 
 /* Determine if URLs should be enabled, based on RULE,
-- 
2.39.2

[PATCH v2 3/3] pretty-print: Don't translate escape sequences to windows console API

[Peter Damianov]

Modern versions of windows (after windows 10 v1511) support VT100 escape
sequences, so translation for them is not necessary. The translation also
mangles embedded warning documentation links.

gcc/ChangeLog:
	* pretty-print.cc (mingw_ansi_fputs): Don't translate escape sequences if
	the console has ENABLE_VIRTUAL_TERMINAL_PROCESSING.

Signed-off-by: Peter Damianov <peter0x44@disroot.org>
---
 gcc/pretty-print.cc | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/gcc/pretty-print.cc b/gcc/pretty-print.cc
index eb59bf424b7..98b6410d6e4 100644
--- a/gcc/pretty-print.cc
+++ b/gcc/pretty-print.cc
@@ -674,8 +674,10 @@ mingw_ansi_fputs (const char *str, FILE *fp)
   /* 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.  */
+  if (GetConsoleMode (h, &mode) && !(mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
+    /* If it is a console, and doesn't support ANSI escape codes, translate
+     * them as needed.
+     */
     for (;;)
       {
 	if ((esc_code = find_esc_head (&prefix_len, &esc_head, read)) == 0)
-- 
2.39.2

Re: [PATCH v2 1/3] diagnostics: Enable escape sequence processing on windows consoles

[Peter0x44]

9 May 2024 6:02:34 pm Peter Damianov <peter0x44@disroot.org>:

> Since windows 10 release v1511, the windows console has had support for 
> VT100
> escape sequences. We should try to enable this, and utilize it where 
> possible.
>
> gcc/ChangeLog:
>     * diagnostic-color.cc (should_colorize): Enable processing of VT100
>     escape sequences on windows consoles
>
> Signed-off-by: Peter Damianov <peter0x44@disroot.org>
> ---
>
> Forgot to add -v2 to git send-email the first time I sent. Sorry for 
> the spam.
>
> gcc/diagnostic-color.cc | 21 ++++++++++++++++-----
> 1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/diagnostic-color.cc b/gcc/diagnostic-color.cc
> index f01a0fc2e37..3af198654af 100644
> --- a/gcc/diagnostic-color.cc
> +++ b/gcc/diagnostic-color.cc
> @@ -213,12 +213,23 @@ should_colorize (void)
>       pp_write_text_to_stream() in pretty-print.cc calls fputs() on
>       that stream.  However, the code below for non-Windows doesn't 
> seem
>       to care about it either...  */
> -  HANDLE h;
> -  DWORD m;
> +  HANDLE handle;
> +  DWORD mode;
> +  BOOL isconsole = false;
>
> -  h = GetStdHandle (STD_ERROR_HANDLE);
> -  return (h != INVALID_HANDLE_VALUE) && (h != NULL)
> -     && GetConsoleMode (h, &m);
> +  handle = GetStdHandle (STD_ERROR_HANDLE);
> +
> +  if ((handle != INVALID_HANDLE_VALUE) && (handle != NULL))
> +    isconsole = GetConsoleMode (handle, &mode);
> +
> +  if (isconsole)
> +    {
> +      /* Try to enable processing of VT100 escape sequences */
> +      mode |= ENABLE_PROCESSED_OUTPUT | 
> ENABLE_VIRTUAL_TERMINAL_PROCESSING;
> +      SetConsoleMode (handle, mode);
> +    }
> +
> +  return isconsole;
> #else
>    char const *t = getenv ("TERM");
>    /* emacs M-x shell sets TERM="dumb".  */
> --
> 2.39.2
I asked a windows terminal maintainer to review the patches here:
https://github.com/microsoft/terminal/discussions/17219#discussioncomment-9375044
And got an "LGTM".

I tested the patches with windows terminal, conhost.exe, and conhost.exe 
with the "use legacy console" box checked, and they all worked correctly.

I think this is okay for trunk.

Re: [PATCH v2 3/3] pretty-print: Don't translate escape sequences to windows console API

[LIU Hao]

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

在 2024-05-10 01:02, Peter Damianov 写道:
> -  if (GetConsoleMode (h, &mode))
> -    /* If it is a console, translate ANSI escape codes as needed.  */
> +  if (GetConsoleMode (h, &mode) && !(mode & ENABLE_VIRTUAL_TERMINAL_PROCESSING))
> +    /* If it is a console, and doesn't support ANSI escape codes, translate
> +     * them as needed.
> +     */

nitpicking: This should probably be

> +     * them as needed.  */


CC'ing Jonathan Yong. This series of patches look good to me.



-- 
Best regards,
LIU Hao


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

Re: [PATCH v2 2/3] diagnostics: Don't hardcode auto_enable_urls to false for mingw hosts

[NightStrike]

On Thu, May 9, 2024 at 1:03 PM Peter Damianov <peter0x44@disroot.org> wrote:
>
> Windows terminal and mintty both have support for link escape sequences, and so
> auto_enable_urls shouldn't be hardcoded to false. For older versions of the
> windows console, mingw_ansi_fputs's console API translation logic does mangle
> these sequences, but there's nothing useful it could do even if this weren't
> the case, so check if the ansi escape sequences are supported at all.
>
> conhost.exe doesn't support link escape sequences, but printing them does not
> cause any problems.

Are there any issues when running under the Wine console, such as when
running the testsuite?

Re: [PATCH v2 2/3] diagnostics: Don't hardcode auto_enable_urls to false for mingw hosts

[Peter0x44]

13 May 2024 1:30:28 pm NightStrike <nightstrike@gmail.com>:

> On Thu, May 9, 2024 at 1:03 PM Peter Damianov <peter0x44@disroot.org> 
> wrote:
>>
>> Windows terminal and mintty both have support for link escape 
>> sequences, and so
>> auto_enable_urls shouldn't be hardcoded to false. For older versions 
>> of the
>> windows console, mingw_ansi_fputs's console API translation logic does 
>> mangle
>> these sequences, but there's nothing useful it could do even if this 
>> weren't
>> the case, so check if the ansi escape sequences are supported at all.
>>
>> conhost.exe doesn't support link escape sequences, but printing them 
>> does not
>> cause any problems.
>
> Are there any issues when running under the Wine console, such as when
> running the testsuite?

I did not try this. There shouldn't be problems if wine implements 
ENABLE_VIRTUAL_TERMINAL_PROCESSING correctly, but I agree it would be 
good to check. Are there instructions anywhere for running the testsuite 
with wine? Anything specific I need to do?