Skip to content
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

Magit diff colors are now hard to read #334

Open
mrkkrp opened this issue Aug 14, 2019 · 13 comments
Open

Magit diff colors are now hard to read #334

mrkkrp opened this issue Aug 14, 2019 · 13 comments

Comments

@mrkkrp
Copy link

mrkkrp commented Aug 14, 2019

Please restore the old colors, it's a pain to work with the colors after the recent update. @tarsius

sucking-diffs

@mrkkrp
Copy link
Author

mrkkrp commented Aug 14, 2019

This is really sadistic.

@mrkkrp
Copy link
Author

mrkkrp commented Aug 14, 2019

goddamnit

I don't know what was the problem with old colors, but at least they were comfortable to work with.

@tarsius
Copy link
Contributor

tarsius commented Aug 14, 2019

The new (aka original) colors are less pretty than the old (aka transitory) colors. They also have lower contrast, which can be an issue for users with bad sight (but zenburn would be a bad theme for such users anyway). I do not claim otherwise.

However...

This is really sadistic.

That is a really silly claim. It makes my motivation to maybe eventually improve the colors myself go to zero.

I have hinted at how the colors can and should be improved: Make everything darker, increase contrast, use saturation to further separate colors (fine vs. line differences). But do not make two distinct faces look exactly the same. Doing that would be bug. If two things look exactly the same, then it is no longer possible to tell them apart. Therefore the change that I reverted introduced a bug and my revert fixed that very same bug.

Oh, I overlooked this until now:

I don't know what was the problem with old colors, but at least they were comfortable to work with.

I see. Zero effort on your part beyond making an outlandish claim. Well I suggest that you inform yourself and then put in the work to make the colors correct as well as pretty. Until someone puts in that work, the colors are merely correct but not exactly pretty.

@tarsius
Copy link
Contributor

tarsius commented Aug 14, 2019

#317

@mrkkrp
Copy link
Author

mrkkrp commented Aug 14, 2019

That is a really silly claim.

It is indeed! It is not meant to be taken seriously.

But do not make two distinct faces look exactly the same. Doing that would be bug. If two things look exactly the same, then it is no longer possible to tell them apart. Therefore the change that I reverted introduced a bug and my revert fixed that very same bug.

It is all very good, but in the end, what users see after the change that was merged 5 days ago is that things are harder to read. There may be any sorts of explanations for that, aesthetic and philosophical, related to code improvements and refactorings, etc., but the user experience is now worse and nothing is given in return. That's what matters. For example, if two faces look the same, it doesn't make my work with the editor harder as long as the final result is easy to read. I don't care if the function from face name to color is not injective. Of course if everything was arranged in a nice and well-thought way, it would be great. But first things first, I'd like to at least see what I'm committing well.

It makes my motivation to maybe eventually improve the colors myself go to zero.

I have no intention to try to make you work on this as you're probably already quite busy with other Emacs things (and thanks for that). As a matter of fact, if everyone else is OK with these new colors, it's not hard for me to revert this locally for myself in my config.

@mrkkrp
Copy link
Author

mrkkrp commented Aug 14, 2019

@tarsius Sorry for the "sadistic" bit. It's just my sense of humor, not an attack. I should not do this to people I do not know well.

@tarsius
Copy link
Contributor

tarsius commented Aug 14, 2019

Okay I overreacted. Apologies for that.

Of course if everything was arranged in a nice and well-thought way, it would be great. But first things first, I'd like to at least see what I'm committing well.

We disagree on which of the two goals should be prioritized. I can agree to disagree on that.

As the maintainer of Magit I care more about users reporting something as a Magit bug that is actually a bug in the used theme. And I also don't think that the current colors are all that horrible--these colors have been in use by this theme for longer than the colors you want back, so they must have been at least acceptable to other users as well.

I have no intention to try to make you work on this as you're probably already quite busy with other Emacs things (and thanks for that). As a matter of fact, if everyone else is OK with these new colors, it's not hard for me to revert this locally for myself in my config.

That's reasonable and appreciated. Since, like me, you do not want to put in the work required to satisfy both requirements (correct and pretty) either (which is perfectly reasonable) we will have to wait for someone else to volunteer. I suggest we close this issue, so that that person can start from a more neutral ground than this little spat.

@mrkkrp
Copy link
Author

mrkkrp commented Aug 16, 2019

I suggest we close this issue

I think the issue documents... an issue, so it's only reasonable for it to continue existing.

@zflat
Copy link

zflat commented Sep 10, 2019

The colors also make it difficult for me to read the diff. The green highlighting is particularly low in contrast to my eyes.
Here is a more objective assessment of the contrast ratio which fails checks for normal text, bold text, etc: https://webaim.org/resources/contrastchecker/?fcolor=FFFFFF&bcolor=7F9F7F (I believe I am selecting the correct colors from the theme).

@thomasf
Copy link

thomasf commented Nov 6, 2019

I don't know if it helps here but I created an simple color selection algorithm for diff/refine yesterday that works very well for solarized and gruvbox palettes.

The trick was something like this:

  1. figure out if a theme variant is dark on bright or bright on dark by comparing normal fg/bg
  2. find the lightest and darkest shade among the base fg/bg colors used in the theme (it could be beneficial to manually pick a shade).
  3. blend every accent color against the lightest and darkest color by some amount (experiment)
  4. potentially apply some lightness +/- for each color group.

Do all of this in LAB colorspace and keep track of the lightness (L of LAB) differece between fg/bg for all combinations at all times, somewhere below 40% it starts to get hard to read diffs.

I quickly added a zenburn palette theme to my utility to calculate and test how my values holds up. It worked out ok without changing anything (as seen below), tweaking it a bit can probably lead to better results though.

Here is the "report" for zenburn:

image

final lc (difference in lightness) between fg and bg for any given pair is at worst 38% which is ok.

The result looks quite good along the regular zenburn colors:

image

image

colors:

 
    (yellow-1bg  . "#55524c")
    (yellow-1fg  . "#f2e6c3")
    (yellow-2bg  . "#777160")
    (yellow-2fg  . "#ece2c7")
    (orange-1bg  . "#534c48")
    (orange-1fg  . "#e7c4ac")
    (orange-2bg  . "#726054")
    (orange-2fg  . "#e4c7b4")
    (red-1bg     . "#504948")
    (red-1fg     . "#dab0af")
    (red-2bg     . "#6b5656")
    (red-2fg     . "#d9b8b6")
    (magenta-1bg . "#52484f")
    (magenta-1fg . "#e5acd1")
    (magenta-2bg . "#705467")
    (magenta-2fg . "#e2b5d2")
    (violet-1bg  . "#4e4d51")
    (violet-1fg  . "#d0cadd")
    (violet-2bg  . "#66636c")
    (violet-2fg  . "#d1ccda")
    (blue-1bg    . "#495051")
    (blue-1fg    . "#addbdd")
    (blue-2bg    . "#556c6c")
    (blue-2fg    . "#b7dada")
    (cyan-1bg    . "#4a5253")
    (cyan-1fg    . "#b3e7e8")
    (cyan-2bg    . "#577172")
    (cyan-2fg    . "#bbe3e3")
    (green-1bg   . "#464a46")
    (green-1fg   . "#a2b8a1")
    (green-2bg   . "#4f5a4e")
    (green-2fg   . "#acbeab")

I am not familiar with zenburn so its entirely possible that I have based it on the wrong shades.

@thomasf
Copy link

thomasf commented Nov 6, 2019

I decided just to push the whole thing so now there is a solarized-zenburn-theme as well. The more the merrier I guess, solarized-zenburn is more of a remix than a straight up zenburn theme anyways.

bbatsov/solarized-emacs@eace864

@georgek
Copy link
Contributor

georgek commented Feb 15, 2020

I recently updated to the latest version of zenburn and initially thought something was wrong the with the magit diff faces. But I've found that now my eyes have adjusted it looks fine. It also seems to depend heavily on the monitor used. Being able to use magit-diff-refine-hunk now makes things much better.

@mrkkrp Do you still find this a problem or have you found that your eyes have adjusted now like me?

@mrkkrp
Copy link
Author

mrkkrp commented Feb 15, 2020

I simply do not use this color theme anymore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants