mina86 / auto-dim-other-buffers.el

Visually makes non-selected windows less prominent
GNU General Public License v3.0
93 stars 13 forks source link

Dim the minibuffer? #33

Open mentalisttraceur opened 1 year ago

mentalisttraceur commented 1 year ago

I sometimes select another window while the minibuffer remains "in use", and it would be nice if the minibuffer itself got dimmed like other windows in those cases.

Compare these two cases:

  1. It is immediately obvious (once you know that black background is active and gray background is dimmed) from just an imprecise glance at either of the windows or the minibuffer if they are active or not:

    Screenshot from 2023-06-01 21-50-48

  2. It is not immediately obvious from an imprecise glance if the minibuffer is active or if the right window is active:

    Screenshot from 2023-06-01 21-50-57

    (Note: this is not due to the (setq auto-dim-other-buffers-dim-on-switch-to-minibuffer nil) line visible in these screenshots - that line was not in effect at the time that I took these screenshots, so auto-dim-other-buffers-dim-on-switch-to-minibuffer was set to t, and in fact it was the right window which was selected, not the minibuffer.)

In fact, in the first picture, we actually can't know from the available visual information if the minibuffer was active - for example, there might be a non-dimmed window in a frame on another monitor.

One example where this comes up: I might be in the middle of writing an M-: line, and then realize I already have some text I want to just reference or copy in a buffer I have open: so I use ace-window (I think plain other-window would work too) to leave the minibuffer, do soomething, and come back.

Another example:

  1. consult-line to find text I I want to copy from elsewhere in the file (or consult-ripgrep if that text is in another file),
  2. "select' the desired match in the minibuffer (this scrolls the main buffer to center that line in that buffer/file) but don't hit RET in the minuffer
  3. switch to the window from the minibuffer (I use ace-window, but I think C-x o/other-window would work) - this leaves the consult function unfinished in the minibuffer, and I can select/edit/copy text in the main buffer which is still "previewing" where the consult search would jump to, and finally 6.switch back to the minibuffer "window", and abort (C-g), which scrolls the main buffer back to where I was when I started the consult search, so I can paste/etc - so obviously this is faster and less manual hassle than if I had to "finish" the consult search, and then navigate back to where I started.

(That second example can be replaced by getting fluent with the mark/jump ring stuff since then I wouldn't need to leave the minibuffer mid-command, but the first one can't, and the point is that this is a very broadly applicable thing - now that I have good keybinds for jumping out of the minibuffer while it's mid-command and then jumping back into it, I find all sorts of situations where that's enough to improve my workflow without me having to learn/configure anything else.)

mentalisttraceur commented 1 year ago

P.S. Thank you very much for making auto-dim-other-buffers!

I just found it this morning (you saw my youtube comments on your video demoing what it looks like), and there's already been one situation where it helped me more quickly recognize that window selection switched in a way I didn't expect.

mina86 commented 1 year ago

Minibuffer is a bit magical in that it never gets dimmed. The assumption being that one doesn’t want echo area to be dimmed. I can take a look into dimming the minibuffer but no promises on ETA.

mentalisttraceur commented 1 year ago

Thanks! At a really quick glance, I think you could just use t for the MINIBUF parameter of the window-list call in adob--rescan-windows and your existing logic seems like it would work fine:

Screenshot from 2023-06-01 23-16-45

Two things happening in that picture:

  1. point and region are on the spot I'm talking about in your code, but also
  2. the minibuffer is dimmed! (for now I just manually ran (let ((w (car (window-list nil t)))) (set-window-parameter w 'adob--dim t) (force-window-update)) to see what it looks like and if anything immediately broke)

So maybe we can just add a customizable auto-dim-other-buffers-dim-minibuffer and then use (if auto-dim-other-buffers-dim-minibuffer t 'n) instead of just 'n in that call?

mentalisttraceur commented 1 year ago

Ah, sorry, after a quick test, my suggestion doesn't actually work. I tried:

-      (dolist (wnd (window-list nil 'n))                                                      
+     (dolist (wnd (window-list nil (if auto-dim-other-buffers-dim-minibuffer                  
+                                        t 'n)))

and then manually evaluated (setq auto-dim-other-buffers-dim-minibuffer t).

I don't really understand why it failed to dim the minibuffer (there are actually moments when the minibuffer is inactive during/after echoes where it does dim/undim, so something is happening...). But hopefully it's more obvious to you what's going on there, since you're already familiar with your code.

mentalisttraceur commented 1 year ago

Ahhh, okay I see you've got some window-minibuffer-p checks in a few places. I've started adjust those and things are starting to work better.

mina86 commented 1 year ago

adob--update also needs to be changed since it has special handling preventing minibuffer from being dimmed.

mentalisttraceur commented 1 year ago

What about adob--focus-out-hook? It also has a window-minibuffer-p check.

Here's a full diff (including focus-out-hook), which gets me behavior that I'm happy with (also, feel free to let me know if adding the line numbers to the diff is helpful or in-the-way!):

  238⋮ 238│                      (get-buffer-window-list buffer 'n 'visible)))
  239⋮ 239│      (set-window-parameter wnd 'adob--dim (not (eq wnd except-in))))))
  240⋮ 240│
+    ⋮ 241│(defun adob--window-minibuffer-p (window)
+    ⋮ 242│  "Like window-minibuffer-p, but lie and say that the window is
+    ⋮ 243│  not on a minibuffer if auto-dim-other-buffers-dim-minibuffer
+    ⋮ 244│  is true, because that variable means to not treat minibuffers
+    ⋮ 245│  specially and dim them like any other windows."
+    ⋮ 246│  (and (not auto-dim-other-buffers-dim-minibuffer)
+    ⋮ 247│       (window-minibuffer-p window)))
+    ⋮ 248│
  241⋮ 249│(defun adob--update ()
  242⋮ 250│  "Make sure that selected window is not dimmed.
  243⋮ 251│Dim previously selected window if selection has changed."
  244⋮ 252│  (when (or auto-dim-other-buffers-dim-on-switch-to-minibuffer
- 245⋮    │            (not (window-minibuffer-p)))
+    ⋮ 253│            (not (adob--window-minibuffer-p)))
  246⋮ 254│    (let* ((wnd (selected-window))
  247⋮ 255│           (buf (window-buffer wnd)))
  248⋮ 256│

  252⋮ 260│        ;; adow-mode is active and window has changed.  Update the adob--dim
  253⋮ 261│        ;; parameter accordingly.
  254⋮ 262│        (when (and (window-live-p adob--last-window)
- 255⋮    │                   (not (window-minibuffer-p adob--last-window)))
+    ⋮ 263│                   (not (adob--window-minibuffer-p adob--last-window)))
  256⋮ 264│          (set-window-parameter adob--last-window 'adob--dim t)
  257⋮ 265│          (force-window-update adob--last-window))
  258⋮ 266│        (setq adob--last-window wnd)
- 259⋮    │        (unless (window-minibuffer-p adob--last-window)
+    ⋮ 267│        (unless (adob--window-minibuffer-p adob--last-window)
  260⋮ 268│          (set-window-parameter adob--last-window 'adob--dim nil)
  261⋮ 269│          (force-window-update adob--last-window)))
  262⋮ 270│       (t

  266⋮ 274│        ;; while at the same time changing buffer in the old window.  In this
  267⋮ 275│        ;; scenario, we’re never dimming the buffer in the old window.
  268⋮ 276│        (and (window-live-p adob--last-window)
- 269⋮    │             (not (window-minibuffer-p adob--last-window))
+    ⋮ 277│             (not (adob--window-minibuffer-p adob--last-window))
  270⋮ 278│             (let ((old-buf (window-buffer adob--last-window)))
  271⋮ 279│               (unless (or (eq old-buf buf)
  272⋮ 280│                           (eq old-buf adob--last-buffer))

  290⋮ 298│  (let* ((selected-window (selected-window))
  291⋮ 299│         (selected-buffer (window-buffer selected-window)))
  292⋮ 300│    (save-current-buffer
- 293⋮    │      (dolist (wnd (window-list nil 'n))
+    ⋮ 301│      (dolist (wnd (window-list nil (if auto-dim-other-buffers-dim-minibuffer
+    ⋮ 302│                                        t 'n)))
  294⋮ 303│        (let ((buf (window-buffer wnd)))
  295⋮ 304│          (cond (adob--adow-mode
  296⋮ 305│                 ;; Update window’s ‘adob--dim’ parameter.  If it changes set

  325⋮ 334│             (buffer-live-p adob--last-buffer))
  326⋮ 335│    (if adob--adow-mode
  327⋮ 336│        (when (and (window-live-p adob--last-window)
- 328⋮    │                   (not (window-minibuffer-p adob--last-window)))
+    ⋮ 337│                   (not (adob--window-minibuffer-p adob--last-window)))
  329⋮ 338│          (set-window-parameter adob--last-window 'adob--dim t)
  330⋮ 339│          (force-window-update adob--last-window))
  331⋮ 340│      (save-current-buffer

Amusingly, with this patch, the echo area is dimmed only while echoing. When it's empty, it's not dimmed. (For me that aesthetic blemish isn't a deal-breaker, and I'd rather have the feature with that flaw than not at all.)

mentalisttraceur commented 1 year ago

Oh sorry also

(defcustom auto-dim-other-buffers-dim-minibuffer nil
  "Whether to dim minibuffers just like regular windows."
  :type 'boolean
  :group 'auto-dim-other-buffers)

Somewhere up top with the other defcustom.

mentalisttraceur commented 1 year ago

If you're basically happy with this I can roll it into a proper PR. If you have any ideas of how to clean up the remaining blemish (protect the minibuffer from dimming just when it's being used as the echo area) that would be cool - I'm going to slightly poke my mind at that problem but unless I can quickly get traction and speed on it I might have to move on.

mentalisttraceur commented 1 year ago

Ah! It seems like *Minibuf-0* is used for echo, and all the other minibuffer-using commands create/use *Minibuf-{1,2,...}*...

So the quickest-and-dirtiest solution is for that adob--window-minibuffer-p function to have a special case for a minibuffer called *Minibuf-0*. I feel a little uncomfortable with that unless we find guarantees that Emacs will never allow another minibuffer to be given the number zero and that the echo minibuffer will never have any other number. But I'll do a quick test down this path to see if it at least works rather than being totally off-base.

mentalisttraceur commented 1 year ago

[edit: this comment was mistaken and is irrelevant/misleading]

mentalisttraceur commented 1 year ago

Pfft, I'm wrong, echo area also uses *Minibuf-1* for echoes [edit: turned out it was *Echo Area {0,1}*]. I guess Minibuf-0 just hangs around doing literally nothing besides looking pretty when nothing else is going on. (So at least one consistency alternative would be to dim Minibuf-0 - that way the echo area at least doesn't un-dim itself in-between echoes.)

mentalisttraceur commented 1 year ago

Promising tidbit, emphasis added:

The echo area is used for displaying error messages (see Errors), for messages made with the message primitive, and for echoing keystrokes. It is not the same as the minibuffer, despite the fact that the minibuffer appears (when active) in the same place on the screen as the echo area.

This is promising because it suggests on some level Emacs recognizes the echo area and the minibuffer as separate concepts, so there's hope that we can figure out how to programmatically detect the difference between normal minibuffer buffers and specifically the "echo area" buffers.

mentalisttraceur commented 1 year ago

Victory!

The magic minibuffer names we need are " *Echo Area {0,1,2,3,...}*".

For some reason my Emacsi all seem to have both Echo Area 0 and Echo Area 1 (and recreate both immediately if I kill those buffers), but just special-casing Echo Area 0 seemed to be enough to keep the echo area from dimming.

mentalisttraceur commented 1 year ago

Okay, so, near as I can tell, there's only *Echo Area 0* and *Echo Area 1* - I guess they're hardcoded? All the code I found floating around either targets just echo area 0 or both 0 and 1, and in 2014, Stefan Monnier (who was head maintainer of Emacs for some time before 2015) assured us that there is never an Echo Area 2 or above.

And it sounds in multiple sources like Emacs alternates between these two buffers as the actual echo area contents?

(What's kinda important and messed up though, is that in manual testing I've noticed it's possible to rename these buffers. So at any time, efforts to detect these buffers by name could be undermined. I guess that's just one of those things we just have to assume for sanity's sake is not our problem - something in the world ought to make sure that the echo area "buffers" are reliably accessible in some way, and the only way we have available is to look for buffers with " Echo Area {0,1}" names - yes it's possible for any package or user to just casually break that "API", but the Right Place(tm) for that problem to be dealt with is not in packages who just need to know if they're looking at one of those buffers or not.)

mentalisttraceur commented 1 year ago

Another thing I figured out is that it is possible to force *Minibuf-0* to take on the dimmed face. (I initially managed to do that with a (with-current-buffer " *Minibuf-0*" (insert "test")) - this of course made my echo area constantly echo "test" when it wasn't echoing anything else, but at least it got the echo area's default inert state to looked dimmed.

Part of why I'm looking at this is that I think there's two "good enough" versions of this feature:

  1. Minibuffers are dimmable, but the "echo area" isn't. (In this case, we want to reliably detect *Echo Area 0* and *Echo Area 1* buffers and not dim them, and we want to leave *Minibuf-0* alone since it's already stubbornly never dimmed.)

  2. Minibuffers are dimmable, and so is the "echo area". (In this case, we don't need to distinguish *Echo Area {0,1}* from other buffers, but we do need to dim *Minibuf-0* for visual consistency/clarity.)

As a user who wants a dimmable minibuffer option, I actually prefer the second, but it sounds like you have some preference for the first, and either one is self-consistent (unlike the diffs I showed so far, which get us most of the way there but have the echo area dim/undim itself, which is only acceptable in the most hacky quick-and-dirty better-than-nothing sense).

Ideally, I would find a way to implement both, and then I image auto-dim-other-buffers-dim-minibuffer would be t for one of them and either any non-nil value or a specific symbol for the other.

If we only had to have one, I would argue for dimming the echo area too, because as a person who wants the minibuffer to be dimmable, I think most people who want the minibuffer to be dimmable specifically want a reliable link between visual dimming and actual "selection"/"input focus" - and the echo area is normally not selected/receiving-input.

Anyway, but so far there is trickiness on both sides: I've not figured out how to get the echo areas to stop being dimmed by turnning on the mode (but deleting them after the mode has turned on causes Emacs to recreate them without the dimming taint, and then I can keep the code from dimming them while the mode is on), and I've only figured out how to get the default blank state minibuf-0 to be dimmed by shoving an insert into it (I haven't yet checked what other operations will trigger the dimming - an invisible-without-programmatic-inspection-of-the-buffer solution is trivial but I'm hoping figure out an entirely buffer-non-mutating one).

mentalisttraceur commented 1 year ago

Ouch, looks like Minibuffers ignore background face when they're totally empty, or something like that. Looks like I'm not the only one who went down this path and had to settle for (insert " ") into *Minibuf-0*.

mina86 commented 1 year ago

If we only had to have one, I would argue for dimming the echo area too

Honestly I have the opposite opinion here. Echo area doesn’t accept input but it is in some sense always active since it displays relevant messages. For me the cue for which window is active is that if there’s a big undimmed rectangle than that’s active; otherwise it’s minibuffer.

One thing to keep in mind is that you can switch to *Echo Area 0* buffer so that it’s displayed in a window. If that’s the case, the window should be dimmed.

Ouch, looks like Minibuffers ignore background face when they're totally empty, or something like that.

Is that a problem? I’m under the impression that if minibuffer is empty it’s not active and echo area is displayed instead.

By the way, when you’re working on it, keep in mind that you can ignore any code-paths where adob--adow-mode is false. Since Emacs 27 it’s true and for simplicity I’d argue it’s acceptable not to implement this feature for older Emacsen.

mentalisttraceur commented 1 year ago

For me the cue for which window is active is that if there’s a big undimmed rectangle than that’s active; otherwise it’s minibuffer.

I do see value in having the echo area visually distinct, especially when there's new information there (in fact, inspired by this discussion, in my own config I will now experiment with two different background colors for the two echo area buffers, so that if something echoes when I don't expect it, there's a bigger cue that the echo area changed).

But I wonder if your mindset/cognition is representative of what's better for users who'd want this feature, since you haven't wanted it enough to implement it. Do you think you'd even use it?

The reason why I want the echo area dimmed is so that merely seeing a selected-colored thing by itself is enough to know that's where I am - that's where my input is, that buffer's keybindings apply, etc. I want that mental path to be as short, simple, and universal as possible, because I know every inference step, need to also look anywhere else, or edge case would slow me down or trip me up.

Is that a problem? I’m under the impression that if minibuffer is empty it’s not active and echo area is displayed instead.

It's actually trickier than that: a lot of the time, the echo area defaults to showing Minibuf-0 - seems to be whenever the last 1-2 commands haven't echoed anything.

You can see for yourself by doing M-: (with-current-buffer " *Minibuf-0* (insert "Hello World!")) and just going on with your day. You'll find yourself seeing "Hello World!" a decent amount of the time in the echo area, unless you're constantly getting other stuff in the echo area (for example, a dense elisp file will always be echoing information about the lisp form you're in - so you'll only see Minibuf-0 contents when point is in the blank space between two defuns or whatever, and even then, weirdly, only after the first point movement after you're already in that blank space) - but in that case, just go visit a plain text file or the scratch buffer for a while.

By the way, when you’re working on it, keep in mind that you can ignore any code-paths where adob--adow-mode is false. Since Emacs 27 it’s true and for simplicity I’d argue it’s acceptable not to implement this feature for older Emacsen.

Cheers, happy to hear that!

mentalisttraceur commented 1 year ago

More technical notes: I'm seeing an inconsistent behavior: whether echo areas get caught along with the minibuffers by the above code changes depends on when the mode is turned on:

If we load the buffers auto-dim-other-buffers-mode in .emacs before init finishes (for example with a non-lazy-loaded (use-package auto-dim-other-buffers :config (auto-dim-other-buffers-mode 1))), or even in after-init-hook, then the echoes don't get dimmed.

If we turn on the mode sometime later (for example with a manual M-x auto-dim-other-buffers), then the initialization ends up causing the echoes to be dimmed (until I delete and recreate the *Echo Area 0* buffer - for some reason just that one?)

mentalisttraceur commented 1 year ago
Results of this aside: > I will now experiment with two different background colors for the two echo area buffers Educational but impractical. [click to expand if you want to read more] Very educational, especially for those of us in this problem space of messing with how the echo area is displayed, but it isn't as useful as I thought because it turns out Emacs does not consistently alternate between these two areas. Turns out, some modes/functions use Echo Area 0 exclusively over and over, while others alternate showing Echo Area 1 every other time. You can see this yourself by evaluating something like this: ```lisp (with-current-buffer (get-buffer-create " *Echo Area 0*") (face-remap-add-relative 'default :background "#B00000")) (with-current-buffer (get-buffer-create " *Echo Area 1*") (face-remap-add-relative 'default :background "#00B000")) ``` Examples: 1. try to move down when already at the bottom of the buffer: the "End of buffer" echo will cycle between areas 0 and 1. 2. in Elisp Mode, the information in the echo area exclusively uses area 0.
mentalisttraceur commented 1 year ago
For anyone else who wants this feature and doesn't want to wait for it maybe making it upstream, here's [the config I am currently using to add minibuffer and echo area dimming](https://github.com/mentalisttraceur/home/blob/d7e8193013fae8ab510980312c5196fb5fc1728d/.emacs#L875-L897). [click to expand if you want to read more] The bulk of it is just functionally the same changes I shared above, but hackily injected with temporary function advice which is added just-in-time and then removed after (`with-advice` is my own macro which is basically just a fancy `unwind-protect` call with `advice-add` in the body and `advice-remove` in the catch-all handler): ```lisp (defun hack-window-list (window-list &optional frame _minibuffer window) (funcall window-list frame t window)) (defun hack-adob--rescan-windows (adob--rescan-windows &rest arguments) (with-advice ('window-list :around 'hack-window-list) (apply adob--rescan-windows arguments))) (advice-add 'adob--rescan-windows :around 'hack-adob--rescan-windows) (defun hack-window-minibuffer-p (&optional _window) nil) (defun hack-adob--update (adob--update &rest arguments) (with-advice ('window-minibuffer-p :override 'hack-window-minibuffer-p) (apply adob--update arguments))) (advice-add 'adob--update :around 'hack-adob--update) (defun hack-adob--focus-out-hook (adob--focus-out-hook &rest arguments) (with-advice ('window-minibuffer-p :override 'hack-window-minibuffer-p) (apply adob--focus-out-hook arguments))) (advice-add 'adob--focus-out-hook :around 'hack-adob--focus-out-hook) ``` Then there are two additional hacks: 1. injecting a newline character into Minibuf-0 (for some reason that buffer does not render custom faces unless there's at least one character in it, and I use a newline instead of a space because there seem to be a couple edge cases where the space gets deleted automatically but the newline isn't... for example, when starting a new frame for a running Emacs server, the "when you're done with this frame, you can close it with `C-x 5 0`" message is actually in Minibuf-0 instead of in the echo area buffers, and I am guessing that message gets cleaned out with logic which deletes a single line), and 2. ensuring that Echo Area 0 and Echo Area 1 are created before turning on the mode (by default the echo area buffers don't get created until later - I am guessing they get made on-demand only once something needs to be echoed): ```lisp (with-current-buffer " *Minibuf-0*" (insert ?\n)) (get-buffer-create " *Echo Area 0*") (get-buffer-create " *Echo Area 1*") ```
mentalisttraceur commented 1 year ago

Okay, so as far as implementing this feature:

  1. The diffs I already came up with a couple days ago are basically almost entirely all we need.

  2. For the echo-area-also-dimmed option, it seems like we just need to

    1. run (get-buffer-create " *Echo Area 0*") and (get-buffer-create " *Echo Area 1*") during mode activation before looping through buffers/windows to dim them, and
    2. run something like (with-current-buffer " *Minibuf-0*" (when (equal (buffer-size) 0) (insert ?\n))) (this is a little icky and might be fragile if anything else ever clears the zeroth minibuffer, but it seems to be the only way to get a custom face to actually render whenever the echo area defaults to showing this buffer - I haven't run into issues though, and if we ever need this to be more robust, we could try escalating by putting that code into a function which is called from something like echo-area-clear-hook).
  3. For the echo-area-not-dimmed option, we just need to

    1. make sure to either avoid buffers named *Echo Area 0* or *Echo Area 1* in the initial dimming loop or delete those buffers afterwards (Emacs will re-create them and they will no longer have the dimming faces), and
    2. (only if we implemented the echo-area dimming option) run something like (with-current-buffer " *Minibuf-0*" (when (and (equal (buffer-size) 1) (equal (buffer-substring 1 2) "\n")) (erase-buffer)) to undo the unfortunate 0th minibuffer hack.
mentalisttraceur commented 1 year ago

Updates!

  1. The tab (\t) should be used instead of the newline (\n) to force consistent minibuffer tinting. Turns out that f.e. when switching between multiple GUI frames, Emacs will actually show a Minibuf-0 with a newline as two lines tall instead of as one tall. Tab doesn't have this problem (while having at least some of the same clearing-resistance that the newline had but the space didn't).

  2. Turns out we need an after-change-functions hook in Minibuf-0 to restore the placeholder character if a mode or other code decides to clear it (for example, I see csharp-mode doing this).

    (with-current-buffer " *Minibuf-0*"
        (insert ?\t))
    (add-hook 'after-change-functions (lambda (&rest _)
        (when (and (equal (buffer-name) " *Minibuf-0*")
                   (< (buffer-size) 1))
            (insert ?\t))))
mina86 commented 1 year ago

But I wonder if your mindset/cognition is representative of what's better for users who'd want this feature, since you haven't wanted it enough to implement it.

It’s based on the belief that echo area needs to be readable when it displays messages and dimmed face by its nature reduces readability (to what extent is up to the user).

Turns out, some modes/functions use Echo Area 0 exclusively over and over, while others alternate showing Echo Area 1 every other time.

src/xdisp.c has a lot of logic regarding which buffer is chosen. In particular with_echo_area_buffer is where the swapping logic is codded.

Turns out we need an after-change-functions hook in Minibuf-0 to restore the placeholder character if a mode or other code decides to clear it (for example, I see csharp-mode doing this).

Yeah, I figured this would be necessary. Ideally we’d figure out why this hack is even needed but the reason might be buried somewhere in C code.

mentalisttraceur commented 1 year ago

[Unrelated to latest comment]

Today I noticed that after-change-functions seems to not be possible to make buffer-local for *Minibuf-0*. I've edited my last post accordingly, and I'm sorry to anyone who had to deal with the same frustrations I did trying to figure out where the auto-indenting was coming from.

In my last post, I had assumed after-change-functions was just inherently always buffer-local automatically, because I couldn't imagine anyone thinking it was sensible design or acceptable implementation wrinkle to do otherwise. By the time I actually observed the problem I thought it was the result of more recent auto-indent settings fiddling and it took me forever to figure it out.

Even after doing an explicit (make-variable-buffer-local 'after-change-functions) in (with-current-buffer " *Minibuf-0*" ...) didn't seem to fix that. I haven't dug into whether this is a global limitation or somehow specific to Minibuf-0, and maybe I'm just doing something wrong (wrong in general or wrong for special/magic buffers like Minibuf-0). So as much as I hate to increase the amount of work done in a global change hook any amount above zero, the only thing I've figured out that works is to let it stay global and check for the buffer name each time.

mina86 commented 1 year ago

after-change-functions is a hook so to add functions to it you should use add-hook. It’s forth argument, LOCAL, if non-nil adds the hook as buffer-local.

mentalisttraceur commented 1 year ago

Cheers, thanks!

(I was+am using add-hook to add functions to it, as you can see in my code snippets. My problem was that I naively assumed I could simply compose the two: make-variable-buffer-local to change scope before the add-hook I was already using, and that it would just check if the given symbol already had a buffer-local binding and operate on that by default if so... and I totally forgot and add-hook's fourth argument, so didn't think to try it. Anyway, thanks again!)

But actually, I've thought of a possible edge-case problem. A buffer-local hook wouldn't survive Minibuf-0 being deleted (and recreated, automatically or otherwise), would it?

mina86 commented 1 year ago

minibuffer-setup-hook is probably what you want.

mentalisttraceur commented 1 year ago

Oh, I forgot: Minibuf-0 is unkillable anyway.

Or at least, when I kill it with kill-buffer or from ibuffer, it just seems to silently be a no-op on that buffer in particular. (This is observably different from the behavior of the echo area buffers, which do get deleted and then get recreated: for example, ibuffer will show "killed 0 buffers" when killing Minibuf-0 but "killed 1 buffers" when killing Echo Area 0.)

So my last concern can probably be ignored: Minibuf-0 might just be immortal.

mentalisttraceur commented 1 year ago

So I tested trying to make if buffer-local with the fourth argument, and with Minibuf-0 it... again has weird wrinkles.

If I just do it during init, for example by having this in my .emacs:

    (with-current-buffer " *Minibuf-0*"
        (insert ?\t)
        (add-hook 'after-change-functions
                  (lambda (&rest _)
                      (when (< (buffer-size) 1)
                          (insert ?\t)))
                  0
                  t))

well during init that just doesn't seem to do anything: that lambda does not appear in the result of (with-current-buffer " *Minibuf-0*" after-change-functions).

If I evaluate that same code later (for example, I paste it into the scratch buffer and evaluate it from there), it seems to work correctly.

So okay, minibuffer-setup-hook to the rescue right? (You'd think just after-init-hook would be enough, but it turns out no, moving the above into after-init-hook doesn't change the result.) Alright, fine Emacs, have it your way, here's a brute-force overkill example:

(add-hook 'minibuffer-setup-hook (lambda ()
    (with-current-buffer " *Minibuf-0*"
        (add-hook 'after-change-functions
                  (lambda (&rest _)
                      (when (< (buffer-size) 1)
                          (insert ?\t)))
                  0
                  t))))

Any time any minibuffer gets set up, we just go ahead and make sure Minibuf-0 has the right after-change hook. Excessive, but if it works we can try to dial it back.

And it... deceptively almost works. It adds the hook in what seems to be the right place. I put that in my init and the result of (with-current-buffer " *Minibuf-0*" after-change-functions) is now a list with that lambda in it, while the global value of that hook remains nil.

And yet! ... That lambda never fires. Despite the hook clearly sitting there, it regresses to the behavior without the hook: once Minibuf-0 has the tab character cleared out, it stays blank. And we can put a (message ...) call at the start of that lambda and it never shows up.

So, somehow, amazingly, changes to Minibuf-0 fire the global after-change-functions but not its buffer-local one.

mina86 commented 1 year ago

Can’t really help you here. You’d have to dig into the code and see when after-change-functions are called. There is some variable which inhibits them so perhaps its set in minibuffer? Or whenever something changes in minibuffer it’s set? Or it could be that whatever is the method by which minibuffer content is changed, it just doesn’t trigger the after-change-functions.

PS. You shouldn’t need with-current-buffer in the last example.

mentalisttraceur commented 1 year ago

Right, I don't expect any help (and I suspect this can only be helped by pushing an upstream C patch). I'm just noting observations which would be relevant to anyone trying to get the behavior of a dimmed minibuffer.

For now I've done as much digging into this as I have the time+motivation to do. Hooking into the global after-change-functions to keep Minibuf-0 dimmed is unfortunate, but so far I experience observable performance degradation from that approximately never and the desire to have the minibuffer dimmed approximately always, so for me the tradeoff is worth it.

I suspect a lessy hacky / more clean option is not possible with current Emacs versions (Minibuf-0 has the can't-be-styled-when-blank and ignores-buffer-local-change-hook quirks) and the current approach (applying a dimming style to all but the active window).

PS. You shouldn’t need with-current-buffer in the last example.

In the minibuffer setup hook? You need it unless you also want a tab potentially getting inserted at every use of the minibuffer, like find-file, M-x, and so on. The minibuffer setup hook runs every time the minibuffer is entered, not just when it's created.

mentalisttraceur commented 1 year ago

I do have one idea, but I think it's probably not worth pursuing: invert the implementation.

When the mode turns on, we could

  1. create an active face with the current default background color as it's background color, and then
  2. set the default background color to the user's preferred dimmed background color, so that we can
  3. apply the active style to the one active window (and optionally the echo area buffers), rather than to all other windows.

This would solve the problem of an empty Minibuf-0 being hacky to dim, but it just shifts the problem of needing to "stuff" Minibuf-0 with a tab to a different case: a minibuffer which isn't being used nor echoing anything becomes hacky to undim.

mina86 commented 1 year ago

Just from architectural point of view, I’d rather not do it that way. Changing the default face feels to be too intrusive for what adob is doing.

mentalisttraceur commented 1 year ago

Yeah I kinda feel the same way. It's a somewhat intrusive hack to work around a quirk that probably ought to be solved inside Emacs' internals.

mentalisttraceur commented 1 year ago

At some point it hit me that the inverted approach doesn't need to touch the default face at all if we also invert the interface: user specifies the active color instead of the inactive color.

From a design perspective, one thing I like about targeting the active window is that it's O(1) at all times instead of sometimes needing to be O(number-of-inactive-things). And semantically it's just a change in perspective/angle; "brightening" the one active thing is the same as "dimming" all inactive things, modulo what gets called default.

But I realize that it doesn't fit your needs so long as Emacs has the Minibuf-0 background color bug. And of course it would be backwards-incompatible for users. So it almost seems better to just release the inverse as auto-brighten-selected-window.el or whatever - and if so then it doesn't even need to be your problem: I or someone else could do that as a friendly fork/re-implementation.

mentalisttraceur commented 1 year ago

One reason why I'm hesitant to do a friendly fork/re-implementation is fragmentation.

For example: I currently have code which makes auto-dim-other-buffers and ace-window look nice together by automatically changing the auto-dim-other-buffers face for the duration of the underlying aw-select call. If I were to release that code as a package, I could reasonably support that automatic enhancement - if auto-dim-other-buffers is installed, also tint that face in addition to the normal background face. I could of course support both (or just pick one that works for me), but we all benefit if there's one community-wide defacto "standard", and right now the closest to that is the "API" of setting auto-dim-other-buffers-face.

mina86 commented 1 year ago

At some point it hit me that the inverted approach doesn't need to touch the default face at all if we also invert the interface: user specifies the active color instead of the inactive color.

The problem is that the interface must look correct even if adob-mode is disabled. That means that we cannot expect user’s default face to be set to inactive colour.

For integration with other packages, the main question is whether there are any hooks that can be added to adob to make such integration possible/easier. I’m not opposed to adding support for other packages but I’d like to see it done in somewhat generic way where adding such package support is a matter of adding a function to a hook or something similar.

mentalisttraceur commented 1 year ago

The problem is that the interface must look correct even if adob-mode is disabled. That means that we cannot expect user’s default face to be set to inactive colour.

Right. Obviously that's true in a package whose whole Concept is "dim other windows". My point is that in a separate "brighten selected window" package, that's just a perspective change:

"The interface must look correct even if absw-mode is disabled. That means we cannot expect the user's default face to be set to the brightened/emphasized color."

mina86 commented 1 year ago

That doesn’t really address the issue. If the default face is one that user prefers to use than emphasized face is worse for user to use in an active window. So to have their preferred face in active windows user would have to set the emphasized face to it and then set default to dimmed face. And now we’re back at the same problem that without the mode enabled user gets diminished user interface.

mentalisttraceur commented 1 year ago

I fully agree that what you're saying is a real issue for users who both

  1. have any desire to ever turn this functionality back off, and
  2. consider their inactive background color a dimished experience.

But for a user like me, neither of those is true. It sounds like for you, they're both true. That's fine. Just like the minibuffer dimming itself - UX needs/wants are very relative to the individual.

I can't speak to your experiences, but here's mine so far:

  1. I've been using adob-mode for months, and I've never wanted to turn it off. To me, active/inactive background color-coding is part of my base functionality. So even if my chosen inactive background color was a diminished experience for me, if I was using the hypothetical emphasize-selected-window-mode, my init would just change the default face background to my inactive color once esw-mode successfully started up - this would cover graceful degradation if the package was missing or there were other issues during init, and that would be the only time I'd need my default background to be the active background.

  2. I don't consider my inactive background color a diminished experience. I use dark gray for inactive, and black for active. The reduced contrast on the dark gray is gentler on the eyes, and for me it's as or more aesthetically pleasing. The black is more reliable in unlikely edge cases: in direct sunlight, or on a screen with an unexpected brightness for a given gray hexcode. If I had to go back to using one background color for both active and inactive windows, I might end up going with the dark gray. (And I think that other users coming at the problem from the "how would I like to change the background of just my active window to indicate activeness?" angle would be more likely to have an inactive background color that's not a diminished experience.)

P.S. friendly reminder that I'm not arguing for you to change your package:

I realize that it doesn't fit your needs so long as Emacs has the Minibuf-0 background color bug. And of course it would be backwards-incompatible for users. So it almost seems better to just release the inverse as auto-brighten-selected-window.el or whatever - and if so then it doesn't even need to be your problem: I or someone else could do that as a friendly fork/re-implementation.

mentalisttraceur commented 1 year ago

Somthing to try one day to make the empty-0th-minibuffer background-changing hack more durable: echo-area-clear-hook.