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

fix: Always use the plain formatter even when the output is not a terminal #83

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

asmeurer
Copy link
Contributor

Fixes #76

I was also looking at whether color support should be modified when the terminal isn't a tty. I think this can be done by enabling supports-colors in owo_colors, although I'm not really a Rust dev and I couldn't quite figure out how to get this working right https://docs.rs/owo-colors/4.1.0/owo_colors/index.html#only-style-on-supported-terminals

@woodruffw
Copy link
Owner

Thanks @asmeurer! This LGTM.

I was also looking at whether color support should be modified when the terminal isn't a tty.

This shouldn't be an issue, since we use anstream::println! to render the output and that will filter colors and other control codes when the output isn't a TTY 🙂

@woodruffw woodruffw added the bugfix Fixes a known bug label Oct 30, 2024
@woodruffw woodruffw changed the title Always use the plain formatter even when the output is not a terminal fix: Always use the plain formatter even when the output is not a terminal Oct 30, 2024
@woodruffw woodruffw merged commit 2c4de7e into woodruffw:main Oct 30, 2024
4 checks passed
@asmeurer
Copy link
Contributor Author

To be clear, I was testing out piping the output to less or to a file and the color codes were not being filtered (even when I tried using if_supports_color like in the owo docs example).

@woodruffw
Copy link
Owner

To be clear, I was testing out piping the output to less or to a file and the color codes were not being filtered (even when I tried using if_supports_color like in the owo docs example).

Huh, that's interesting -- for me locally, they get filtered when I pipe to less. Could you share the exact command you're running?

For context, I tried with both less and less -R and didn't see any colors.

@asmeurer
Copy link
Contributor Author

I ran cargo build --release then ./target/release/zizmor on a repo, both with a pipe to less and redirecting into a file (which I confirmed with a hex editor contains the ANSI codes). I wouldn't be surprised if something went awry in my building somehow, though.

@woodruffw
Copy link
Owner

Weird. I'm at a loss to explain that -- I ran the following:

./target/debug/zizmor ~/personal/devel/gha-hazmat/ > /tmp/zizmor.txt

and opened zizmor.txt, and it's 100% plain text. I also tried with | tee /tmp/zizmor.txt | less interposed and also got plain text.

This might be a host OS thing. Are you on Windows?

@asmeurer
Copy link
Contributor Author

I am on a Mac.

@woodruffw
Copy link
Owner

Okay, me too. I'm stumped then 🙂

@daviddavis
Copy link

daviddavis commented Oct 31, 2024

FWIW, I ran:

cargo build --release
./target/release/zizmor ../gha-hazmat > /tmp/zizmor.txt

And got plain output, no json or colors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a known bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not change the output format when stdout is a pipe
3 participants