-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Stop using bold text for input and answers #11250
Stop using bold text for input and answers #11250
Conversation
Normal printed output is non-bold. I find that the difference helps make it clearer what's going on. We already make colored responses opt-in. At this rate, we're headed towards the git situation where 99% of users don't know how to make it look colorful or nice. |
+1. I've had this locally for a while. I agree that bold should only be used for emphasizing pieces of text. |
This would also resolve #8096 (colors would show up correctly for people using the Solarized colorscheme). Note that (as I mention in that issue) I think that this is basically Solarized's fault for changing the meaning of bold text, but it is a relatively popular theme, and it seems weird to use bold unnecessarily. @StefanKarpinski isn't the color by itself enough to differentiate from normal printed text? Maybe colored text could print in non-bold color and if color is turned off all colored text just gets bolded instead? |
I just got annoyed by the bold fonts again. It really makes the text harder to read and I agree with @jayschwa that it is wrong to use bold here so I'm in favor of merging this. |
9850d29
to
c5105e7
Compare
Rebased from master and resolved merge conflict. |
c5105e7
to
6827085
Compare
@jayschwa Thanks. I'll merge tomorrow unless someone objects. |
@andreasnoack I didn't sense a consensus on this one, yet. |
Being able to disambiguate at the repl:
seems useful. I don't care about |
I agree that it should be possible to disambiguate |
Is it possible to enable bold (or a color) on windows, or is this a linux only feature? I don't have anything bolded. |
@mmoh, Julia on Windows uses non-bold text by default. Bold doesn't appear to do much in a Windows terminal. It brightens the color a bit, but doesn't change the visual weight of the characters. Here's how to play with it: print_with_color(:bold, "This is bold") |
I think it should work in mintty though |
@jayschwa Do you have a proposal for distinguishing the return from a function from what is written to STDOUT? |
Is |
I think we should set a default value. |
Using a prefix seems more in line with what other language REPLs do, but I feel like that falls outside the scope of this PR. I personally can't recall many times where differentiating between a return value and stdout was problematic for me. |
I have return values as |
Let's try with default output in magenta. Then I think we have a compromise that will eliminate the bold letters and still disambiguate printed output from return values. @jayschwa, if you update then I'll merge. |
NO!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! |
I concur, I think magenta will be more offensive to my eyes than bold 🙈 I'm skeptical that there is widespread confusion between prints and REPL output. |
@StefanKarpinski Color seems to be a sensitive issue. Should we try without and see if it causes confusion that return values in the REPL and printed output are identical? I like the magenta output idea, but my main priority is to get rid of the bold letters. |
Apparently the worst bikeshed is the original: what color to paint the bike shed. |
I worry that this now defaults to giving people the least information and the least "fancy" setup. This is kind of like how git defaults to not using colors. As a result, I've seen people who've used git for years with no colors. I turn the option on for them and suddenly the whole experience is changed. I don't want Julia to end up like this. Even if you don't like the default output, at least the use of colors and bold lets you know that colors and bold are an option and something that you can configure. |
Why does bold == fancy? It seems like a subjective opinion. I don't like that you take away bold as an option to emphasize particular output. So it seems like you can add more information to output when bold by default is turned off. |
If it's easy and well documented to turn off the bold, what's the problem? I'm not sure magenta is a good choice either. |
The problem is outlined in the original post. Matching the terminal's font shouldn't require explicit action, it should happen by default. |
d054781
to
8c490d6
Compare
I have rebased the PR off master and replaced some hardcoded ANSI codes with @KristofferC, I did not remove the bold from [Updated] After playing, I see that |
16d346e
to
20d7a86
Compare
👍 |
@@ -958,7 +956,7 @@ type StreamREPL <: AbstractREPL | |||
waserror::Bool | |||
StreamREPL(stream,pc,ic,ac) = new(stream,pc,ic,ac,false) | |||
end | |||
StreamREPL(stream::IO) = StreamREPL(stream, julia_green, Base.input_color(), Base.answer_color()) | |||
StreamREPL(stream::IO) = StreamREPL(stream, Base.text_color[:green], Base.input_color(), Base.answer_color()) |
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.
This should probably be light_green
if we want to keep the same color as before.
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.
it should also be text_colors
, which tells me this line isn't hit by tests at all
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.
When is this even used?
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.
remote repl maybe? untested stuff that probably only Keno uses?
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.
Re-pushed with the typo fixed. :green
was left as-is because that matches what it was before. The only difference is that it no longer contains the code for bold, but it was redundant since bold is prefixed later on by Prompt
. The effect remains the same: bold, green julia>
.
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.
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.
The bold is still being applied, just somewhere else, so the color isn't actually changing (nor the bold). I see what you mean though, so I will updated it. It probably makes sense to do the same in the banner too, right?
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, if the boldness is still being applied then it doesn't really matter. I remember now that I had to add that here: https://github.com/JuliaLang/julia/pull/18628/files#diff-8dc8200a114faecb709b7b96e96fc0cfR632. Maybe change to light_green
and remove that bold part so the prompt is not in bold?
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.
I'm going to leave the prompts bold. Changing the color to light_green
still might make sense because it decouples the color from its boldness. However, do the light variants work reliably across platforms?
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.
Everywhere I have tested, yes. Errors are currently being printed in light_red
for example.
This needs a NEWS entry preferably with a link to the manual how to change (and get back the bold) for the input / ouput color. |
20d7a86
to
79d03c4
Compare
Pushed an update that contains a NEWS entry for the style change. |
79d03c4
to
c5d8e55
Compare
Some hardcoded ANSI codes were also replaced with `text_colors`.
c5d8e55
to
e5ed283
Compare
@KristofferC, I just updated the prompt and banner colors to use light variants. In the future, if bold is turned off on any of these things, their color will be unaffected. Dots in the banner are still bold, but I did unbold the ASCII art "julia" part since I think it looks better. Are there any Windows builds that are automatically generated? I'm interested to see how light variants work there (but don't let that hold up the PR from being merged). |
Can't you just print some hard coded ANSI to see how it looks on windows? With everything else being not bold I don't see a reason for having the prompt bold but I don't mind that much. |
Non-bold green is pretty dark in some terminals. |
Yes but see discussion at #11250 (comment) |
I just played with a nightly on Windows and convinced myself that light_[color] is working and equivalent to bold [color]. I think it's good to go.
This PR has been lingering for a year and a half. I'd rather not increase the scope 😄. |
Ok, I'll make that PR then ;) |
What is the status of this? It seems to have stalled out again, although the only conflict is NEWS.md |
I resolved the conflict in NEWS, which was caused by all of the entries being wrapped to a different line length on master. I suggest using "Display the rich diff" when reviewing that file. |
For example, one way of doing this is adding `ENV["JULIA_INPUT_COLOR"] = :bold` and `ENV["JULIA_ANSWER_COLOR"] = :bold` to the `.juliarc.jl` file. | ||
See the [manual section on customizing colors](http://docs.julialang.org/en/latest/manual/interacting-with-julia/#customizing-colors) for more information. | ||
|
||
* The default color for info messages has been changed from blue to cyan, and for warning messages from red to yellow. |
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.
this should probably have its issue/pr reference added, it got lost at some point
It looks like all the underscore E.g. on 0.6 Is it possible to remove the |
Yes, that should work fine. |
thanks @KristofferC mind submitting a PR or should I? |
Likely to happen faster if you do it. |
This pull request turns off bold text everywhere and changes it to "normal" text.
Julia should respect the user's font settings; if they really wanted to see bold text everywhere, they presumably would have adjusted it in their terminal's settings. I don't think there's a compelling reason for Julia not to toe the line here. For the existing Julia users who like the good old days, bold text can still be enabled via the
JULIA_[INPUT|ANSWER]_COLOR=bold
environment variables.Bold styling should be used conservatively to highlight information. Switching the baseline to normal text opens the door for more intelligent use of bold in the future. 🚀