Fix ANSI color env var handling

Author: stephentoub

src/libraries/System.Console/src/System/ConsolePal.Unix.cs

@@ -39,6 +39,9 @@ internal static class ConsolePal
         private static int s_windowHeight;  // Cached WindowHeight, invalid when s_windowWidth == -1.
         private static int s_invalidateCachedSettings = 1; // Tracks whether we should invalidate the cached settings.
 
+        /// <summary>Whether to output ansi color strings.</summary>
+        private static volatile int s_emitAnsiColorCodes = -1;
+
         public static Stream OpenStandardInput()
         {
             return new UnixConsoleStream(SafeFileHandleHelper.Open(() => Interop.Sys.Dup(Interop.Sys.FileDescriptors.STDIN_FILENO)), FileAccess.Read,
@@ -779,8 +782,10 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
             // Changing the color involves writing an ANSI character sequence out to the output stream.
             // We only want to do this if we know that sequence will be interpreted by the output.
             // rather than simply displayed visibly.
-            if (!SupportsAnsiColor())
+            if (!EmitAnsiColorCodes)
+            {
                 return;
+            }
 
             // See if we've already cached a format string for this foreground/background
             // and specific color choice.  If we have, just output that format string again.
@@ -813,31 +818,50 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
         /// <summary>Writes out the ANSI string to reset colors.</summary>
         private static void WriteResetColorString()
         {
-            if (SupportsAnsiColor())
+            if (EmitAnsiColorCodes)
             {
                 WriteStdoutAnsiString(TerminalFormatStrings.Instance.Reset);
             }
         }
 
-        /// <summary>
-        /// Tests whether ANSI color codes should be emitted or not.
-        /// <para>
-        /// If the <c>DOTNET_CONSOLE_ANSI_COLOR</c> environment variable contains <c>true</c> (case insensitive) or <c>1</c>
-        /// then ANSI color codes are supported. If the <c>DOTNET_CONSOLE_ANSI_COLOR</c> environment variable is not defined
-        /// or contains a non truthy value, then ANSI color codes are supported if the console output is not redirected.
-        /// </para>
-        /// </summary>
-        /// <returns><c>true</c> if ANSI color escape codes must be emitted, <c>false</c> if they must not be emitted.</returns>
-        /// <remarks>This was discussed in https://github.com/dotnet/runtime/issues/33980</remarks>
-        private static bool SupportsAnsiColor()
+        /// <summary>Get whether to emit ANSI color codes.</summary>
+        private static bool EmitAnsiColorCodes
         {
-            string? consoleAnsiColor = Environment.GetEnvironmentVariable("DOTNET_CONSOLE_ANSI_COLOR");
-            if (consoleAnsiColor != null)
+            get
             {
-                return consoleAnsiColor == "1" || (bool.TryParse(consoleAnsiColor, out bool enabled) && enabled);
-            }
+                // The flag starts at -1.  If it's no longer -1, it's 0 or 1 to represent false or true.
+                int emitAnsiColorCodes = s_emitAnsiColorCodes;
+                if (emitAnsiColorCodes != -1)
+                {
+                    return Convert.ToBoolean(emitAnsiColorCodes);
+                }
 
-            return !Console.IsOutputRedirected;
+                // We've not yet computed whether to emit codes or not.  Do so now.  We may race with
+                // other threads, and that's ok; this is idempotent unless someone is currently changing
+                // the value of the relevant environment variables, in which case behavior here is undefined.
+
+                // By default, we emit ANSI color codes if output isn't redirected, and suppress them if output is redirected.
+                bool enabled = !Console.IsOutputRedirected;
+
+                if (enabled)
+                {
+                    // We subscribe to the informal standard from https://no-color.org/.  If we'd otherwise emit
+                    // ANSI color codes but the NO_COLOR environment variable is set, disable emitting them.
+                    enabled = Environment.GetEnvironmentVariable("NO_COLOR") is null;
+                }
+                else
+                {
+                    // We also support overriding in the other direction.  If we'd otherwise avoid emitting color
+                    // codes but the DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION environment variable is
+                    // set to 1 or true, enable color.
+                    string? envVar = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION");
+                    enabled = envVar is not null && (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase));
+                }
+
+                // Store and return the computed answer.
+                s_emitAnsiColorCodes = Convert.ToInt32(enabled);
+                return enabled;
+            }
         }
 
         /// <summary>

src/libraries/System.Console/tests/Color.cs

@@ -2,15 +2,18 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System;
-using System.IO;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Globalization;
 using System.Linq;
-using System.Runtime.InteropServices;
 using System.Text;
-using Microsoft.DotNet.XUnitExtensions;
+using Microsoft.DotNet.RemoteExecutor;
 using Xunit;
 
 public class Color
 {
+    private const char Esc = (char)0x1B;
+
     [Fact]
     [SkipOnPlatform(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Not supported on Browser, iOS, MacCatalyst, or tvOS.")]
     public static void InvalidColors()
@@ -64,9 +67,58 @@ public static void RedirectedOutputDoesNotUseAnsiSequences()
             Console.ResetColor();
             Console.Write('4');
 
-            const char Esc = (char)0x1B;
             Assert.Equal(0, Encoding.UTF8.GetString(data.ToArray()).ToCharArray().Count(c => c == Esc));
             Assert.Equal("1234", Encoding.UTF8.GetString(data.ToArray()));
         });
     }
+
+    public static bool TermIsSet => !string.IsNullOrEmpty(Environment.GetEnvironmentVariable("TERM"));
+
+    [ConditionalTheory(nameof(TermIsSet))]
+    [PlatformSpecific(TestPlatforms.AnyUnix)]
+    [SkipOnPlatform(TestPlatforms.Browser | TestPlatforms.iOS | TestPlatforms.MacCatalyst | TestPlatforms.tvOS, "Not supported on Browser, iOS, MacCatalyst, or tvOS.")]
+    [InlineData(null)]
+    [InlineData("1")]
+    [InlineData("true")]
+    [InlineData("tRuE")]
+    [InlineData("0")]
+    [InlineData("false")]
+    public static void RedirectedOutput_EnvVarSet_EmitsAnsiCodes(string envVar)
+    {
+        var psi = new ProcessStartInfo { RedirectStandardOutput = true };
+        psi.Environment["DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION"] = envVar;
+
+        for (int i = 0; i < 3; i++)
+        {
+            Action<string> main = i =>
+            {
+                Console.Write("SEPARATOR");
+                switch (i)
+                {
+                    case "0":
+                        Console.ForegroundColor = ConsoleColor.Blue;
+                        break;
+
+                    case "1":
+                        Console.BackgroundColor = ConsoleColor.Red;
+                        break;
+
+                    case "2":
+                        Console.ResetColor();
+                        break;
+                }
+                Console.Write("SEPARATOR");
+            };
+
+            using RemoteInvokeHandle remote = RemoteExecutor.Invoke(main, i.ToString(CultureInfo.InvariantCulture), new RemoteInvokeOptions() { StartInfo = psi });
+
+            bool expectedEscapes = envVar is not null && (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase));
+
+            string stdout = remote.Process.StandardOutput.ReadToEnd();
+            string[] parts = stdout.Split("SEPARATOR");
+            Assert.Equal(3, parts.Length);
+
+            Assert.Equal(expectedEscapes, parts[1].Contains(Esc));
+        }
+    }
 }