Fixes and tweaks to diagnostics color support on Windows (GH issue #312)

Specifically:

- Pass -fansi-escape-codes for Clang on Windows.

- Enable diagnostics color by default if already enabled on the terminal.
  Only try to enable it ourselves with explicit --diag-color.

2 files changed, 25 insertions, 20 deletions

diff --git a/libbuild2/cc/common.cxx b/libbuild2/cc/common.cxx
index 2a8bc50..9a4a07c 100644
--- a/libbuild2/cc/common.cxx
+++ b/libbuild2/cc/common.cxx
@@ -1691,6 +1691,10 @@ namespace build2
           // Note that clang-cl appears to use -fansi-escape-codes. See GH
           // issue #312 for background.
           //
+          // Note that MSVC ignores /diagnostics:color if diagnostics is
+          // written to a pipe. See GH issue #312 for details and a link to
+          // the MSVC bug report.
+          //
           if (show_diag_color ())
           {
             if (cvariant.empty () &&
@@ -1716,7 +1720,8 @@ namespace build2
           //
           // Supported from GCC 4.9 (8.1 on Windows) and (at least) from Clang
           // 3.5. Clang supports -f[no]color-diagnostics in addition to the
-          // GCC's spelling.
+          // GCC's spelling. Note that to enable color on Windows Clang also
+          // needs -fansi-escape-codes.
           //
           if (
 #ifndef _WIN32
@@ -1742,7 +1747,14 @@ namespace build2
                     show_diag_color () ? "-fdiagnostics-color"    :
                     stderr_term        ? "-fno-diagnostics-color" :
                     nullptr))
+              {
                 args.push_back (o);
+
+#ifdef _WIN32
+                if (ctype == compiler_type::clang && o[2] != 'n')
+                  args.push_back ("-fansi-escape-codes");
+#endif
+              }
             }
           }
 
diff --git a/libbuild2/diagnostics.cxx b/libbuild2/diagnostics.cxx
index 4a46756..1fd1139 100644
--- a/libbuild2/diagnostics.cxx
+++ b/libbuild2/diagnostics.cxx
@@ -55,31 +55,24 @@ namespace build2
 
     if (st)
     {
-      // @@ TMP: eventually we want to enable on Windows by default.
+      // Only attempt to enable if explicitly requested by the user. Note that
+      // while we may enable color for our process, who knows if this gets
+      // inherited by other processes we start (e.g., compilers) and/or
+      // whether they will do something sensible about any of this.
       //
-#ifdef _WIN32
-      if (c && *c)
-      {
-#endif
-        stderr_term_color = fdterm_color (stderr_fd (), !c || *c /* enable */);
+      stderr_term_color = fdterm_color (stderr_fd (), c && *c /* enable */);
 
-        // If the user specified --diag-color on POSIX we will trust the color
-        // is supported (e.g., wrong TERM value, etc).
-        //
-        if (!stderr_term_color && c && *c)
-        {
+      // If the user specified --diag-color on POSIX we will trust the color
+      // is supported (e.g., wrong TERM value, etc).
+      //
+      if (!stderr_term_color && c && *c)
+      {
 #ifdef _WIN32
-          fail << "unable to enable diagnostics color support for stderr";
+        fail << "unable to enable diagnostics color support for stderr";
 #else
-          stderr_term_color = true;
+        stderr_term_color = true;
 #endif
-        }
-
-#ifdef _WIN32
       }
-      else
-        stderr_term_color = false;
-#endif
     }
     else
       stderr_term_color = false;