Skip to content

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

Merged
304NotModified merged 2 commits intoNLog:masterfrom
snakefoot:ColorConsoleWordConditions
Apr 20, 2020
Merged

ColoredConsoleTarget - Allow using Condition-logic for word highlighting rules#3891
304NotModified merged 2 commits intoNLog:masterfrom
snakefoot:ColorConsoleWordConditions

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

snakefoot commented Apr 14, 2020

Before tuning:

image

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

image

@304NotModified
Copy link
Member

304NotModified commented Apr 14, 2020

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

304NotModified commented Apr 19, 2020

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

304NotModified
304NotModified reviewed Apr 19, 2020
View reviewed changes
@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

snakefoot commented Apr 19, 2020

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

sonarqubecloud bot commented Apr 19, 2020

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

304NotModified commented Apr 20, 2020

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

snakefoot commented Apr 20, 2020

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

304NotModified commented Apr 20, 2020

ow indeed! NICE!

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

304NotModified commented Apr 20, 2020

Thanks! merged!

@snakefoot
Copy link
Contributor Author

snakefoot commented Apr 26, 2020

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
@dependabot dependabot bot mentioned this pull request Aug 1, 2025
Open
This was referenced Sep 2, 2025
Open
Closed
@dependabot dependabot bot mentioned this pull request Oct 1, 2025
Open
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