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.

Sign up for GitHub

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

Jump to bottom

ColoredConsoleTarget - Allow using Condition-logic for word highlighting rules #3891

Merged
merged 2 commits into from
Apr 20, 2020
Merged

ColoredConsoleTarget - Allow using Condition-logic for word highlighting rules #3891

merged 2 commits into from
Apr 20, 2020

Conversation

snakefoot
Copy link
Contributor

@snakefoot snakefoot commented Apr 14, 2020 •
edited
Loading

Condition-Logic is also faster than RegEx. Especially when combined with #3894

@snakefoot
Copy link
Contributor Author

Before tuning:

image

After tuning (Faster handling of newlines and reduced number of console color changes):

image

@304NotModified
Copy link
Member

Cool!

Would be nice if we could cover these lines:

image

image

@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 14, 2020 •
edited
Loading

Would be nice if we could cover these lines:

Yes I'm missing unit-tests. Just playing around with comparing Microsoft AddConsole and NLog ColoredConsole. Noticed that Microsoft AddConsole is not cheap.

See also: https://github.com/NLog/NLog.Extensions.Logging/wiki/NLog-Console-and-AddConsole

@snakefoot snakefoot force-pushed the ColorConsoleWordConditions branch from dd2c594 to 21d720f Compare April 14, 2020 22:37
@snakefoot snakefoot added this to the 4.7.1 milestone Apr 15, 2020
@pull-request-size pull-request-size bot added size/M and removed size/S labels Apr 15, 2020
@snakefoot snakefoot force-pushed the ColorConsoleWordConditions branch from 21d720f to 3029d44 Compare April 18, 2020 09:48
@pull-request-size pull-request-size bot added size/L and removed size/M labels Apr 18, 2020
@snakefoot snakefoot force-pushed the ColorConsoleWordConditions branch 7 times, most recently from c8ceabf to 45f111a Compare April 19, 2020 07:14
@304NotModified 304NotModified marked this pull request as draft April 19, 2020 08:28
@snakefoot snakefoot marked this pull request as ready for review April 19, 2020 08:30
@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 19, 2020 •
edited
Loading

@304NotModified Was just in a battle with old code-smells, and needed some retries before it worked. Also the Azure-Build-Server sometimes like to timeout, so one needs an extra runs.

@304NotModified
Copy link
Member

Well it's stuck on azure then,as the timeout is 60min and it should be < 20min.

@snakefoot snakefoot force-pushed the ColorConsoleWordConditions branch 2 times, most recently from 526cbd9 to 04e93a5 Compare April 19, 2020 19:50
@snakefoot snakefoot force-pushed the ColorConsoleWordConditions branch from 04e93a5 to 38a6d47 Compare April 19, 2020 19:52
@snakefoot
Copy link
Contributor Author

Sigh. Now my refactoring revealed that we never really tested other control-characters than newline. New unit-test is needed.

@snakefoot snakefoot force-pushed the ColorConsoleWordConditions branch from 38a6d47 to 0546794 Compare April 19, 2020 20:14
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@304NotModified
Copy link
Member

Sigh. Now my refactoring revealed that we never really tested other control-characters than newline. New unit-test is needed.

So convert this PR to draft? Or is this fixed in the force push?

@snakefoot
Copy link
Contributor Author

So convert this PR to draft? Or is this fixed in the force push?

Fixed as code-coverage is back at 100%

@304NotModified
Copy link
Member

ow indeed! NICE!

@304NotModified 304NotModified merged commit 66219a9 into NLog:master Apr 20, 2020
@304NotModified
Copy link
Member

Thanks! merged!

@snakefoot
Copy link
Contributor Author

Updated wiki: https://github.com/NLog/NLog/wiki/ColoredConsole-target

@snakefoot snakefoot added the documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) label Apr 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@304NotModified 304NotModified 304NotModified approved these changes

Assignees
No one assigned
Labels
console-target documentation done all docs done (wiki, api docs, lists on nlog-project.org, xmldocs) feature needs documentation on wiki performance size/L
Projects
None yet
Milestone
4.7.1
Development

Successfully merging this pull request may close these issues.
2 participants
@snakefoot @304NotModified