Join GitHub today
GitHub is home to over 31 million developers working together to host and review code, manage projects, and build software together.
Sign upAutodetect TTY and add colors #394
+46
−4
Conversation
peschkaj
force-pushed the
peschkaj:378-rustc-colors
branch
from
ffb1f96
to
740b51e
May 2, 2016
peschkaj
referenced this pull request
May 4, 2016
Closed
Enabling telemetry disables rustc colors #378
brson
reviewed
May 4, 2016
src/rustup/command.rs
Outdated
| let e = e.as_ref().to_str().unwrap_or(""); | ||
| e == "--color" | ||
| }) | ||
| { |
This comment has been minimized.
This comment has been minimized.
brson
May 4, 2016
Contributor
Please pull out this expression from (&args).iter() onward into a local variable with a descriptive name like, has_color_arg.
I believe this check should be e.starts_with("--color") since it could see e.g. --color=always.
brson
reviewed
May 4, 2016
| let mut out = 0; | ||
| GetConsoleMode(handle, &mut out) != 0 | ||
| } | ||
| } |
This comment has been minimized.
This comment has been minimized.
brson
May 4, 2016
Contributor
This function already exists in rustup_cli::tty. Can you extract that module to rustup_utils::tty so it can be reused here?
This comment has been minimized.
This comment has been minimized.
|
Looks good. Thank you. |
This comment has been minimized.
This comment has been minimized.
|
Thanks for the eyeballs! Fixed those issues, test pass locally, now to wait for appveyor and travis. |
This comment has been minimized.
This comment has been minimized.
|
lgtm but the test failure is legit |
peschkaj
force-pushed the
peschkaj:378-rustc-colors
branch
from
c2663a0
to
4e616ef
May 5, 2016
This comment has been minimized.
This comment has been minimized.
|
Should be fixed in the current commit. This is the reverse of the usual pain - I frequently fail to test on Linux. |
brson
merged commit 2da4bef
into
rust-lang:master
May 6, 2016
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
peschkaj commentedMay 2, 2016
If *nix users are running on a tty (using the same checks as rustc & cargo), then colors will be displayed.
Windows users are still out in the cold. This could potentially be fixed by changing rustc to emit ANSI color codes on Windows 10 and newer - but that's a rustc thing, not a rustup thing.