gonewest818 / dimmer.el

Interactively highlight which buffer is active by dimming the others.
GNU General Public License v3.0
267 stars 14 forks source link

Documentation about how "dimming" the background actually makes it lighter with dark themes #31

Closed alphapapa closed 4 years ago

alphapapa commented 4 years ago

Hi Neil,

Quick suggestion: I used to use auto-dim-other-buffers to dim the background color, so when I tried using dimmer again, I configured it to dim the background as well. I was immediately surprised by the background of inactive buffers appearing to get lighter rather than dimmer when used with a dark theme. :)

After experimenting and reading the docs about how it works, I see that what it does when affecting the background is bring the background color closer to the color of the default face's foreground color. When used with a dark theme, this has the effect of making the background color brighter rather than dimmer.

So, suggestions:

  1. Somewhere (presumably in the readme, and maybe also in some docstrings), it should be explained that, when used with a dark theme, adjusting the background color will actually make the background lighter.
  2. It might be helpful to mention that an alternative way to think of how it works, regardless of theme or foreground/background setting, is that it reduces contrast in inactive buffers. Depending on the theme and foreground/background setting, that may mean making buffers appear dimmer or brighter.
  3. The dimmer-fraction variable's docstring says that its range is 0-1, but it may actually be set to a negative value as well. In the case where the user is using a dark theme and wants the background color to get darker in inactive buffers, this is necessary. Of course, doing that means that it actually increases contrast between text and the background, which may confound the previous suggestions about documentation. :) But I certainly appreciate the flexibility!

Thanks a lot for your work on this package. I'm actually really impressed at how well it performs, considering that it appears to remap over 1,000 faces when the active buffer changes! (My face-list command returns over 1,000, anyway.)

gonewest818 commented 4 years ago

Hi alphapapa, thanks for the thoughtful comments. It does seem I should clarify the readme and docstrings somehow. FYI the background case is relatively new and I frankly resisted doing it for a while until I found some free time end of last year.

I did at some point consider dimming the background in the opposite direction (silently treating the fraction as negative in the background case). But my inner voice said:

But all points taken. If you have suggested wording for any of these docs please go ahead and propose. Else I’ll keep this ticket open until I get a chance to address. Thanks.

alphapapa commented 4 years ago

Thanks, your new explanation is great!

rationalis commented 4 years ago

I feel like the current behavior is somewhat unintuitive (frac set to 0.4, using solarized dark):

image

Whereas if I force the target background (fake default foreground) to be black:

image

The latter is more expected / desirable to me. Also in general I feel like making whole windows brighter when the theme is dark is sort of uncomfortable. It might be difficult to make the color selection any "smarter" but it would be nice just to expose the target colors as variables.

gonewest818 commented 4 years ago

If you want the background to be darker in the other windows, try using a negative fraction rather than forcing in a color in the code like that. You might get close enough to what you want.

It is definitely possible to expose the target colors, but my concern is that it'll get even harder to explain what's happening. I personally think perhaps a gallery of themes and settings might be helpful as a next step. Case in point, the use of a negative fraction is definitely not intuitive.

rationalis commented 4 years ago

The use of a negative fraction doesn't really help with the issue because it just makes the foreground brighter instead:

image

The behavior I would expect is that both the foreground and background get darker, for a dark theme like this.

gonewest818 commented 4 years ago

I understand, and please just keep in mind the ability to change the background is relatively new and I’m constantly learning how people like to see things with feedback like yours.

In terms of making the documentation clear, I think we’ve made a step forward with the new wording in the readme. I’m working on some “gallery” screenshots with common themes too.

In terms of getting you your desired look, the best workaround I have right now is to use :background mode (not :both) and a small negative fraction. Or, as you have already modified the code, you can always continue with your mod for now.

This discussion warrants a ticket with the request for a new feature. I can imagine either setting the “target face” to be something other than ’default or setting the dimming algorithm to be a choice of “reduce contrast” (what it does now), “darker” (what you want), or “lighter”. Either way you could then get the look you want with some more custom configuration.

I appreciate your understanding on this ... what I would like to do is wrap up the work I’m doing to automate the gallery screenshots before contemplating that new feature. Also have some other projects underway that can’t be sidetracked. So it may be a little while before I can do more here.

rationalis commented 4 years ago

Ah, I don't want to make it sound like I'm demanding things be added, I just wanted to demonstrate the issue for the moment and suggest a quick & dirty fix. If I find the time maybe I'll make a PR. Thanks for the quick responses and this neat extension! :)

gonewest818 commented 4 years ago

See https://gonewest818.github.io/2020/03/dimmer-gallery/

alphapapa commented 4 years ago

See https://gonewest818.github.io/2020/03/dimmer-gallery/

That's really cool! Thanks for putting that together.

Curious, did you code the image-generation in Emacs?

gonewest818 commented 4 years ago

Well, it’s a mix of elisp and python. I couldn’t figure out how to fully automate the loading of themes, changing dimmer settings, and redisplay. I suspect in batch mode the interactive loop doesn’t actually run, or not the way I had hoped. So there’s a makefile that loops over all the cases, and I actually have to use a hot key to take the screenshot. See docs/gallery.el