From a0a99c42de16a109942ea757d71ead50d7acb713 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Tue, 5 Dec 2023 19:57:23 +0100 Subject: [PATCH 1/5] Console.Unix: fix ANSI colors over redirected standard output. --- .../src/System/ConsolePal.Unix.cs | 24 ++++++++++++------- .../tests/WindowAndCursorProps.cs | 4 ++-- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index 7d9a74d6a85528..374febf07afee0 100644 --- 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 buffer) } } - internal static void WriteToTerminal(ReadOnlySpan buffer, bool mayChangeCursorPosition = true) + internal static void WriteToTerminal(ReadOnlySpan buffer, SafeFileHandle? overrideHandle = null, bool mayChangeCursorPosition = true) { - Debug.Assert(s_terminalHandle is not null); + SafeFileHandle? handle = overrideHandle ?? 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); + /// Writes a terminfo-based ANSI escape string to stdout. /// The string to write. + /// Handle to use instead of s_terminalHandle. /// Writing this value may change the cursor position. - internal static void WriteTerminalAnsiString(string? value, bool mayChangeCursorPosition = true) + internal static void WriteTerminalAnsiString(string? value, SafeFileHandle? overrideHandle = 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, overrideHandle, mayChangeCursorPosition); } } } diff --git a/src/libraries/System.Console/tests/WindowAndCursorProps.cs b/src/libraries/System.Console/tests/WindowAndCursorProps.cs index 10d0371d9a9031..de9516c8ecdbff 100644 --- 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("width", () => Console.WindowWidth = value); + AssertExtensions.Throws("value", () => Console.WindowWidth = value); } } @@ -89,7 +89,7 @@ public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int } else { - AssertExtensions.Throws("height", () => Console.WindowHeight = value); + AssertExtensions.Throws("value", () => Console.WindowHeight = value); } } From 4b0a10f1771b3f4c37e4fee06f962220fa9240d5 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 6 Dec 2023 15:59:00 +0100 Subject: [PATCH 2/5] PR feedback. --- .../System.Console/src/System/ConsolePal.Unix.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs index 374febf07afee0..23d4b9ba595f8d 100644 --- a/src/libraries/System.Console/src/System/ConsolePal.Unix.cs +++ b/src/libraries/System.Console/src/System/ConsolePal.Unix.cs @@ -952,9 +952,9 @@ private static unsafe int Read(SafeFileHandle fd, Span buffer) } } - internal static void WriteToTerminal(ReadOnlySpan buffer, SafeFileHandle? overrideHandle = null, bool mayChangeCursorPosition = true) + internal static void WriteToTerminal(ReadOnlySpan buffer, SafeFileHandle? handle = null, bool mayChangeCursorPosition = true) { - SafeFileHandle? handle = overrideHandle ?? s_terminalHandle; + handle ??= s_terminalHandle; Debug.Assert(handle is not null); lock (Console.Out) // synchronize with other writers @@ -1119,9 +1119,9 @@ internal static void WriteTerminalAnsiColorString(string? value) /// Writes a terminfo-based ANSI escape string to stdout. /// The string to write. - /// Handle to use instead of s_terminalHandle. + /// Handle to use instead of s_terminalHandle. /// Writing this value may change the cursor position. - internal static void WriteTerminalAnsiString(string? value, SafeFileHandle? overrideHandle = null, bool mayChangeCursorPosition = true) + internal static void WriteTerminalAnsiString(string? value, SafeFileHandle? handle = null, bool mayChangeCursorPosition = true) { if (string.IsNullOrEmpty(value)) return; @@ -1139,7 +1139,7 @@ internal static void WriteTerminalAnsiString(string? value, SafeFileHandle? over } EnsureConsoleInitialized(); - WriteToTerminal(data, overrideHandle, mayChangeCursorPosition); + WriteToTerminal(data, handle, mayChangeCursorPosition); } } } From c121b5f5f514db59ee888dd77b0d44f19681c5d6 Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 7 Dec 2023 11:54:17 +0100 Subject: [PATCH 3/5] Pass names to ArgumentOutOfRangeException. --- src/libraries/System.Console/src/System/Console.cs | 8 ++++---- .../System.Console/tests/WindowAndCursorProps.cs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index 9a297e6c36b0b7..cf57c6d1210aa9 100644 --- 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, "width"); ConsolePal.WindowWidth = value; } @@ -426,7 +426,7 @@ public static int WindowHeight throw new IOException(SR.InvalidOperation_SetWindowSize); } - ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, "height"); 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/tests/WindowAndCursorProps.cs b/src/libraries/System.Console/tests/WindowAndCursorProps.cs index de9516c8ecdbff..10d0371d9a9031 100644 --- 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("value", () => Console.WindowWidth = value); + AssertExtensions.Throws("width", () => Console.WindowWidth = value); } } @@ -89,7 +89,7 @@ public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int } else { - AssertExtensions.Throws("value", () => Console.WindowHeight = value); + AssertExtensions.Throws("height", () => Console.WindowHeight = value); } } From 62b1a44a563c9a86a37827882160de650770365a Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Thu, 7 Dec 2023 11:57:52 +0100 Subject: [PATCH 4/5] Skip SetWindowSize_GetWindowSize_ReturnsExpected on Unix because the test can't pass. --- src/libraries/System.Console/tests/WindowAndCursorProps.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/System.Console/tests/WindowAndCursorProps.cs b/src/libraries/System.Console/tests/WindowAndCursorProps.cs index 10d0371d9a9031..82e7ad470be5d2 100644 --- a/src/libraries/System.Console/tests/WindowAndCursorProps.cs +++ b/src/libraries/System.Console/tests/WindowAndCursorProps.cs @@ -559,7 +559,7 @@ public void SetWindowPosition_Unix_ThrowsPlatformNotSupportedException() Assert.Throws(() => 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() { From 325a8629502accecad2922e634dd58e09cc5579f Mon Sep 17 00:00:00 2001 From: Tom Deseyn Date: Wed, 20 Dec 2023 10:17:18 +0100 Subject: [PATCH 5/5] Match exception parameter name with property name. --- src/libraries/System.Console/src/System/Console.cs | 4 ++-- src/libraries/System.Console/tests/WindowAndCursorProps.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Console/src/System/Console.cs b/src/libraries/System.Console/src/System/Console.cs index cf57c6d1210aa9..63736b59741706 100644 --- 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, "width"); + 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, "height"); + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(value, nameof(WindowHeight)); ConsolePal.WindowHeight = value; } diff --git a/src/libraries/System.Console/tests/WindowAndCursorProps.cs b/src/libraries/System.Console/tests/WindowAndCursorProps.cs index 82e7ad470be5d2..7ea07511594c46 100644 --- 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("width", () => Console.WindowWidth = value); + AssertExtensions.Throws("WindowWidth", () => Console.WindowWidth = value); } } @@ -89,7 +89,7 @@ public static void WindowHeight_SetInvalid_ThrowsArgumentOutOfRangeException(int } else { - AssertExtensions.Throws("height", () => Console.WindowHeight = value); + AssertExtensions.Throws("WindowHeight", () => Console.WindowHeight = value); } }