-
-
Notifications
You must be signed in to change notification settings - Fork 96
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
console work #258
console work #258
Conversation
Hey @AnthonyLloyd this is looking really good! I'm curious why you are choosing to create a new logger instead of modifying the Is it possible (and easier?) to provide a new |
Here's a couple of good resources for ANSI codes and
|
I didn't really think of just creating a new outputWriter. I think because I needed to add Flush. I'll have a look at doing this as I integrate the progress indicator and flush on error. Thanks for the idea. I tested the SetConsoleMode and it didn't have any effect. I think this is switched on by default now in Windows 10? Should possible consider for legacy versions. |
Hey @AnthonyLloyd, as far as I can tell, it's not going to be just a "legacy" issue. Applications need to "opt in" to VT mode, and if you want to output ANSI sequences, you'll need to "opt in". When I pulled down this PR to try it out, I actually got the ANSI escape sequences shown on screen instead of the (expected) colours. See here for information directly from an MS person with knowledge of the situation: microsoft/WSL#1173 (comment)
|
@adamchester Thanks, I'll add this in, best to be safe. |
|
||
let finishTime = | ||
lazy | ||
totalTicks |> (+) (Stopwatch.GetTimestamp()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remember that Stopwatch's ticks differs between operating systems and is not the same as DateTime/BCL ticks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you need to use Stopwatch.Frequency to convert between seconds and Stopwatch ticks. It does this in the lines just above here.
Expecto/Logging.fs
Outdated
@@ -12,10 +12,12 @@ | |||
/// | |||
/// Changes: | |||
/// Changed namespace to Expecto.Logging and file name to Logging.fs - Anthony Lloyd - 11 Jun 2018 | |||
/// Add IFlushable - Anthony Lloyd - 19 Jun 2018 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why keep a changelog at the top? Feels a bit year 2000.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read that it was a good thing to do when making changes to Apache code. To help when back porting the code. Could be got from git but it contains a bit more specific info.
This PR aims to resolve #256 #249 #248 #230 #190 #161 #152 #147