Console.Unix: fix ANSI colors over redirected standard output. (#95654)

* Console.Unix: fix ANSI colors over redirected standard output. * Match exception parameter name with property name.
dotnet · Dec 21, 2023 · ec2bfd7 · ec2bfd7
1 parent 476a455
commit ec2bfd7
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 15 deletions.
diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs
@@ -406,7 +406,7 @@ public static int WindowWidth
                     throw new IOException(SR.InvalidOperation_SetWindowSize);
                 }
 
-                ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);
+                ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowWidth));
 
                 ConsolePal.WindowWidth = value;
             }
@@ -426,7 +426,7 @@ public static int WindowHeight
                     throw new IOException(SR.InvalidOperation_SetWindowSize);
                 }
 
-                ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value);
+                ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowHeight));
 
                 ConsolePal.WindowHeight = value;
             }
@@ -449,8 +449,8 @@ public static void SetWindowSize(int width, int height)
                 throw new IOException(SR.InvalidOperation_SetWindowSize);
             }
 
-            ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width);
-            ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height);
+            ArgumentOutOfRangeException.ThrowIfNegativeOrZero(width, nameof(width));
+            ArgumentOutOfRangeException.ThrowIfNegativeOrZero(height, nameof(height));
 
             ConsolePal.SetWindowSize(width, height);
         }

diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs
@@ -801,7 +801,7 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
             string evaluatedString = s_fgbgAndColorStrings[fgbgIndex, ccValue]; // benign race
             if (evaluatedString != null)
             {
-                WriteTerminalAnsiString(evaluatedString);
+                WriteTerminalAnsiColorString(evaluatedString);
                 return;
             }
 
@@ -841,7 +841,7 @@ private static void WriteSetColorString(bool foreground, ConsoleColor color)
                     int ansiCode = consoleColorToAnsiCode[ccValue] % maxColors;
                     evaluatedString = TermInfo.ParameterizedStrings.Evaluate(formatString, ansiCode);
 
-                    WriteTerminalAnsiString(evaluatedString);
+                    WriteTerminalAnsiColorString(evaluatedString);
 
                     s_fgbgAndColorStrings[fgbgIndex, ccValue] = evaluatedString; // benign race
                 }
@@ -853,7 +853,7 @@ private static void WriteResetColorString()
         {
             if (ConsoleUtils.EmitAnsiColorCodes)
             {
-                WriteTerminalAnsiString(TerminalFormatStringsInstance.Reset);
+                WriteTerminalAnsiColorString(TerminalFormatStringsInstance.Reset);
             }
         }
 
@@ -952,13 +952,14 @@ private static unsafe int Read(SafeFileHandle fd, Span<byte> buffer)
             }
         }
 
-        internal static void WriteToTerminal(ReadOnlySpan<byte> buffer, bool mayChangeCursorPosition = true)
+        internal static void WriteToTerminal(ReadOnlySpan<byte> buffer, SafeFileHandle? handle = null, bool mayChangeCursorPosition = true)
         {
-            Debug.Assert(s_terminalHandle is not null);
+            handle ??= s_terminalHandle;
+            Debug.Assert(handle is not null);
 
             lock (Console.Out) // synchronize with other writers
             {
-                Write(s_terminalHandle, buffer, mayChangeCursorPosition);
+                Write(handle, buffer, mayChangeCursorPosition);
             }
         }
 
@@ -1110,10 +1111,17 @@ private static void InvalidateTerminalSettings()
             Volatile.Write(ref s_invalidateCachedSettings, 1);
         }
 
+        // Ansi colors are enabled when stdout is a terminal or when
+        // DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION is set.
+        // In both cases, they are written to stdout.
+        internal static void WriteTerminalAnsiColorString(string? value)
+            => WriteTerminalAnsiString(value, Interop.Sys.FileDescriptors.STDOUT_FILENO, mayChangeCursorPosition: false);
+
         /// <summary>Writes a terminfo-based ANSI escape string to stdout.</summary>
         /// <param name="value">The string to write.</param>
+        /// <param name="handle">Handle to use instead of s_terminalHandle.</param>
         /// <param name="mayChangeCursorPosition">Writing this value may change the cursor position.</param>
-        internal static void WriteTerminalAnsiString(string? value, bool mayChangeCursorPosition = true)
+        internal static void WriteTerminalAnsiString(string? value, SafeFileHandle? handle = null, bool mayChangeCursorPosition = true)
         {
             if (string.IsNullOrEmpty(value))
                 return;
@@ -1131,7 +1139,7 @@ internal static void WriteTerminalAnsiString(string? value, bool mayChangeCursor
             }
 
             EnsureConsoleInitialized();
-            WriteToTerminal(data, mayChangeCursorPosition);
+            WriteToTerminal(data, handle, mayChangeCursorPosition);
         }
     }
 }
diff --git a/src/libraries/System.Console/tests/WindowAndCursorProps.cs b/src/libraries/System.Console/tests/WindowAndCursorProps.cs
@@ -57,7 +57,7 @@ public static void WindowWidth_SetInvalid_ThrowsArgumentOutOfRangeException(int
         }
         else
         {
-            AssertExtensions.Throws<ArgumentOutOfRangeException>("width", () => Console.WindowWidth = value);
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("WindowWidth", () => Console.WindowWidth = value);
         }
     }
 
@@ -89,7 +89,7 @@ public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int
         }
         else
         {
-            AssertExtensions.Throws<ArgumentOutOfRangeException>("height", () => Console.WindowHeight = value);
+            AssertExtensions.Throws<ArgumentOutOfRangeException>("WindowHeight", () => Console.WindowHeight = value);
         }
     }
 
@@ -559,7 +559,7 @@ public void SetWindowPosition_Unix_ThrowsPlatformNotSupportedException()
         Assert.Throws<PlatformNotSupportedException>(() => Console.SetWindowPosition(50, 50));
     }
 
-    [PlatformSpecific((TestPlatforms.Windows) | (TestPlatforms.AnyUnix & ~TestPlatforms.Browser & ~TestPlatforms.iOS & ~TestPlatforms.MacCatalyst & ~TestPlatforms.tvOS))]
+    [PlatformSpecific(TestPlatforms.Windows)]
     [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoNorServerCore))]
     public void SetWindowSize_GetWindowSize_ReturnsExpected()
     {