Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,6 @@ System.CommandLine
public static class CompletionSourceExtensions
public static System.Void Add(this System.Collections.Generic.List<System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem>>> completionSources, System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.String>> completionsDelegate)
public static System.Void Add(this System.Collections.Generic.List<System.Func<System.CommandLine.Completions.CompletionContext,System.Collections.Generic.IEnumerable<System.CommandLine.Completions.CompletionItem>>> completionSources, System.String[] completions)
public static class ConsoleExtensions
public static System.Void Write(this IConsole console, System.String value)
public static System.Void WriteLine(this IConsole console, System.String value)
public class Directive : Symbol
.ctor(System.String name)
public CliAction Action { get; set; }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ private sealed class CustomCliAction : CliAction

public async override Task<int> InvokeAsync(InvocationContext context, CancellationToken cancellationToken = default)
{
context.Console.WriteLine(ChildProcessWaiting);
Console.WriteLine(ChildProcessWaiting);

bool infiniteDelay = context.GetValue(InfiniteDelayOption);

Expand Down
4 changes: 2 additions & 2 deletions src/System.CommandLine.Tests/TestApps/NativeAOT/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ private static int Main(string[] args)

void Run(InvocationContext context)
{
context.Console.WriteLine($"Bool option: {context.ParseResult.GetValue(boolOption)}");
context.Console.WriteLine($"String option: {context.ParseResult.GetValue(stringOption)}");
Console.WriteLine($"Bool option: {context.ParseResult.GetValue(boolOption)}");
Console.WriteLine($"String option: {context.ParseResult.GetValue(stringOption)}");
}
}
}
2 changes: 1 addition & 1 deletion src/System.CommandLine.Tests/TestApps/Trimming/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

command.SetAction(context =>
{
context.Console.Write($"The file you chose was: {context.ParseResult.GetValue(fileArgument)}");
Console.Write($"The file you chose was: {context.ParseResult.GetValue(fileArgument)}");
});

command.Invoke(args);
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@ int Default(Exception exception, InvocationContext context)
{
if (exception is not OperationCanceledException)
{
context.Console.ResetTerminalForegroundColor();
context.Console.SetTerminalForegroundRed();
ConsoleHelpers.ResetTerminalForegroundColor();
ConsoleHelpers.SetTerminalForegroundRed();

context.Console.Error.Write(LocalizationResources.ExceptionHandlerHeader());
context.Console.Error.WriteLine(exception.ToString());

context.Console.ResetTerminalForegroundColor();
ConsoleHelpers.ResetTerminalForegroundColor();
}
return errorExitCode;
}
Expand Down
29 changes: 0 additions & 29 deletions src/System.CommandLine/ConsoleExtensions.cs

This file was deleted.

39 changes: 39 additions & 0 deletions src/System.CommandLine/ConsoleHelpers.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
// Copyright (c) .NET Foundation and contributors. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

using System.Runtime.InteropServices;

namespace System.CommandLine
{
internal static class ConsoleHelpers
{
private static readonly bool ColorsAreSupported = GetColorsAreSupported();

private static bool GetColorsAreSupported()
#if NET7_0_OR_GREATER
=> !(OperatingSystem.IsBrowser() || OperatingSystem.IsAndroid() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS())
#else
=> !(RuntimeInformation.IsOSPlatform(OSPlatform.Create("BROWSER"))
|| RuntimeInformation.IsOSPlatform(OSPlatform.Create("ANDROID"))
|| RuntimeInformation.IsOSPlatform(OSPlatform.Create("IOS"))
|| RuntimeInformation.IsOSPlatform(OSPlatform.Create("TVOS")))
#endif
&& !Console.IsOutputRedirected;

internal static void SetTerminalForegroundRed()
{
if (ColorsAreSupported)
{
Console.ForegroundColor = ConsoleColor.Red;
}
}

internal static void ResetTerminalForegroundColor()
{
if (ColorsAreSupported)
{
Console.ResetColor();
}
}
}
}
58 changes: 0 additions & 58 deletions src/System.CommandLine/IO/ConsoleExtensions.cs

This file was deleted.

6 changes: 3 additions & 3 deletions src/System.CommandLine/Invocation/ParseErrorResult.cs
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@ internal sealed class ParseErrorResultAction : CliAction
{
public override int Invoke(InvocationContext context)
{
context.Console.ResetTerminalForegroundColor();
context.Console.SetTerminalForegroundRed();
ConsoleHelpers.ResetTerminalForegroundColor();
ConsoleHelpers.SetTerminalForegroundRed();
Comment on lines 13 to +16

@KalleOlaviNiemitalo KalleOlaviNiemitalo Mar 17, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If an application is designed to parse and invoke commands from custom sources like network sockets, and it should not modify System.Console.ForegroundColor during these operations (because such changes might mess up output from a different thread), then it apparently needs to avoid UseParseErrorReporting entirely and avoid UseExceptionHandler with the default handler. That's tolerable but not quite obvious.

If you're eventually going to have TextWriter Error, then perhaps you can compare Error == System.Console.Error and change Console.ForegroundColor only in that matches. Or have some bool AllowConsoleColor property in CommandLineConfiguration.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking on — if class CliConfiguration is going to have TextWriter Error, then it can have bool UseErrorColor { get; set; } such that…

  • If UseErrorColor has not been set, then its getter computes the value from this.Error == System.Console.Error && !System.Console.IsErrorRedirected, defaulting to false on PlatformNotSupportedException. Does not cache the result into a field of CliConfiguration because it's going to be used only once per error. Instead, each method that reads this property should save the result to a local so that it restores the colours if and only if it has set them.
  • If UseErrorColor has been set, then trust the application-assigned value when deciding whether to set the foreground color, and do not check whether error has been redirected.

Not sure about IsErrorRedirected vs. IsOutputRedirected. The error text should go to the CliConfiguration.Error TextWriter, but if System.Console.ForegroundColor outputs ECMA-48 control sequences instead of using the Windows console API, I guess it cannot be told whether those should go to Console.Error or Console.Out. So perhaps the UseErrorColor getter should check this.Error == System.Console.Error && !System.Console.IsErrorRedirected && !System.Console.IsOutputRedirected. I'm omitting this.Out == System.Console.Out because errors should never go to this.Out.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When https://github.com/dotnet/runtime/blob/5edef4b20babd4c3ddac7460e536f86fd0f2d724/src/libraries/System.Console/src/System/ConsolePal.Unix.cs sets colors, it does lock (Console.Out) but writes to STDOUT_FILENO. So Console.SetOut(TextWriter) affects the locking but does not affect where the SGR control sequence goes.


foreach (var error in context.ParseResult.Errors)
{
Expand All @@ -22,7 +22,7 @@ public override int Invoke(InvocationContext context)

context.Console.Error.WriteLine();

context.Console.ResetTerminalForegroundColor();
ConsoleHelpers.ResetTerminalForegroundColor();

new HelpOption().Action!.Invoke(context);

Expand Down