JuliaLang / julia

The Julia Programming Language
https://julialang.org/
MIT License
45.41k stars 5.45k forks source link

Unreadable stacktrace in solarized theme #38730

Closed Arkoniak closed 3 years ago

Arkoniak commented 3 years ago
julia> versioninfo()
Julia Version 1.6.0-DEV.1537
Commit c36cf8cbc1 (2020-11-20 21:16 UTC)
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-11.0.0 (ORCJIT, skylake)
Environment:
  JULIA_NUM_THREADS = 4

I have a terminal with solarized colorscheme, and in 1.6.0 stacktrace is unreadable.

flameshot-2020-12-06T09-14-16

I've tried to circumvent it with overriding :light_black color in startup.jl

atreplinit() do repl
    try
        Base.text_colors[:light_black] = Base.text_colors[241]
    catch
    end
end

But it helped only partially. Now I can see most of the messages except the number of the line where an error has happened.

flameshot-2020-12-06T09-09-13

KristofferC commented 3 years ago

It seems you have a colorscheme where some of the colors are the same as the background? Isn't the issue with the colorscheme then?

Arkoniak commented 3 years ago

Yes, unfortunately in solarized color8 is the same as background (and the same as :light_black), which is a problem of this colorscheme. But there was no such issue in 1.5. I'll be happy if I can just turn off stacktrace colorizing and get back to 1.5 behaviour. Unfortunately, I wasn't able to find where I can make such adjustments, it looks like colors are hardcoded.

And solarized is rather pretty popular scheme, so I am not going to be the only one who encounter this issue, I am afraid.

Arkoniak commented 3 years ago

So, there are two source of this problem (apart from bad theme itself).

  1. First one is the :light_black which is equal to background. It can be solved on my side with redefinition of :light_black as I did in startup.jl

  2. Second is this: https://github.com/JuliaLang/julia/blob/master/base/errorshow.jl#L746. Here colour code is hardcoded into print statement itself and there is no way to override it except changing the source code or overwriting whole print_stackframe function. So it's going to break any colorscheme which uses background color "\033[90;4m"

KristofferC commented 3 years ago

I guess "\033[90;4m" could be replaced with Base.text_colors[:light_black] * "\033[4m"

Arkoniak commented 3 years ago

Two questions:

  1. Base.text_colors[:light_black] * "\033[4m" works just fine. I can make PR with this patch if it helps.
  2. Are there any hidden problems in adding boolean underline keyword to the with_output_color function? If not, I can try to add it and make another PR without hardcoded underlining.
Arkoniak commented 3 years ago

I've patched locally with_output_color so it now supports underline keyword and result is flameshot-2020-12-06T13-28-27

Arkoniak commented 3 years ago

I guess I am the only one, who was hit by this bug and others are happy with the current situation. No need to keep it open.

mauro3 commented 3 years ago

No, I'm impacted by this too. And others too: https://discourse.julialang.org/t/solarized-dark-theme-with-1-6-0

I find it unfortunate that a very popular color-theme (and other themes too, I use https://github.com/dexpota/kitty-themes/blob/master/themes/3024_Night.conf and in emacs zenburn https://github.com/bbatsov/zenburn-emacs, both sharing this issue) has to be modified to work well with stock Julia. In particular for on-boarding people to Julia.

vtjnash commented 3 years ago

Solarized is a low-contrast color scheme, designed to minimize the visibility of these elements (using hue more heavily than brightness to distinguish them). It is a nice theme, but opinionated. If you don't want a low-contrast, this may not be the right theme.

vtjnash commented 3 years ago

(Note, the goal of zenburn is the same: to have the lowest legible contrast. I don't know about Night.)

mcabbott commented 3 years ago

iTerm2 has a handy "Minimum Contrast" slider for fixing themes, do other terminals have this?

I wondered for a bit whether the logic of making :light_black match the background was that :black was going to be visible, but in fact it seems to be slightly lighter (before being fixed by the slider)

Screenshot 2021-03-30 at 10 50 31

VS Code has a similar setting, although its variant of "Solarized Dark" seems to be fixed already (and has :black darker by default):

Screenshot 2021-03-30 at 11 00 15
Arkoniak commented 3 years ago

Not all terminals have such setting.

xfce-terminal do not have it (http://manpages.ubuntu.com/manpages/xenial/man1/xfce4-terminal.1.html) rxvt do not have it (https://linux.die.net/man/1/urxvt) st do not have it (https://st.suckless.org/)

StefanKarpinski commented 3 years ago

The only solution that's going to work for everyone is to name the color scheme themeable and let people design themes that work for their setups. We could ship a few and make it easy to choose between name themes.

rafaqz commented 3 years ago

Solarized dark is one of the most popular colorschemes.... A lot of people just wont see the types in the stack trace and wont understand why. It probably should work by default.

rafaqz commented 3 years ago

For reference: https://github.com/search?o=desc&q=colorscheme&s=stars&type=Repositories

Solarized seems to be by far the most popular colorscheme in the world. We should probably test major theme changes against themes on this list to avoid issues like this in future.

simeonschaub commented 3 years ago

The issue described above has been fixed by #40288, so it is now straightforward to use a different color for light_black in your startup.jl. That it's broken by default is really just a duplicate of https://github.com/altercation/solarized/issues/220 – skimming through the linked issues there, Julia is by far not the only application affected by this.

Having support for themeable color schemes would probably not be a bad idea, but that should really be a separate issue.

Arkoniak commented 3 years ago

I would disagree, that PR is only half-broken workaround. Half-broken, because Pkg output has the same issues, since it uses the same philosophy of color scheme ignoring. Also it appears again when you are launching julia from shell prompt. Also, that PR has no documentation (because it was never intended to have it), so most users will be stuck, since there are no official ways to use it.

I am sorry, but this is ignoring of the problem, not a solution.

simeonschaub commented 3 years ago

Pkg output has the same issues, since it uses the same philosophy of color scheme ignoring

I tried it with Base.text_colors[:light_black] = Base.text_colors[:red] and it looked to me like it was applied correctly in Pkg output. If it's ignored anywhere though, that should definitely be fixed.

Also, that PR has no documentation (because it was never intended to have it), so most users will be stuck, since there are no official ways to use it.

Documentation for this would definitely be welcome!

Please keep this discussion focused on what we can do to let users customize these colors easier though. There will always be some user setups where we won't be able to guarantee a great Julia experience out of the box. We should try our best to allow users to customize the experience to make it work for their setup, but I don't think always targeting the lowest common denominator of features is a viable option.

adigitoleo commented 3 years ago

Related: https://github.com/JuliaLang/julia/issues/28690