From 0406a5361efebf4f1260f4d1366f3ad575d47790 Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Thu, 22 Jun 2017 20:11:34 +1000 Subject: [PATCH 1/3] Fixes and tidy-up - Always use display formatting at the top level for scalars - `ConsoleThemeStyle.Object` -> `ConsoleThemeStyle.Scalar` - Color JSON based on the underlying capture type to distinguish actual strings from other scalars --- .../Formatting/ThemedDisplayValueFormatter.cs | 4 +- .../Formatting/ThemedJsonValueFormatter.cs | 56 +++++++++++-------- .../Formatting/ThemedValueFormatter.cs | 2 +- .../Formatting/ThemedValueFormatterState.cs | 1 + .../SystemConsole/Themes/AnsiConsoleThemes.cs | 6 +- .../SystemConsole/Themes/ConsoleThemeStyle.cs | 11 +++- .../Themes/SystemConsoleThemes.cs | 4 +- .../ThemedMessageTemplateRendererTests.cs | 13 +++-- 8 files changed, 59 insertions(+), 38 deletions(-) diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedDisplayValueFormatter.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedDisplayValueFormatter.cs index 5760d88..07bab37 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedDisplayValueFormatter.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedDisplayValueFormatter.cs @@ -184,7 +184,7 @@ value is decimal || value is byte || value is sbyte || value is short || if (value is char ch) { - using (ApplyStyle(output, ConsoleThemeStyle.String, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Scalar, ref count)) { output.Write('\''); output.Write(ch); @@ -194,7 +194,7 @@ value is decimal || value is byte || value is sbyte || value is short || } } - using (ApplyStyle(output, ConsoleThemeStyle.Object, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Scalar, ref count)) scalar.Render(output, format, _formatProvider); return count; diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedJsonValueFormatter.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedJsonValueFormatter.cs index 39309b5..a8c7ec8 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedJsonValueFormatter.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedJsonValueFormatter.cs @@ -42,7 +42,12 @@ protected override int VisitScalarValue(ThemedValueFormatterState state, ScalarV { if (scalar == null) throw new ArgumentNullException(nameof(scalar)); - return FormatLiteralValue(scalar, state.Output, state.Format); + + // At the top level, for scalar values, use "display" rendering. + if (state.IsTopLevel) + return _displayFormatter.FormatLiteralValue(scalar, state.Output, state.Format); + + return FormatLiteralValue(scalar, state.Output); } protected override int VisitSequenceValue(ThemedValueFormatterState state, SequenceValue sequence) @@ -63,7 +68,7 @@ protected override int VisitSequenceValue(ThemedValueFormatterState state, Seque state.Output.Write(delim); delim = ", "; - Visit(state, sequence.Elements[index]); + Visit(state.Nest(), sequence.Elements[index]); } using (ApplyStyle(state.Output, ConsoleThemeStyle.TertiaryText, ref count)) @@ -96,8 +101,9 @@ protected override int VisitStructureValue(ThemedValueFormatterState state, Stru using (ApplyStyle(state.Output, ConsoleThemeStyle.TertiaryText, ref count)) state.Output.Write(": "); - count += Visit(state, property.Value); + count += Visit(state.Nest(), property.Value); } + if (structure.TypeTag != null) { using (ApplyStyle(state.Output, ConsoleThemeStyle.TertiaryText, ref count)) @@ -121,7 +127,7 @@ protected override int VisitStructureValue(ThemedValueFormatterState state, Stru protected override int VisitDictionaryValue(ThemedValueFormatterState state, DictionaryValue dictionary) { - int count = 0; + var count = 0; using (ApplyStyle(state.Output, ConsoleThemeStyle.TertiaryText, ref count)) state.Output.Write('{'); @@ -135,13 +141,19 @@ protected override int VisitDictionaryValue(ThemedValueFormatterState state, Dic delim = ", "; - using (ApplyStyle(state.Output, ConsoleThemeStyle.String, ref count)) + var style = element.Key.Value == null + ? ConsoleThemeStyle.Null + : element.Key.Value is string + ? ConsoleThemeStyle.String + : ConsoleThemeStyle.Scalar; + + using (ApplyStyle(state.Output, style, ref count)) JsonValueFormatter.WriteQuotedJsonString((element.Key.Value ?? "null").ToString(), state.Output); using (ApplyStyle(state.Output, ConsoleThemeStyle.TertiaryText, ref count)) state.Output.Write(": "); - count += Visit(state, element.Value); + count += Visit(state.Nest(), element.Value); } using (ApplyStyle(state.Output, ConsoleThemeStyle.TertiaryText, ref count)) @@ -150,12 +162,8 @@ protected override int VisitDictionaryValue(ThemedValueFormatterState state, Dic return count; } - int FormatLiteralValue(ScalarValue scalar, TextWriter output, string format) + int FormatLiteralValue(ScalarValue scalar, TextWriter output) { - // At the top level, if a format string is specified, non-JSON rendering is used. - if (format != null) - return _displayFormatter.FormatLiteralValue(scalar, output, format); - var value = scalar.Value; var count = 0; @@ -175,7 +183,7 @@ int FormatLiteralValue(ScalarValue scalar, TextWriter output, string format) if (value is ValueType) { - if (value is int || value is uint || value is long || value is ulong || value is decimal || value is byte || (value is sbyte || value is short) || value is ushort) + if (value is int || value is uint || value is long || value is ulong || value is decimal || value is byte || value is sbyte || value is short || value is ushort) { using (ApplyStyle(output, ConsoleThemeStyle.Number, ref count)) output.Write(((IFormattable)value).ToString(null, CultureInfo.InvariantCulture)); @@ -184,23 +192,25 @@ int FormatLiteralValue(ScalarValue scalar, TextWriter output, string format) if (value is double d) { - if (double.IsNaN(d) || double.IsInfinity(d)) - using (ApplyStyle(output, ConsoleThemeStyle.String, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Number, ref count)) + { + if (double.IsNaN(d) || double.IsInfinity(d)) JsonValueFormatter.WriteQuotedJsonString(d.ToString(CultureInfo.InvariantCulture), output); - else - using (ApplyStyle(output, ConsoleThemeStyle.Number, ref count)) + else output.Write(d.ToString("R", CultureInfo.InvariantCulture)); + } return count; } if (value is float f) { - if (double.IsNaN(f) || double.IsInfinity(f)) - using (ApplyStyle(output, ConsoleThemeStyle.String, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Number, ref count)) + { + if (double.IsNaN(f) || double.IsInfinity(f)) JsonValueFormatter.WriteQuotedJsonString(f.ToString(CultureInfo.InvariantCulture), output); - else - using (ApplyStyle(output, ConsoleThemeStyle.Number, ref count)) + else output.Write(f.ToString("R", CultureInfo.InvariantCulture)); + } return count; } @@ -214,14 +224,14 @@ int FormatLiteralValue(ScalarValue scalar, TextWriter output, string format) if (value is char ch) { - using (ApplyStyle(output, ConsoleThemeStyle.String, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Scalar, ref count)) JsonValueFormatter.WriteQuotedJsonString(ch.ToString(), output); return count; } if (value is DateTime || value is DateTimeOffset) { - using (ApplyStyle(output, ConsoleThemeStyle.String, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Scalar, ref count)) { output.Write('"'); output.Write(((IFormattable)value).ToString("O", CultureInfo.InvariantCulture)); @@ -231,7 +241,7 @@ int FormatLiteralValue(ScalarValue scalar, TextWriter output, string format) } } - using (ApplyStyle(output, ConsoleThemeStyle.String, ref count)) + using (ApplyStyle(output, ConsoleThemeStyle.Scalar, ref count)) JsonValueFormatter.WriteQuotedJsonString(value.ToString(), output); return count; diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs index cb9bc6d..3048e13 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs @@ -36,7 +36,7 @@ protected StyleReset ApplyStyle(TextWriter output, ConsoleThemeStyle style, ref public int Format(LogEventPropertyValue value, TextWriter output, string format) { - return Visit(new ThemedValueFormatterState { Output = output, Format = format }, value); + return Visit(new ThemedValueFormatterState { Output = output, Format = format, IsTopLevel = true }, value); } public abstract ThemedValueFormatter SwitchTheme(ConsoleTheme theme); diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatterState.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatterState.cs index cf9e948..1c720c0 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatterState.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatterState.cs @@ -20,6 +20,7 @@ struct ThemedValueFormatterState { public TextWriter Output; public string Format; + public bool IsTopLevel; public ThemedValueFormatterState Nest() { diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/AnsiConsoleThemes.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/AnsiConsoleThemes.cs index efa7d59..d578ee0 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/AnsiConsoleThemes.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/AnsiConsoleThemes.cs @@ -30,7 +30,7 @@ static class AnsiConsoleThemes [ConsoleThemeStyle.String] = "\x1b[36;1m", [ConsoleThemeStyle.Number] = "\x1b[35;1m", [ConsoleThemeStyle.Boolean] = "\x1b[34;1m", - [ConsoleThemeStyle.Object] = "\x1b[32;1m", + [ConsoleThemeStyle.Scalar] = "\x1b[32;1m", [ConsoleThemeStyle.LevelVerbose] = "\x1b[37m", [ConsoleThemeStyle.LevelDebug] = "\x1b[37m", [ConsoleThemeStyle.LevelInformation] = "\x1b[37;1m", @@ -51,7 +51,7 @@ static class AnsiConsoleThemes [ConsoleThemeStyle.String] = "\x1b[1m\x1b[37;1m", [ConsoleThemeStyle.Number] = "\x1b[1m\x1b[37;1m", [ConsoleThemeStyle.Boolean] = "\x1b[1m\x1b[37;1m", - [ConsoleThemeStyle.Object] = "\x1b[1m\x1b[37;1m", + [ConsoleThemeStyle.Scalar] = "\x1b[1m\x1b[37;1m", [ConsoleThemeStyle.LevelVerbose] = "\x1b[30;1m", [ConsoleThemeStyle.LevelDebug] = "\x1b[30;1m", [ConsoleThemeStyle.LevelInformation] ="\x1b[37;1m", @@ -72,7 +72,7 @@ static class AnsiConsoleThemes [ConsoleThemeStyle.String] = "\x1b[38;5;0216m", [ConsoleThemeStyle.Number] = "\x1b[38;5;151m", [ConsoleThemeStyle.Boolean] = "\x1b[38;5;0038m", - [ConsoleThemeStyle.Object] = "\x1b[38;5;0079m", + [ConsoleThemeStyle.Scalar] = "\x1b[38;5;0079m", [ConsoleThemeStyle.LevelVerbose] = "\x1b[37m", [ConsoleThemeStyle.LevelDebug] = "\x1b[37m", [ConsoleThemeStyle.LevelInformation] = "\x1b[37;1m", diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/ConsoleThemeStyle.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/ConsoleThemeStyle.cs index 1db4663..2106801 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/ConsoleThemeStyle.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/ConsoleThemeStyle.cs @@ -12,6 +12,9 @@ // See the License for the specific language governing permissions and // limitations under the License. +using System; +using System.ComponentModel; + namespace Serilog.Sinks.SystemConsole.Themes { /// @@ -66,10 +69,16 @@ public enum ConsoleThemeStyle /// Boolean, + /// + /// All other scalar values, e.g. instances. + /// + Scalar, + /// /// Unrecogized literal values, e.g. instances. /// - Object, + [Obsolete("Use ConsoleThemeStyle.Scalar instead"), EditorBrowsable(EditorBrowsableState.Never)] + Object = Scalar, /// /// Level indicator. diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/SystemConsoleThemes.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/SystemConsoleThemes.cs index 5b307e0..c52b4f8 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/SystemConsoleThemes.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Themes/SystemConsoleThemes.cs @@ -31,7 +31,7 @@ static class SystemConsoleThemes [ConsoleThemeStyle.String] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Cyan }, [ConsoleThemeStyle.Number] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Magenta }, [ConsoleThemeStyle.Boolean] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Blue }, - [ConsoleThemeStyle.Object] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Green }, + [ConsoleThemeStyle.Scalar] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Green }, [ConsoleThemeStyle.LevelVerbose] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Gray }, [ConsoleThemeStyle.LevelDebug] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.Gray }, [ConsoleThemeStyle.LevelInformation] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, @@ -52,7 +52,7 @@ static class SystemConsoleThemes [ConsoleThemeStyle.String] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, [ConsoleThemeStyle.Number] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, [ConsoleThemeStyle.Boolean] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, - [ConsoleThemeStyle.Object] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, + [ConsoleThemeStyle.Scalar] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, [ConsoleThemeStyle.LevelVerbose] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.DarkGray }, [ConsoleThemeStyle.LevelDebug] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.DarkGray }, [ConsoleThemeStyle.LevelInformation] = new SystemConsoleThemeStyle { Foreground = ConsoleColor.White }, diff --git a/test/Serilog.Sinks.Console.Tests/Rendering/ThemedMessageTemplateRendererTests.cs b/test/Serilog.Sinks.Console.Tests/Rendering/ThemedMessageTemplateRendererTests.cs index 416a79c..18debf0 100644 --- a/test/Serilog.Sinks.Console.Tests/Rendering/ThemedMessageTemplateRendererTests.cs +++ b/test/Serilog.Sinks.Console.Tests/Rendering/ThemedMessageTemplateRendererTests.cs @@ -4,11 +4,9 @@ using System.Linq; using System.Text; using Xunit; -using Serilog.Core; using Serilog.Sinks.SystemConsole.Themes; using Serilog.Sinks.SystemConsole.Formatting; using Serilog.Sinks.SystemConsole.Rendering; -using MessageTemplateParser = Serilog.Parsing.MessageTemplateParser; namespace Serilog.Sinks.Console.Tests.Rendering { @@ -17,8 +15,9 @@ public class ThemedMessageTemplateRendererTests class Chair { // ReSharper disable UnusedMember.Local - public string Back { get { return "straight"; } } - public int[] Legs { get { return new[] { 1, 2, 3, 4 }; } } + public string Back => "straight"; + + public int[] Legs => new[] { 1, 2, 3, 4 }; // ReSharper restore UnusedMember.Local public override string ToString() { @@ -29,9 +28,11 @@ public override string ToString() class Receipt { // ReSharper disable UnusedMember.Local - public decimal Sum { get { return 12.345m; } } - public DateTime When { get { return new DateTime(2013, 5, 20, 16, 39, 0); } } + public decimal Sum => 12.345m; + + public DateTime When => new DateTime(2013, 5, 20, 16, 39, 0); // ReSharper restore UnusedMember.Local + public override string ToString() { return "a receipt"; From 46674e65fd4d8c1af1fc2062570898812c4f4a80 Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Thu, 22 Jun 2017 20:46:43 +1000 Subject: [PATCH 2/3] Fix tests - verify second-level formatting, since top-level values now always use 'display' format --- .../Formatting/ThemedJsonValueFormatterTests.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/Serilog.Sinks.Console.Tests/Formatting/ThemedJsonValueFormatterTests.cs b/test/Serilog.Sinks.Console.Tests/Formatting/ThemedJsonValueFormatterTests.cs index cc40b42..3e28425 100644 --- a/test/Serilog.Sinks.Console.Tests/Formatting/ThemedJsonValueFormatterTests.cs +++ b/test/Serilog.Sinks.Console.Tests/Formatting/ThemedJsonValueFormatterTests.cs @@ -20,8 +20,9 @@ public TestThemedJsonValueFormatter() public string Format(object literal) { var output = new StringWriter(); - Format(new ScalarValue(literal), output, null); - return output.ToString(); + Format(new SequenceValue(new [] {new ScalarValue(literal)}), output, null); + var o = output.ToString(); + return o.Substring(1, o.Length - 2); } } From bea3a621b740d4bf3fa2e7a27475184f55218574 Mon Sep 17 00:00:00 2001 From: Nicholas Blumhardt Date: Fri, 23 Jun 2017 08:18:23 +1000 Subject: [PATCH 3/3] Final tweak - only use literal formatting at the top level when opted-in. --- .../Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs | 4 ++-- .../SystemConsole/Rendering/ThemedMessageTemplateRenderer.cs | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs index 3048e13..ea780e0 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Formatting/ThemedValueFormatter.cs @@ -34,9 +34,9 @@ protected StyleReset ApplyStyle(TextWriter output, ConsoleThemeStyle style, ref return _theme.Apply(output, style, ref invisibleCharacterCount); } - public int Format(LogEventPropertyValue value, TextWriter output, string format) + public int Format(LogEventPropertyValue value, TextWriter output, string format, bool literalTopLevel = false) { - return Visit(new ThemedValueFormatterState { Output = output, Format = format, IsTopLevel = true }, value); + return Visit(new ThemedValueFormatterState { Output = output, Format = format, IsTopLevel = literalTopLevel }, value); } public abstract ThemedValueFormatter SwitchTheme(ConsoleTheme theme); diff --git a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Rendering/ThemedMessageTemplateRenderer.cs b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Rendering/ThemedMessageTemplateRenderer.cs index de41648..9fa9ca4 100644 --- a/src/Serilog.Sinks.Console/Sinks/SystemConsole/Rendering/ThemedMessageTemplateRenderer.cs +++ b/src/Serilog.Sinks.Console/Sinks/SystemConsole/Rendering/ThemedMessageTemplateRenderer.cs @@ -126,7 +126,7 @@ int RenderAlignedPropertyTokenUnbuffered(PropertyToken pt, TextWriter output, Lo int RenderValue(ConsoleTheme theme, ThemedValueFormatter valueFormatter, LogEventPropertyValue propertyValue, TextWriter output, string format) { - if (_isLiteral && propertyValue is ScalarValue sv && (sv.Value is string str || sv.Value is char ch)) + if (_isLiteral && propertyValue is ScalarValue sv && sv.Value is string) { var count = 0; using (theme.Apply(output, ConsoleThemeStyle.String, ref count)) @@ -134,7 +134,7 @@ int RenderValue(ConsoleTheme theme, ThemedValueFormatter valueFormatter, LogEven return count; } - return valueFormatter.Format(propertyValue, output, format); + return valueFormatter.Format(propertyValue, output, format, _isLiteral); } } }