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

Let windows users have the full color experience #427

Closed
jcwillox opened this issue Sep 1, 2021 · 4 comments · Fixed by #441
Closed

Let windows users have the full color experience #427

jcwillox opened this issue Sep 1, 2021 · 4 comments · Fixed by #441

Comments

@jcwillox
Copy link

jcwillox commented Sep 1, 2021

I don't believe anyone's asked this before but I can't see a good reason to downgrade windows users from full to color especially when they explicitly specify it.

hyperfine/src/main.rs

Lines 219 to 222 in 1027940

// We default Windows to NoColor if full had been specified.
if cfg!(windows) && options.output_style == OutputStyleOption::Full {
options.output_style = OutputStyleOption::NoColor;
}

In fact, you may consider making windows default to full as Windows Terminal has support for true colour and recently added bold support in the previews. As well the default windows conhost now has support for ANSI colours, although you will need to enable support in your program or enable it globally via the registry. That last bit isn't as important as I'd assume most people wouldn't be using hyperfine with conhost.

https://stackoverflow.com/a/51681675/12346307 enabling ANSI support for conhost
https://github.com/mattn/go-colorable a library I've used before that handles enabling ANSI support, not sure if rust has an equivalent.

@sharkdp
Copy link
Owner

sharkdp commented Sep 3, 2021

I agree, let's change this. Related (old) discussion: #37

@sharkdp
Copy link
Owner

sharkdp commented Oct 17, 2021

@jcwillox Could you let me know if this works in https://github.com/sharkdp/hyperfine/releases/tag/v1.12.0 ?

@jcwillox
Copy link
Author

Yessss, fantastic colours are working correctly, thanks!

Side note the latest update broke the scoop manifest, as the binary is now inside a directory, I've already published a PR to fix this though ScoopInstaller/Main#2773.

@sharkdp
Copy link
Owner

sharkdp commented Oct 18, 2021

Side note the latest update broke the scoop manifest, as the binary is now inside a directory, I've already published a PR to fix this though ScoopInstaller/Main#2773.

Thank you!

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.

2 participants