use bold magenta instead of bold white for highlighting #118756
Conversation
rustbot
added
A-testsuite
compiler/rustc_errors/src/emitter.rs
Outdated
| } | ||
| Style::HeaderMsg | Style::NoStyle => {} | ||
| Style::Level(lvl) => { | ||
| spec = lvl.color(); | ||
| spec.set_bold(true); | ||
| } | ||
| Style::Highlight => { | ||
| spec.set_bold(true); | ||
| spec.set_fg(Some(Color::Magenta)); |
There was a problem hiding this comment.
Could you keep the bold? I think bold + a color is easier to read than just a color.
There was a problem hiding this comment.
i am not a giant fan of how the bold looks, it feels too strong
There was a problem hiding this comment.
That is significantly easier to read in my opinion
There was a problem hiding this comment.
Subjectively, it feels easier to read to me than the darker version. What do these look like in Windows?
There was a problem hiding this comment.
Specifically, the magenta in the "after" picture you linked really weakens the contrast against a black background, especially given the thin font face/lack-of-bolding.
There was a problem hiding this comment.
ok, done.
i will check the colors on windows in a sec, give me ~15 minutes
There was a problem hiding this comment.
in vscode:
in powershell:
in cmd.exe:
There was a problem hiding this comment.
there is also some horrible thing called "windows powershell ISE", but rustc correctly determines that it doesn't support ANSI escape codes and just prints everything in the default red
according to a poll of gay people in my phone, purple is the most popular color to use for highlighting | color | percentage | | ---------- | ---------- | | bold white | 6% | | blue | 14% | | cyan | 26% | | purple | 37% | | magenta | 17% | unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason. until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it. instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty.
|
Ideally we'd support themes I guess, so that people could choose the colors/boldness/etc they see fit. Although that sounds somewhat annoying since rustc probably shouldn't try to find configs (do we use env for that? cargo support?). Either way, purple pretty, so LGTM. (also I agree with @compiler-errors that without bolding the colors are too dark, at least in vscode) |
@estebank has suggested a per-user |
|
Please keep it bold if the color is changing |
jyn514
changed the title
|
Could not assign reviewer from: |
|
@bors r+ |
bors
added
S-waiting-on-bors
Yeah, this is also what I was talking/thinking about, but it feels wrong-ish. Mainly because "find all the possible places config can be" seems to be outside of compiler responsibilities. But it also feels nice. And maybe niceness is more meaningful than purity here. |
use bold magenta instead of bold white for highlighting according to a poll of gay people in my phone, purple is the most popular color to use for highlighting | color | percentage | | ---------- | ---------- | | bold white | 6% | | blue | 14% | | cyan | 26% | | purple | 37% | | magenta | 17% | unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason. until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it. instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty. before:  after:  other colors for comparison: blue:  cyan:  purple:  magenta without bolding:  r? `@estebank`
use bold magenta instead of bold white for highlighting according to a poll of gay people in my phone, purple is the most popular color to use for highlighting | color | percentage | | ---------- | ---------- | | bold white | 6% | | blue | 14% | | cyan | 26% | | purple | 37% | | magenta | 17% | unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason. until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it. instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty. before:  after:  other colors for comparison: blue:  cyan:  purple:  magenta without bolding:  r? ``@estebank``
…kingjubilee Rollup of 8 pull requests Successful merges: - rust-lang#118198 (coverage: Use `SpanMarker` to improve coverage spans for `if !` expressions) - rust-lang#118512 (Add tests related to normalization in implied bounds) - rust-lang#118610 (update target feature following LLVM API change) - rust-lang#118666 (coverage: Simplify the heuristic for ignoring `async fn` return spans) - rust-lang#118737 (Extend tidy alphabetical checking to `tests/`.) - rust-lang#118756 (use bold magenta instead of bold white for highlighting) - rust-lang#118762 (Some more minor `async gen`-related nits) - rust-lang#118764 (Make async generators fused by default) r? `@ghost` `@rustbot` modify labels: rollup
|
Failed in #118768 (comment) While it's true that purple is pretty @bors r- |
bors
added
S-waiting-on-author
this is kinda jank because it means people need both machines to bless the tests
|
i pushed a change that adds separate revisions for the test, since we use different colors on unix and windows. it has the downside that to bless it, you need to have both a windows and linux machine. i can just mark the test as ignore-windows, but then we won't have any tests for the colors we output on windows :/ @rustbot ready |
rustbot
added
S-waiting-on-review
|
@bors r+ |
bors
added
S-waiting-on-bors
use bold magenta instead of bold white for highlighting according to a poll of gay people in my phone, purple is the most popular color to use for highlighting | color | percentage | | ---------- | ---------- | | bold white | 6% | | blue | 14% | | cyan | 26% | | purple | 37% | | magenta | 17% | unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason. until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it. instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty. before:  after:  other colors for comparison: blue:  cyan:  purple:  magenta without bolding:  r? `@estebank`
…mpiler-errors Rollup of 7 pull requests Successful merges: - rust-lang#118445 (Let `reuse` look inside git submodules) - rust-lang#118534 (codegen: panic when trying to compute size/align of extern type) - rust-lang#118756 (use bold magenta instead of bold white for highlighting) - rust-lang#118797 (End locals' live range before suspending coroutine) - rust-lang#118840 (remove some redundant clones) - rust-lang#118844 (Monomorphize args while building Instance body in StableMIR) - rust-lang#118848 (Add myself back to review rotation) r? `@ghost` `@rustbot` modify labels: rollup
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#118445 (Let `reuse` look inside git submodules) - rust-lang#118756 (use bold magenta instead of bold white for highlighting) - rust-lang#118797 (End locals' live range before suspending coroutine) - rust-lang#118840 (remove some redundant clones) - rust-lang#118844 (Monomorphize args while building Instance body in StableMIR) - rust-lang#118846 (Fix BinOp `ty()` assertion and `fn_sig()` for closures) - rust-lang#118848 (Add myself back to review rotation) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#118756 - jyn514:colors, r=estebank use bold magenta instead of bold white for highlighting according to a poll of gay people in my phone, purple is the most popular color to use for highlighting | color | percentage | | ---------- | ---------- | | bold white | 6% | | blue | 14% | | cyan | 26% | | purple | 37% | | magenta | 17% | unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason. until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it. instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty. before:  after:  other colors for comparison: blue:  cyan:  purple:  magenta without bolding:  r? ``@estebank``
@whentze mentioned this about terminal colors:
"small amount of colors, user-themeable" is exactly what we want, and we don't use specific colors anywhere else in rustc, so i have changed my mind and don't think we should add detection for 256-color terms. the color difference between purple and magenta is usually small in any case. |
…ilstrieb use `if cfg!` instead of `#[cfg]` this pr is specifically for waffle because i love it <3 fixes rust-lang#118756 (comment) r? `@WaffleLapkin`
Rollup merge of rust-lang#118993 - jyn514:cfg-color, r=WaffleLapkin,Nilstrieb use `if cfg!` instead of `#[cfg]` this pr is specifically for waffle because i love it <3 fixes rust-lang#118756 (comment) r? `@WaffleLapkin`
according to a poll of gay people in my phone, purple is the most popular color to use for highlighting
unfortunately, purple is not supported by 16-color terminals, which rustc apparently wants to support for some reason.
until we require support for full 256-color terms (e.g. by doing the same feature detection as we currently do for urls), we can't use it.
instead, i have collapsed the purple votes into magenta on the theory that they're close, and also because magenta is pretty.
before:
after:
other colors for comparison:
blue:
cyan:
purple:
magenta without bolding:
r? @estebank