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 syntax highlighting #196

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add syntax highlighting #196

wants to merge 2 commits into from

Conversation

ambiso
Copy link

@ambiso ambiso commented Jul 12, 2022

Colors! Because who doesn't like them?!

Just opening this PR to see if there's demand.
It's a really slow/lazy way to implement syntax highlighting but it works. I can improve the implementation if there's a need.

Summary

JSON syntax highlighting:

terminal output with JSON Token header and Token claims; the JSON values are highlighted now with color

The color turns off if the application is not outputting to a TTY:

image

Preflight checklist

  • Code formatted with rustfmt
  • Relevant tests added
  • Any new documentation added

@mike-engel
Copy link
Owner

Hey @ambiso, thanks for this, and thanks for your patience! I think this looks ok to me, although I wonder if there should be a flag to turn this off. I think it's probably ok to not have one for now unless folks start raising issues about it, but I'd like to get your thoughts.

Finally, looks like there's some cargo conflicts. If you have time, could you merge/rebase?

@ambiso
Copy link
Author

ambiso commented Nov 4, 2022

I rewrote the changes for the latest version; I tried adding a --color=always | never | auto flag, but couldn't find a neat way to do it with clap - it has support for a ColorChoice but it doesn't look like it's easy to support this ColorChoice as a command line flag. https://docs.rs/clap/latest/clap/builder/struct.Command.html#method.color

@mike-engel
Copy link
Owner

Thanks @ambiso! In researching a solution for #197, I realized that bunt via termcolor has support for this that we could emulate (or maybe bat supports NO_COLOR?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants