#50575 default case color behaviour is enabled #74349
Conversation
|
Tagging subscribers to this area: @dotnet/area-extensions-logging Issue DetailsUse the default color behavior, enabling color except when the console output is redirected.Enables color except when the console output is redirected. Default case should behave as enabled in this test case. Fix #50575 and #51398
|
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@mkhamoyan the test you are changing is failing the CI. Maybe it is better to fix it for Android and Apple OS's only? |
|
/azp run runtime-extra-platforms |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Totally forgot about other platforms :) Fixed it! |
| @@ -34,6 +32,9 @@ public void Log_WritingScopes_LogsWithCorrectColorsWhenColorEnabled(LoggerColorB | |||
| logger.Log(LogLevel.Information, 0, _state, null, _defaultFormatter); | |||
| } | |||
|
|
|||
| // For Android/iOS/tvOS/MacCatalyst default color behavior, enables the color | |||
| colorBehavior = colorBehavior == LoggerColorBehavior.Default && (PlatformDetection.IsAndroid || PlatformDetection.IsAppleMobile) ? LoggerColorBehavior.Enabled : colorBehavior; | |||
There was a problem hiding this comment.
Why is the color enabled by default on Android and Apple?
There was a problem hiding this comment.
From documentation LoggerColorBehavior.Default means color is not enabled when the console output is redirected.
In case of Android/Apple it was enabled which I'm assuming means console output is not redirected on this platforms.
https://github.com/dotnet/runtime/blob/main/src/libraries/Microsoft.Extensions.Logging.Console/src/SimpleConsoleFormatter.cs#L160
https://github.com/dotnet/runtime/blob/main/src/libraries/Common/src/System/Console/ConsoleUtils.cs#L31
There was a problem hiding this comment.
We shouldn't be outputting color codes for Android or the Apple mobile platforms, they have no shell* and all the output gets redirected to some log file.
(*) Android kinda does via adb shell, but it's not meant for running apps.
There was a problem hiding this comment.
Is the issue here Console.IsOutputRedirected behave differently between Android/Apple Mobile than other platforms? If this is the case, the line https://source.dot.net/#Microsoft.Extensions.Logging.Console/SimpleConsoleFormatter.cs,161 will need to be fixed for Android and Apple mobile I guess.
There was a problem hiding this comment.
I will revert this PR and create new one.
Microsoft.Extensions.Logging.Console.LoggerColorBehavior.Default
- Use the default color behavior, enabling color except when the console output is redirected.
Default case should behave as enabled in this test case.
Fix #50575 and #51398