~protesilaos/lin

5 2

Add faces for selected line in inactive window?

Details
Message ID
<m15yma5xjt.fsf@christiantietze.de>
DKIM signature
fail
Download raw message
DKIM signature: fail
Hi,

As a Mac user, I've noticed that with lin-mode enabled in e.g. one of 3 windows, the selection color draws my attention out of habit. Because even when the window doesn't have focus, its selection color is the color of an active selection.

So basically my idea would be this:

For inactive windows, change the lin foreground and background color. So we'd have two sets of colors in the end.

What do you think?

Cheers,
Christian
Details
Message ID
<877d6qgk0a.fsf@protesilaos.com>
In-Reply-To
<m15yma5xjt.fsf@christiantietze.de> (view parent)
DKIM signature
pass
Download raw message
> From: Christian Tietze <me@christiantietze.de>
> Date: Thu, 12 May 2022 22:02:46 +0200
>
> As a Mac user, I've noticed that with lin-mode enabled in e.g. one of
> 3 windows, the selection color draws my attention out of
> habit. Because even when the window doesn't have focus, its selection
> color is the color of an active selection.
>
> So basically my idea would be this:
>
> For inactive windows, change the lin foreground and background
> color. So we'd have two sets of colors in the end.
>
> What do you think?

I think this is an important usability improvement.  Though we should
make a feature request in Emacs so that it is implemented directly in
hl-line.el.  In Emacs, they can provide a new hl-line-inactive-window
face which we will then remap accordingly (if we have to).

Do you think we can formulate a patch and send it together with the
feature request?  Otherwise just asking for it may be enough for someone
to implement it.

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<m135hd6ec3.fsf@christiantietze.de>
In-Reply-To
<877d6qgk0a.fsf@protesilaos.com> (view parent)
DKIM signature
fail
Download raw message
DKIM signature: fail
> I think this is an important usability improvement.  Though we should
> make a feature request in Emacs so that it is implemented directly in
> hl-line.el.  In Emacs, they can provide a new hl-line-inactive-window
> face which we will then remap accordingly (if we have to).

That sounds quite useful. I looked at hl-line.el and found in the documentation at the top:

;; An overlay is used.  In the non-sticky cases, this overlay is
;; active only on the selected window.  A hook is added to
;; `post-command-hook' to activate the overlay and move it to the line
;; about point.

And sure enough, `hl-line-sticky-flag' exists and is `t' by default. So that's why there's a highlight in the non-active window in the first place.

This flag also makes it a bit weird to propose a patch: instead of introducing a new feature, we're now in the position to adapt an existing boolean flag into a configurable face setting.

In programming projects, I'd usually try to do it defensively as an opt-in this way:

1. Introduce a new defcustom setting, e.g.
   `hl-line-inactive-window-face'. And a constant
   `hl-line-inactive-window-default-face'.

2. Set `hl-line-inactive-window-face' to nil (or the default face and
   make it inherit everything, i.e. not change the style). The goal is
   to not affect current looks.

3. To grandfather `hl-line-sticky-flag', add a 'setter' to it, so that
   when its value is changed, we can reactively update
   `hl-line-inactive-window-face'

   - When sticky is `t', set `hl-line-inactive-window-face' to the
     regular hl-line face. This replicates the current behavior of
     stickiness.

   - When sticky is `nil', set the face to nil or the initial
     pass-through face setting, i.e. don't show any visible overlay.

4. Tweak global-hl-line-mode and its stickiness in a similar way. Might
   not work to use the same new face; a
   `global-hl-line-inactive-window-face' could be needed, just like
   there's a local and a global stickiness flag.

Do you think we might get by with doing this less carefully and break things?

It's probably wisest to bring this to the developers' attention first and start the discussion, then coordinate on patches to make this happen. To me it sounds unlikely that I can create a patch easily and propose the change "just so". At least not a patch of quality. Not sure how you feel about the complexity of the problem.

-- Christian
Details
Message ID
<87tu9rmn04.fsf@protesilaos.com>
In-Reply-To
<m135hd6ec3.fsf@christiantietze.de> (view parent)
DKIM signature
pass
Download raw message
> From: Christian Tietze <me@christiantietze.de>
> Date: Fri, 13 May 2022 10:12:28 +0200
>
>> I think this is an important usability improvement.  Though we should
>> make a feature request in Emacs so that it is implemented directly in
>> hl-line.el.  In Emacs, they can provide a new hl-line-inactive-window
>> face which we will then remap accordingly (if we have to).
>
> That sounds quite useful. I looked at hl-line.el and found in the documentation at the top:
>
> ;; An overlay is used.  In the non-sticky cases, this overlay is
> ;; active only on the selected window.  A hook is added to
> ;; `post-command-hook' to activate the overlay and move it to the line
> ;; about point.
>
> And sure enough, `hl-line-sticky-flag' exists and is `t' by
> default. So that's why there's a highlight in the non-active window in
> the first place.

Correct.  I always set it to nil, by the way.

> This flag also makes it a bit weird to propose a patch: instead of
> introducing a new feature, we're now in the position to adapt an
> existing boolean flag into a configurable face setting.
>
> [... 26 lines elided]
>
> Do you think we might get by with doing this less carefully and break things?

All this might mean that we will have a hard time convincing the
maintainers about our patch.

> It's probably wisest to bring this to the developers' attention first
> and start the discussion, then coordinate on patches to make this
> happen. To me it sounds unlikely that I can create a patch easily and
> propose the change "just so". At least not a patch of quality. Not
> sure how you feel about the complexity of the problem.

It is a tricky case.  Because we want to make it performant.  I tried to
come up with a prototype that uses the post-command-hook, but it slows
things down:

    (defvar-local lin--inactive-cookie nil)

    (defun lin--remap-other-windows ()
      (dolist (buffer (buffer-list))
        (with-current-buffer buffer
          (if (and (eq buffer (window-buffer (selected-window)))
                   hl-line-mode)
              (setq lin--cookie
                    (face-remap-add-relative (lin--source-face) lin-face))
            (setq lin--inactive-cookie
                  (face-remap-add-relative (lin--source-face) 'lin-red)))))) ; red for demo purposes

    (add-hook 'post-command-hook #'lin--remap-other-windows)

I have eventually produced the following, which performs considerably
better because it does not map through the entire buffer list:

    (defvar-local lin--inactive-cookie nil)

    (defun lin--apply-remap (window)
      (let ((buffer (window-buffer window)))
        (with-current-buffer buffer
          (when lin-mode
            (if (and (window-live-p window) (eq window (selected-window)))
                (setq lin--cookie (face-remap-add-relative (lin--source-face) lin-face))
              ;; green for the demo
              (setq lin--inactive-cookie (face-remap-add-relative (lin--source-face) 'lin-green)))))))

    (defun lin--remap-windows ()
      (mapc #'lin--apply-remap (window-list)))

    (add-hook 'window-state-change-hook #'lin--remap-windows)

A couple of problems, one about the code and another on its usability:

1. My second snippet fails when the current buffer that has lin-mode
   enabled is displayed in more than one window.  I believe we will fix
   this.

2. How do we handle the case where hl-line-mode is enabled in a buffer
   that is not using lin?  I keep a faint gray background in them and
   use lin's more prominent colour in dired and the like.  But if some
   inactive lin buffers also have gray and my current buffer is gray,
   will that not be confusing?

-- 
Protesilaos Stavrou
https://protesilaos.com
Details
Message ID
<m1r14o3cf7.fsf@christiantietze.de>
In-Reply-To
<87tu9rmn04.fsf@protesilaos.com> (view parent)
DKIM signature
fail
Download raw message
DKIM signature: fail
>> And sure enough, `hl-line-sticky-flag' exists and is `t' by
>> default. So that's why there's a highlight in the non-active window in
>> the first place.
>
> Correct.  I always set it to nil, by the way.

I've set that, too, but the results are not what I expected :)

If I split the window, only _one_ window highlights the line if both
visit the same buffer.

If I'm in e.g. notmuch-tree-mode, the currently selected mail in the
tree view is in the top split, the email content in the bottom split.
The highlighted line in the top split remains. On one hand, that's good,
otherwise I wouldn't see which email I'm looking at. On the other hand
that's exactly the case where I want to change the color to
selected-but-not-focused :)

>> Do you think we might get by with doing this less carefully and break things?
>
> All this might mean that we will have a hard time convincing the
> maintainers about our patch.

So the course of action would be to not change anything "upstream"?

Maybe showing how nice things look in lin once the out-of-focus face
variant is added can be the carrot on a stick for emacs-devel to pick
this up eventually.

>> It's probably wisest to bring this to the developers' attention first
>> and start the discussion, then coordinate on patches to make this
>> happen. To me it sounds unlikely that I can create a patch easily and
>> propose the change "just so". At least not a patch of quality. Not
>> sure how you feel about the complexity of the problem.
>
> I have eventually produced the following, which performs considerably
> better because it does not map through the entire buffer list:
>
>     (defvar-local lin--inactive-cookie nil)
>
>     (defun lin--apply-remap (window)
>       ...)
>     (add-hook 'window-state-change-hook #'lin--remap-windows)

Coming up with a solution was quick! Great job.

auto-dim-other-buffers.el decorates `after-focus-change-function' to do
its thing. That might reduce the amount of events even further?

> 1. My second snippet fails when the current buffer that has lin-mode
>    enabled is displayed in more than one window.  I believe we will fix
>    this.

I would think fixing that must be possible if auto-dim-other-buffers can
do it :)

> 2. How do we handle the case where hl-line-mode is enabled in a buffer
>    that is not using lin?  I keep a faint gray background in them and
>    use lin's more prominent colour in dired and the like.  But if some
>    inactive lin buffers also have gray and my current buffer is gray,
>    will that not be confusing?

The whole window has a faint gray background, you mean? Same here. Am
using `auto-dim-other-buffers-mode' ("adob") for that.

I wonder if we could also hook into that to determine the face color for
lin. There are hooks that could help:

- adob--buffer-list-update-hook
- adob--focus-out-hook
- adob--focus-change-hook

To me, it sounds sensible to make lin respect "adob" settings, and
continue to look the way things look now otherwise.

If you want to make lin react to out-of-focus windows independently from
"adob", we could still copy the approach.

But again, for a prototype, it sounds like hooking into "adob" would be
a good start.

Regarding the colors: This may or may not be helpful to you, but macOS
has this covered :)

Key window: selectedContentBackgroundColor
Non-key window: unemphasizedSelectedContentBackgroundColor

Where "key" means: is in focus and receives key events. There can only
ever be one of these.

Color previews for you: https://mar.codes/apple-colors

In this case, it'd indeed be a very dark gray in dark mode, and a
lighter gray (looks about similar to modus-themes's mode line color)
otherwise.

So I personally would have a somewhat tasteful color palette to pick
from for macOS system look and feel consistency -- but I'm too
color-stupid to come up with a solution for your case if you have
different colors, I'm afraid.

I believe I cannot send PNG images to mailing lists, so here's what
overlaying the colors onto dimmed and in-focus buffers would look like:

https://imgur.com/a/2DmNh6N

I think the contrast is not great, but would good enough. After all, the
point of dimming out-of-focus buffers is to make them stand out less :)

-- Christian
Details
Message ID
<877d6ecop2.fsf@protesilaos.com>
In-Reply-To
<m1r14o3cf7.fsf@christiantietze.de> (view parent)
DKIM signature
pass
Download raw message
> From: Christian Tietze <me@christiantietze.de>
> Date: Fri, 20 May 2022 09:12:28 +0200
>
>>> Do you think we might get by with doing this less carefully and break things?
>>
>> All this might mean that we will have a hard time convincing the
>> maintainers about our patch.
>
> So the course of action would be to not change anything "upstream"?
>
> Maybe showing how nice things look in lin once the out-of-focus face
> variant is added can be the carrot on a stick for emacs-devel to pick
> this up eventually.

A change in emacs.git would be the best course of action, because
everyone can benefit from it.  It also is consistent with how lin is
designed to merely tweak the face of the current line highlight.

>> 2. How do we handle the case where hl-line-mode is enabled in a buffer
>>    that is not using lin?  I keep a faint gray background in them and
>>    use lin's more prominent colour in dired and the like.  But if some
>>    inactive lin buffers also have gray and my current buffer is gray,
>>    will that not be confusing?
>
> The whole window has a faint gray background, you mean? Same here. Am
> using `auto-dim-other-buffers-mode' ("adob") for that.

No, not the whole background.  I mean just the inactive lin face (in the
scenario that does not involve auto-dim-other-buffers-mode): it will be
grey, but grey is also the colour of the active hl-line face in buffers
that do not use lin-mode.

Put differently, we cannot limit this to lin-mode because there are
cases where it will create confusion over which mode controls what.
This favours the approach of patching emacs.git directly to avoid
inconsistencies.  Otherwise we would need to make lin affect all
instances of hl-line-mode, which is not in line with what it is supposed
to do right now.

> I wonder if we could also hook into that to determine the face color for
> lin. There are hooks that could help:
>
> - adob--buffer-list-update-hook
> - adob--focus-out-hook
> - adob--focus-change-hook
>
> To me, it sounds sensible to make lin respect "adob" settings, and
> continue to look the way things look now otherwise.
>
> If you want to make lin react to out-of-focus windows independently from
> "adob", we could still copy the approach.

Yes, I prefer it does not require other packages: this arrangement helps
keep a defined scope and is easier to maintain.  Integration of that
sort is best done either by a third, purspose-specific package or at the
level of user configurations.

> [... 18 lines elided]
>
> So I personally would have a somewhat tasteful color palette to pick
> from for macOS system look and feel consistency -- but I'm too
> color-stupid to come up with a solution for your case if you have
> different colors, I'm afraid.

Picking colours is not a problem at this stage.  The issue I have
identified is what I mentioned above about the potential for confusion
between lin-mode buffers and those that do not use lin but have
hl-line-mode enabled.  The former will have dimming functionality, while
the latter will not, thus creating a mismatch.

-- 
Protesilaos Stavrou
https://protesilaos.com
Reply to thread Export thread (mbox)