From 5fb9588c15045f28e0b7915f2f4d72a68a962b66 Mon Sep 17 00:00:00 2001 From: Stephen Toub Date: Fri, 2 Jul 2021 10:06:21 -0400 Subject: [PATCH] Fix ANSI color env var handling --- .../src/System/ConsolePal.Unix.cs | 60 +++++++++++++------ src/libraries/System.Console/tests/Color.cs | 60 +++++++++++++++++-- 2 files changed, 98 insertions(+), 22 deletions(-) diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index 7442d3a01a40f3..5f2e485aca5e43 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/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. + /// Whether to output ansi color strings. + 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) /// Writes out the ANSI string to reset colors. private static void WriteResetColorString() { - if (SupportsAnsiColor()) + if (EmitAnsiColorCodes) { WriteStdoutAnsiString(TerminalFormatStrings.Instance.Reset); } } - /// - /// Tests whether ANSI color codes should be emitted or not. - /// - /// If the DOTNET_CONSOLE_ANSI_COLOR environment variable contains true (case insensitive) or 1 - /// then ANSI color codes are supported. If the DOTNET_CONSOLE_ANSI_COLOR environment variable is not defined - /// or contains a non truthy value, then ANSI color codes are supported if the console output is not redirected. - /// - /// - /// true if ANSI color escape codes must be emitted, false if they must not be emitted. - /// This was discussed in https://github.com/dotnet/runtime/issues/33980 - private static bool SupportsAnsiColor() + /// Get whether to emit ANSI color codes. + 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; + } } /// diff --git a/src/libraries/System.Console/tests/Color.cs b/src/libraries/System.Console/tests/Color.cs index 567faad01ca4c4..55843aa2f37246 100644 --- a/src/libraries/System.Console/tests/Color.cs +++ b/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 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)); + } + } }