-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
Propagate --color option to rustc #2779
Conversation
r? @wycats (rust_highfive has picked a reviewer for you, use r? to override) |
Ah yeah I think this location is fine, other places aren't necessarily invoking the compiler so it probably wants to avoid those directly. Thanks! With a test r=me |
1f75043
to
cb3c5e9
Compare
Added the test. And, OMG, someone has removed the |
Hm, ironically the tests pass in the IDE and not on the CI, because
so no colors in the IDE with |
cb3c5e9
to
5b7a163
Compare
ColorConfig::Never => "never", | ||
}.to_string() | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh actually, this should implement fmt::Display
conventionally instead of ToString
directly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know, thanks!
5b7a163
to
76a48a9
Compare
Propagate --color option to rustc closes #2740 Will try to add a test for this soon (and fix failing tests if any, compiling/running tests locally is slow :( ). I am not sure what is the right place to add `--color` option to the command line. I use [`build_base_args`]. [`process`] also looks like a good candidate, because it is more general, but if we look at the [`CommandType`] we see that only `rustc` command supports `--color`. [`build_base_args`]: https://github.com/matklad/cargo/blob/1f7504397ce7c40ff708e2d31da164822e88ed37/src/cargo/ops/cargo_rustc/mod.rs#L449 [`process`]: https://github.com/matklad/cargo/blob/1f7504397ce7c40ff708e2d31da164822e88ed37/src/cargo/ops/cargo_rustc/mod.rs#L608 [`CommandType`]: https://github.com/matklad/cargo/blob/1f7504397ce7c40ff708e2d31da164822e88ed37/src/cargo/ops/cargo_rustc/engine.rs#L102
☀️ Test successful - cargo-cross-linux, cargo-linux-32, cargo-linux-64, cargo-mac-32, cargo-mac-64, cargo-win-gnu-32, cargo-win-gnu-64, cargo-win-msvc-32, cargo-win-msvc-64 |
closes #2740
Will try to add a test for this soon (and fix failing tests if any, compiling/running tests locally is slow :( ).
I am not sure what is the right place to add
--color
option to the command line. I usebuild_base_args
.process
also looks like a good candidate, because it is more general, but if we look at theCommandType
we see that onlyrustc
command supports--color
.