Skip to content

Conversation

@nblumhardt
Copy link
Member

Always use display formatting at the top level for scalars - it turns out that it seems odd to see a DateTime end up as ISO-8601 in a log message.

ConsoleThemeStyle.Object -> ConsoleThemeStyle.Scalar (obsoleting the old name). This name was carried over from the old literate console sink and isn't representative of what the color means, which is, a scalar-captured value of type other than number, bool or string.

Color JSON based on the underlying capture type to distinguish actual strings from other scalars.

Ugly example, showing the difference between rendering of a top-level DateTime and one embedded within a structure. Also note that only the actual string uses ConsoleThemeStyle.String - the number, rendered as the string "NaN", uses number coloring, and likewise the DateTime "Utc" uses scalar coloring.

image

In real log data, where most entries are real strings and regular numbers, this isn't so ridiculously colorful ;-)

 - 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
@nblumhardt
Copy link
Member Author

Tests should be sorted now.

It's tough getting the right balance between predictability and ergonomics. I really put this sink through its paces today debugging some new code in Seq - I think these are the somewhat annoying quirks in the new version, should be super awesome now :-)

@nblumhardt
Copy link
Member Author

Folks, I'm keen to push this one through since a lot of people are just now trying the updated sink for the first time, and will find some of these quirks (particularly the first one - using JSON-style formatting a bit over-zealously) a bit odd. Going to hit the button when these tests pass (if, I suppose ;-))

@nblumhardt nblumhardt merged commit 8ebc371 into serilog:dev Jun 22, 2017
@nblumhardt nblumhardt mentioned this pull request Jun 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant