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

Add colors to log module #175

Merged
merged 8 commits into from Feb 11, 2019

Conversation

3 participants
@sh7dm
Copy link
Contributor

sh7dm commented Feb 7, 2019

Add different colors to log messages by levels

@sh7dm

This comment has been minimized.

Copy link
Contributor Author

sh7dm commented Feb 7, 2019

Maybe add emojis too?

@bartlomieju
Copy link
Contributor

bartlomieju left a comment

Cool, could you show a screenshot how it looks?

Show resolved Hide resolved log/handlers.ts
Show resolved Hide resolved log/handlers.ts
@sh7dm

This comment has been minimized.

Copy link
Contributor Author

sh7dm commented Feb 7, 2019

Screenshot:
default

Show resolved Hide resolved log/handlers.ts Outdated
Show resolved Hide resolved log/handlers.ts Outdated
Show resolved Hide resolved log/handlers.ts Outdated
@ry

This comment has been minimized.

Copy link
Contributor

ry commented Feb 7, 2019

We should support https://no-color.org/ in core - I think that's where the logic belongs for switching off color.

@bartlomieju

This comment has been minimized.

Copy link
Contributor

bartlomieju commented Feb 8, 2019

@ry so should colors be always enabled unless there's NO_COLOR env variable? Without option to switch it off without NO_COLOR?

@ry

This comment has been minimized.

Copy link
Contributor

ry commented Feb 8, 2019

@bartlomieju I think we don't want to require people to use --allow-env just to use color... so we should add deno.NO_COLOR (or something) which is set at startup...
Ref denoland/deno#1706

See also https://github.com/denoland/deno/blob/ca397f6793604a2988fce5577b2085abd20d646a/src/ansi.rs#L27-L29

@bartlomieju

This comment has been minimized.

Copy link
Contributor

bartlomieju commented Feb 8, 2019

@ry then that definitely makes sense to control it directly from colors module 👍

@ry ry referenced this pull request Feb 8, 2019

Merged

Adds deno.noColor #1716

@bartlomieju

This comment has been minimized.

Copy link
Contributor

bartlomieju commented Feb 10, 2019

@sh7dm I think you can remove ConsoleHandlerOptions and check regarding colors. It is now controlled by NO_COLOR env variable directly from colors - see #182

@bartlomieju
Copy link
Contributor

bartlomieju left a comment

LGTM

@ry

ry approved these changes Feb 11, 2019

Copy link
Contributor

ry left a comment

LGTM

@ry ry merged commit d895c60 into denoland:master Feb 11, 2019

2 checks passed

denoland.deno_std #20190210.7 succeeded
Details
license/cla Contributor License Agreement is signed.
Details

@sh7dm sh7dm deleted the sh7dm:log-colors branch Feb 11, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.