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

Pretty indendation and coloring should be separate options #15

Closed
hukka opened this issue Oct 7, 2022 · 8 comments · Fixed by #16
Closed

Pretty indendation and coloring should be separate options #15

hukka opened this issue Oct 7, 2022 · 8 comments · Fixed by #16

Comments

@hukka
Copy link

hukka commented Oct 7, 2022

Currently the only options are to either indent and colorize, or not do either. Colors make it difficult to work with files, or really anything that cannot handle the terminal/ANSI color codes. Would be nice to have command line options to have the pretty indentation without colors.

Or perhaps logic that can see if stdout is attached to a TTY, but looks like that can get pretty hairy in JVM. I guess even more so with graalvm.

@Macroz
Copy link
Contributor

Macroz commented Oct 7, 2022

So maybe don't force puget/cprint here but use ppt/pprint or puget/pprint? https://github.com/markus-wa/cq/blob/main/src/cq/formats.clj#L42

Since this repo is in Hacktoberfest I could be interested in doing a PR on this.

@markus-wa
Copy link
Owner

Thanks for the suggestion!

I think let's start with --color=off --color=auto (default) --color=on

with shorthands -c for on and -C for off

off = puget/pprint
on = puget/cprint
auto = on (later we can add tty detection)

happy for a PR on this @Macroz

@markus-wa
Copy link
Owner

@hukka
Copy link
Author

hukka commented Oct 28, 2022

pretty was released with automatic logic for enabling colors. Seems pretty straightforward: clj-commons/pretty@7cb0c16

@markus-wa
Copy link
Owner

Thanks @hukka - I've created #18 to support this.

PRs welcome as I probably won't get to this for a while

@Macroz
Copy link
Contributor

Macroz commented Oct 29, 2022

So I implemented #18 as described, but I'm not sure this logic (from pretty) works as desired.

The code detects if you are in a TTY alright, but if you don't pipe to a file but to your terminal (e.g., for one-off manual use and exploration), you probably want to see the colors. You are likely also not in a REPL. So this kind of detection would mean the default is uncolored, and you must force the colors with a flag pretty much always.

Any more ideas?

@markus-wa
Copy link
Owner

Thanks @Macroz - I think that's as good as it gets.

As far as I'm aware there is no way to differentiate between pipe to command vs pipe to file (also, commands like tee may write to both a file and to TTY).

It's preferrable to break colouring vs breaking pipe to file.

Would you be able to open a PR? 😄

@Macroz
Copy link
Contributor

Macroz commented Oct 31, 2022

Right!

How about this for a PR #20?

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 a pull request may close this issue.

3 participants