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

Test on Windows #45

Closed
2 of 3 tasks
sharkdp opened this issue Jul 17, 2019 · 9 comments
Closed
2 of 3 tasks

Test on Windows #45

sharkdp opened this issue Jul 17, 2019 · 9 comments

Comments

@sharkdp
Copy link
Owner

sharkdp commented Jul 17, 2019

  • do we have to include the code to enable ANSI codes on Windows? This does not seem to be required on Windows 10?
  • Make sure that we don't show the "COLORTERM" warning. PowerShell is not going to set this for us.
  • pastel pick => any supported tools?
@sharkdp sharkdp added this to the Initial release milestone Jul 26, 2019
@sharkdp sharkdp removed this from the Initial release milestone Aug 25, 2019
@sharkdp
Copy link
Owner Author

sharkdp commented Aug 25, 2019

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
Copy link
Owner Author

sharkdp commented Aug 25, 2019

@rivy
Copy link
Contributor

rivy commented Aug 27, 2019

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
Copy link
Owner Author

sharkdp commented Aug 27, 2019

@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
Copy link
Owner Author

sharkdp commented Aug 27, 2019

Now we just need an external colorpicker for Windows that we could add here:

/// Run an external X11 color picker tool (e.g. gpick or xcolor) and get the output as a string.
pub fn run_external_colorpicker() -> Result<String> {
let tools = [
ColorPickerTool {
command: "gpick",
args: vec!["--pick", "--single", "--output"],
version_args: vec!["--version"],
version_output_starts_with: b"Gpick",
},
ColorPickerTool {
command: "xcolor",
args: vec!["--format", "hex"],
version_args: vec!["--version"],
version_output_starts_with: b"xcolor",
},
ColorPickerTool {
command: "grabc",
args: vec!["-hex"],
version_args: vec!["-v"],
version_output_starts_with: b"grabc",
},
ColorPickerTool {
command: "colorpicker",
args: vec!["--one-shot", "--short"],
version_args: vec!["--help"],
version_output_starts_with: b"",
},
ColorPickerTool {
command: "chameleon",
args: vec![],
version_args: vec!["-h"],
version_output_starts_with: b"",
},
];

@rivy
Copy link
Contributor

rivy commented Aug 27, 2019

😄
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
Copy link
Owner Author

sharkdp commented Sep 22, 2019

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.

@sharkdp sharkdp closed this as completed Sep 22, 2019
@kvnxiao
Copy link

kvnxiao commented Sep 26, 2019

@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
Copy link
Owner Author

sharkdp commented Sep 30, 2019

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants