sharkdp / pastel

A command-line tool to generate, analyze, convert and manipulate colors
Apache License 2.0
5.04k stars 97 forks source link

Test on Windows #45

Closed sharkdp closed 4 years ago

sharkdp commented 5 years ago
sharkdp commented 5 years ago

Due to the fix by @lzybkr, we now have working truecolor support on Windows.

However, the output_vt100 crate only seems to enable truecolor support for STDOUT, not for STDERR: https://github.com/Phundrak/output-vt100-rs/blob/795d3ecf77e8a6bfbd259dfb52c72231452b1b00/src/lib.rs#L30-L33

I guess this is the reason that the color spectrum in pastel pick does not show up in truecolor: image

Apart from that, everything seems to work fine: image

sharkdp commented 5 years ago

ansi_term does the following: https://github.com/ogham/rust-ansi-term/blob/451c1ab78f4e8c146512998ef7b5963fed04540f/src/windows.rs#L25-L27

See also https://github.com/ogham/rust-ansi-term/pull/50 for an explanation.

rivy commented 5 years ago

However, the output_vt100 crate only seems to enable truecolor support for STDOUT, not for STDERR: https://github.com/Phundrak/output-vt100-rs/blob/795d3ecf77e8a6bfbd259dfb52c72231452b1b00/src/lib.rs#L30-L33

As long as STDOUT isn't redirected, the referenced code for output_vt100 should correctly invoke the enhanced display semantics for the console display. The CreateFile(...) method in my PR for ansi-term just fixes the problem for a redirected STDOUT. And I've since created a better version of the same fix, using CreateFileW().

But I don't think that this would be the problem for this issue, unless STDOUT is redirected in some way for the spectrum display. It may be a Windows console bug instead. Or maybe a "pixel" size/density issue?

sharkdp commented 5 years ago

@rivy Thank you very much for the clarification! I was on the wrong way there..

Your comment actually made me look into our code again and it turns out that this was still a problem with pastel. There were actually two places where I checked for the COLORTERM environment variable and only one of them was "patched" in #68 to always yield "truecolor" for Windows.

I just pushed an update that fixes the issue:

image

sharkdp commented 5 years ago

Now we just need an external colorpicker for Windows that we could add here: https://github.com/sharkdp/pastel/blob/4ba074cd38b278b3159aea2b36874840089a145d/src/cli/colorpicker.rs#L64-L97

rivy commented 5 years ago

😄 Ha! You beat me to it. I was just playing around with it and discovered that COLORTERM=truecolor fixed the issue. I was just going to post about it.

sharkdp commented 4 years ago

Going to close this for now. If someone finds a colorpicker for Windows that could be included, let me know. Apart from this, the Windows build should work just fine.

kvnxiao commented 4 years ago

@sharkdp I know this issue is currently closed but would you be releasing pre-build Windows .exe binaries for this, or must we build from src?

sharkdp commented 4 years ago

If someone wants to write the deployment scripts (via travis or appveyor), I'm happy to include them. But I'm probably not going to work on this on my own. (Deployment code for Windows can be found in my other Rust projects, e.g. bat or fd).