Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix default log color behavior when console is redirected #70504

Merged
merged 7 commits into from
Jun 20, 2022
Merged
Show file tree
Hide file tree
Changes from 4 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 @@ -21,7 +21,7 @@ public class ConsoleLoggerProvider : ILoggerProvider, ISupportExternalScope
private readonly IOptionsMonitor<ConsoleLoggerOptions> _options;
private readonly ConcurrentDictionary<string, ConsoleLogger> _loggers;
private ConcurrentDictionary<string, ConsoleFormatter> _formatters;
private readonly ConsoleLoggerProcessor _messageQueue;
private ConsoleLoggerProcessor _messageQueue;
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved

private IDisposable? _optionsReloadToken;
private IExternalScopeProvider _scopeProvider = NullExternalScopeProvider.Instance;
Expand Down Expand Up @@ -59,6 +59,7 @@ public ConsoleLoggerProvider(IOptionsMonitor<ConsoleLoggerOptions> options, IEnu
console = new AnsiParsingLogConsole();
errorConsole = new AnsiParsingLogConsole(stdErr: true);
}

maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
_messageQueue = new ConsoleLoggerProcessor(console, errorConsole);
}

Expand Down Expand Up @@ -162,7 +163,7 @@ private static void UpdateFormatterOptions(ConsoleFormatter formatter, ConsoleLo
{
defaultFormatter.FormatterOptions = new SimpleConsoleFormatterOptions()
{
ColorBehavior = deprecatedFromOptions.DisableColors ? LoggerColorBehavior.Disabled : LoggerColorBehavior.Enabled,
ColorBehavior = deprecatedFromOptions.DisableColors ? LoggerColorBehavior.Disabled : LoggerColorBehavior.Default,
IncludeScopes = deprecatedFromOptions.IncludeScopes,
TimestampFormat = deprecatedFromOptions.TimestampFormat,
UseUtcTimestamp = deprecatedFromOptions.UseUtcTimestamp,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ namespace Microsoft.Extensions.Logging.Console
{
internal sealed class SimpleConsoleFormatter : ConsoleFormatter, IDisposable
{
/// <summary>Whether to output ansi color strings.</summary>
private static volatile int s_emitAnsiColorCodes = -1;
private const string LoglevelPadding = ": ";
private static readonly string _messagePadding = new string(' ', GetLogLevelString(LogLevel.Information).Length + LoglevelPadding.Length);
private static readonly string _newLineWithMessagePadding = Environment.NewLine + _messagePadding;
Expand Down Expand Up @@ -155,10 +157,50 @@ private static string GetLogLevelString(LogLevel logLevel)
};
}

/// <summary>Get whether to emit ANSI color codes.</summary>
private static bool EmitAnsiColorCodes
{
get
{
// The flag starts at -1. If it's no longer -1, it's 0 or 1 to represent false or true.
int emitAnsiColorCodes = s_emitAnsiColorCodes;
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved
if (emitAnsiColorCodes != -1)
{
return Convert.ToBoolean(emitAnsiColorCodes);
}

// We've not yet computed whether to emit codes or not. Do so now. We may race with
// other threads, and that's ok; this is idempotent unless someone is currently changing
// the value of the relevant environment variables, in which case behavior here is undefined.

// By default, we emit ANSI color codes if output isn't redirected, and suppress them if output is redirected.
bool enabled = !System.Console.IsOutputRedirected;

if (enabled)
{
// We subscribe to the informal standard from https://no-color.org/. If we'd otherwise emit
// ANSI color codes but the NO_COLOR environment variable is set, disable emitting them.
enabled = Environment.GetEnvironmentVariable("NO_COLOR") is null;
}
else
{
// We also support overriding in the other direction. If we'd otherwise avoid emitting color
// codes but the DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION environment variable is
// set to 1 or true, enable color.
string? envVar = Environment.GetEnvironmentVariable("DOTNET_SYSTEM_CONSOLE_ALLOW_ANSI_COLOR_REDIRECTION");
enabled = envVar is not null && (envVar == "1" || envVar.Equals("true", StringComparison.OrdinalIgnoreCase));
}
maryamariyan marked this conversation as resolved.
Show resolved Hide resolved

// Store and return the computed answer.
s_emitAnsiColorCodes = Convert.ToInt32(enabled);
return enabled;
}
}

private ConsoleColors GetLogLevelConsoleColors(LogLevel logLevel)
{
bool disableColors = (FormatterOptions.ColorBehavior == LoggerColorBehavior.Disabled) ||
(FormatterOptions.ColorBehavior == LoggerColorBehavior.Default && System.Console.IsOutputRedirected);
(FormatterOptions.ColorBehavior == LoggerColorBehavior.Default && !EmitAnsiColorCodes);
if (disableColors)
{
return new ConsoleColors(null, null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,10 @@
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Text.RegularExpressions;
using Microsoft.DotNet.RemoteExecutor;
using Microsoft.Extensions.Configuration;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging.Test.Console;
Expand Down Expand Up @@ -413,6 +415,36 @@ public void WriteInformation_LogsCorrectColors()
Assert.Equal(TestConsole.DefaultForegroundColor, write.ForegroundColor);
}

[ConditionalFact(nameof(IsThreadingAndRemoteExecutorSupported))]
public void AddConsole_IsOutputRedirected_ColorDisabled()
{
RemoteExecutor.Invoke(() =>
{
// Arrange
var sink = new ConsoleSink();
var console = new TestConsole(sink);
var loggerOptions = new ConsoleLoggerOptions();
var consoleLoggerProcessor = new TestLoggerProcessor(console, null!);

var loggerProvider = new ServiceCollection()
.AddLogging(builder => builder
.AddConsole()
)
.BuildServiceProvider()
.GetRequiredService<ILoggerProvider>();

var consoleLoggerProvider = Assert.IsType<ConsoleLoggerProvider>(loggerProvider);
var logger = (ConsoleLogger)consoleLoggerProvider.CreateLogger("Category");

// Assert
Assert.True(System.Console.IsOutputRedirected);
Assert.Null(logger.Options.FormatterName);
var formatter = Assert.IsType<SimpleConsoleFormatter>(logger.Formatter);
Assert.Equal(LoggerColorBehavior.Default, formatter.FormatterOptions.ColorBehavior);

}, new RemoteInvokeOptions { StartInfo = new ProcessStartInfo() { RedirectStandardOutput = true } }).Dispose();
}

[ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsThreadingSupported))]
public void WriteDebug_LogsCorrectColors()
{
Expand Down Expand Up @@ -1365,6 +1397,9 @@ private string CreateHeader(ConsoleLoggerFormat format, int eventId = 0)
throw new ArgumentOutOfRangeException(nameof(format));
}
}

public static bool IsThreadingAndRemoteExecutorSupported =>
PlatformDetection.IsThreadingSupported && RemoteExecutor.IsSupported;
}

internal class TestLoggerProcessor : ConsoleLoggerProcessor
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<IncludeRemoteExecutor>true</IncludeRemoteExecutor>
<TargetFrameworks>$(NetCoreAppCurrent);$(NetFrameworkMinimum)</TargetFrameworks>
<TestRuntime>true</TestRuntime>
<EnableDefaultItems>true</EnableDefaultItems>
Expand Down