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.

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

Implement colored output in console logger. #5

Closed
wants to merge 1 commit into from

Conversation

nicokoch
Copy link
Contributor

@nicokoch nicokoch commented Nov 4, 2015

This implementation is based on the prototype provided by @ryantaylor in #3.

Adds a new color syntax to the pattern parser which looks like this:
%c:color(...colored text...)

Also changes the default pattern to %c:highlight(%d %l %t -) %m.

closes #3

This implementation is based on the prototype provided by @ryantaylor in estk#3.

Adds a new color syntax to the pattern parser which looks like this:
%c:color(...colored text...)

Also changes the default pattern to `%c:highlight(%d %l %t -) %m`.

closes estk#3
@nicokoch
Copy link
Contributor Author

nicokoch commented Nov 4, 2015

I chose to use the %c: prefix because it made integrating the parsing a lot easier.

You can run the test crate to take a quick look at suggested default highlighter.

//! When logging to console, color codes can be used to display text in different colors.
//!
//! The basic syntax looks like this:
//! `%c:{color}(...)` , where {color} is one of the following:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like it would make more sense to use {} instead of () to stay in line with %d formats?

@nicokoch
Copy link
Contributor Author

nicokoch commented Nov 5, 2015

Thanks for the suggestions, will take a closer look later.

@sfackler
Copy link
Collaborator

sfackler commented Nov 5, 2015

Oh, and with respect to simplifying parsing, I want to rewrite the parser in terms of nom or LALRPOP instead of hand-rolling it. It seems unfortunate to have the syntax we pick be decided on the level of difficulty involved in updating the parser :)

Maybe I'll tackle that tonight.

@nicokoch
Copy link
Contributor Author

nicokoch commented Nov 5, 2015

Very good idea, i was also thinking about this earlier.

I will clean up the colorization logic but leave the parsing to you then ;)

@sfackler
Copy link
Collaborator

sfackler commented Apr 9, 2016

I'm going to close this out as it's a bit out of date. I'd still very much like to have coloration support, though!

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.

Terminal coloration
2 participants