remove dependency to System.CommandLine.Rendering #46209
Conversation
…nsions.Logging.Console instead
ghost
added
| string? verbosity, | ||
| string? binarylog, | ||
| string? report, | ||
| IConsole console); |
There was a problem hiding this comment.
this delegate was using IConsole abstraction from System.CommandLine.Rendering. Initially I just wanted to update it, but then I got to the conclusion that it's not being used anywhere and can be deleted. But I can be wrong as not all VS features work for me when I work with a local build of the SDK.
There was a problem hiding this comment.
Seems like we missed removing this in a refactor at some point. Thanks!
|
I've added the NO MERGE label to ensure that it's not merged before the Preview 1 snap. |
|
@adamsitnik I tried out using the SimpleConsoleLogger from Logging.Console and it doesn't give the experience we are looking for. Always writing a "info:" prefix clutters the output. Could we add a simpler logger like the one implemented in dotnet/roslyn-tools@c60d28f? |
@JoeRobich big thanks for giving it a try!
In such a case I am going to revert some of my changes and bring back the old behavior, just without using |
| !(OperatingSystem.IsBrowser() || OperatingSystem.IsAndroid() || OperatingSystem.IsIOS() || OperatingSystem.IsTvOS()) | ||
| && !Console.IsOutputRedirected; | ||
|
|
||
| private readonly static object _gate = new object(); |
There was a problem hiding this comment.
I made the gate static, as Console is a static singleton and I expect that we want to prevent from multiple threads changing the color.
| internal class SimpleConsoleLogger : ILogger | ||
| { | ||
| private readonly object _gate = new object(); | ||
| private static readonly bool ColorsAreSupported = |
There was a problem hiding this comment.
|
@JoeRobich done, PTAL |
Looks good to me. Thanks! |
|
/ba-g the tiemouts are know infra problems, but despite that the Build Analysis is red
|
* remove the dependency to System.System.CommandLine.Rendering similarly to what I did in dotnet/sdk#46209 * update to latest System.CommandLine * add the missing nuget feed * Apply suggestions from code review
2 participants
We are no longer going to support System.CommandLine.Rendering.
The
SystemConsolethat was used bydotnet formatwas just a wrapper aroundSystem.Console: https://github.com/dotnet/command-line-api/blob/3bbb940ceeb3254790899d751a8d418348563d40/src/System.CommandLine.Rendering/IO/SystemConsole.cs#L9Since other projects in this repo were already using Microsoft.Extensions.Logging.Console, it felt like the best alternative.