diff mbox series

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

Message ID 20240509170157.8534-2-peter0x44@disroot.org
State New
Headers show
Series [v2,1/3] diagnostics: Enable escape sequence processing on windows consoles | expand

Commit Message

Peter0x44 May 9, 2024, 5:01 p.m. UTC
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(-)

Comments

NightStrike May 13, 2024, 12:30 p.m. UTC | #1
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?
Peter0x44 May 13, 2024, 12:36 p.m. UTC | #2
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?
Peter0x44 May 13, 2024, 2:28 p.m. UTC | #3
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 installed wine and gave compiling a file emitting a warning a try. 
Unfortunately, yes, gcc emits mangled warnings here. Even simply running 
this patch under wine causes problems, it's not just wine's conhost.exe.

I'm not sure whether it's my fault or wine's. I've attached two 
screenshots demonstrating exactly what happens. (I think???) wine should 
only be advertising that it supports those settings regarding escape 
sequences if it actually does. Also, on this machine, wine is near 
unusably slow, I'm talking multiple seconds to react to a keypress 
through the wine conhost. I will not be attempting to run the testsuite, 
I severely doubt it will work.
diff mbox series

Patch

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,