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

switch to picocolors for console formatting #628

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

benmccann
Copy link
Contributor

@bcherny
Copy link
Owner

bcherny commented Sep 3, 2024

Thanks for the contribution!

Is there a benefit of this approach for your use case? Presumably, this would add a dependency to your project.

@benmccann
Copy link
Contributor Author

It doesn't require someone to install an undocumented dependency in order to get highlighting. We could keep it as being included at runtime, but it might be nice to mention that you need to install it and this would still be a lightweight package that users are more likely to already have in their dependency tree

@bcherny
Copy link
Owner

bcherny commented Sep 3, 2024

Log highlighting is something you can only enable locally, in development -- I'm not sure it would make much sense in production, unless you have a use case in mind?

See

export function log(style: LogStyle, title: string, ...messages: unknown[]): void {
if (!process.env.VERBOSE) {
return
}

@benmccann
Copy link
Contributor Author

Yeah, I agree it doesn't really make sense in production. I've moved it back to dynamically requiring the library

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.

2 participants