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

gh-131952: Add color to the json CLI #132126

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

Conversation

tomasr8
Copy link
Member

@tomasr8 tomasr8 commented Apr 5, 2025

Example:

image

I'm open to changing the colors. For now it's green for strings, yellow for numbers and cyan for true/false/null.

Copy link
Contributor

@StanFromIreland StanFromIreland left a comment

Choose a reason for hiding this comment

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

How much of a performance impact does this have?

@picnixz
Copy link
Member

picnixz commented Apr 5, 2025

How much of a performance impact does this have?

It doesn't matter here. json.tool is more of a visualization tool. If users don't want to colorize the output they can also disable it using the NO_COLOR env. var IIRC. OTOH, we could add an explicit --pretty flag to enable colorization instead of making the default.

@tomasr8
Copy link
Member Author

tomasr8 commented Apr 5, 2025

OTOH, we could add an explicit --pretty flag to enable colorization instead of making the default.

I think we might not need that, can_colorize can already detect most situations where color is not desirable (e.g. not a TTY). You can also set NO_COLOR to turn off colors unconditionally.

@AA-Turner AA-Turner changed the title gh-131952: Add color to the json.tool CLI. gh-131952: Add color to the json.tool CLI Apr 5, 2025
@picnixz
Copy link
Member

picnixz commented Apr 5, 2025

What about CLIs? they would be broken if data is piped?

@tomasr8
Copy link
Member Author

tomasr8 commented Apr 5, 2025

What about CLIs? they would be broken if data is piped?

AFAIU they wouldn't (and local testing confirms that), see this comment from Hugo: #131952 (comment)

@raztd
Copy link

raztd commented Apr 6, 2025

@tomasr8 thank you for taking the time to look into it. couple of notes regarding the keys 1) could you make them a different color from the string values? 2) could you make them bold?

this is what jq does and i think it provides a good ux https://static1.howtogeekimages.com/wordpress/wp-content/uploads/2020/01/5-7.png

@tomasr8
Copy link
Member Author

tomasr8 commented Apr 6, 2025

@tomasr8 thank you for taking the time to look into it. couple of notes regarding the keys 1) could you make them a different color from the string values? 2) could you make them bold?

this is what jq does and i think it provides a good ux https://static1.howtogeekimages.com/wordpress/wp-content/uploads/2020/01/5-7.png

That should be in theory possible, we can distinguish keys from string literals since keys are always followed by :. I will make a couple different color versions and then we can decide which one looks the best.

@python-cla-bot
Copy link

python-cla-bot bot commented Apr 6, 2025

All commit authors signed the Contributor License Agreement.

CLA signed

Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Can we move the helper functions before the main function? it's easier to read when you're coming from the bottom of the file (and make them private so that someone is not using them by importing json.tool?)

@hugovk hugovk changed the title gh-131952: Add color to the json.tool CLI gh-131952: Add color to the json CLI Apr 6, 2025
Co-authored-by: Adam Turner <[email protected]>
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.

9 participants