initial commit for the console logger #29
Conversation
| { | ||
| private readonly string _name; | ||
|
|
||
| public ConsoleLogger(string name) |
|
FYI, there's a great component (AnsiConsole) in the |
| return true; | ||
| } | ||
| var color = System.Console.ForegroundColor; // save current color | ||
| var severity = traceType.ToString().ToUpper(); |
There was a problem hiding this comment.
Should use .ToUpperInvariant() to prevent the current locale from affecting this.
sonjakhan
force-pushed
the
console-logger
branch
from
b42b1be to
8ffba94
Compare
| public class ConsoleLogger : ILogger | ||
| { | ||
| private readonly string _name; | ||
| private Func<string, TraceType, bool> _shouldLog; |
|
Need to figure out how to write some unit tests for this. You could create an abstraction of the |
|
@Eilon we could just take a TextWriter, and pass in Console.Out and Console.Error by default. The unit tests can pass a StringWriter |
|
@davidfowl need to abstract the console color stuff too. In part that's important so that something like xUnit doesn't spew all kinds of random colors into the real console when the tests run 😄 |
|
Oh snap, console colors... |
| @@ -27,6 +35,16 @@ Global | |||
| Release|x86 = Release|x86 | |||
| EndGlobalSection | |||
| GlobalSection(ProjectConfigurationPlatforms) = postSolution | |||
| {19D1B6C5-8A62-4387-8816-C54874D1DF5F}.aspnet50|Any CPU.ActiveCfg = Release|Any CPU | |||
There was a problem hiding this comment.
Something bizarre going on here
There was a problem hiding this comment.
I'm not sure how to fix this
There was a problem hiding this comment.
You could try reverting the SLN file and then adding the existing project (kproj) back into the solution.
sonjakhan
force-pushed
the
console-logger
branch
3 times, most recently
from
e3cf807 to
7fa1263
Compare
|
Added tests, accidentally squashed my commits |
| SetConsoleColor(traceType); | ||
| try | ||
| { | ||
| Console.WriteLine("[{0}:{1}] {2}", severity, _name, message); |
There was a problem hiding this comment.
Should we write Critical \ Error traces to Console.Error?
sonjakhan
force-pushed
the
console-logger
branch
from
fcba4d0 to
eb543ef
Compare
| }, | ||
| "aspnetcore50": { | ||
| "dependencies": { | ||
| "System.Runtime": "4.0.20.0", |
| @@ -31,9 +34,12 @@ public void Main(string[] args) | |||
| } | |||
| catch (Exception ex) | |||
| { | |||
| _logger.WriteError("Unexpected error starting application", ex); | |||
| _logger.WriteCritical("Unexpected error starting application", ex); | |||
There was a problem hiding this comment.
Recommend changing the messages here to say something like Unexpected critical error / error / warning starting application. etc. so that it's clear that the right output is being produced for each call.
|
Overall looks great! Just had a few small comments, so once those are addressed, |
!
This is a very basic implementation and is missing scoping altogether. Just wanted to put this out there for some feedback. The colors and format match the NLog configuration in the sample app, as evidenced by the image below.
