bug#67650: [PATCH] ; Hide completion preview when switching windows

[Eli Zaretskii <eliz@gnu.org>]

> From: Eshel Yaron <me@eshelyaron.com>
> Cc: 67650@debbugs.gnu.org
> Date: Wed, 06 Dec 2023 19:29:32 +0100
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > OK, I understand.  However, it means that now we will not only slow
> > down every command in the buffer that has the completion-preview mode
> > turned on, but we will also slow down every redisplay cycle, even if
> > the buffer was not switched.
> 
> How so?  The docstring of `window-selection-change-functions` says:
> 
>   Functions specified buffer-locally are called for each window showing
>   the corresponding buffer if and only if that window has been selected
>   or deselected since the last redisplay.
> 
> And indeed I see that the function this patch adds to that hook is only
> called in those circumstances.  What performance impact do you envision
> for other redisplay cycles?

Look at the implementation.  Each redisplay cycle we call
run_window_change_functions, which then needs to decide whether to run
any of the window-change related hooks, and for which windows.  IOW,
before we determine whether a window was selected or deselected, we
need to examine them all.  This wastes CPU cycles.

I'm also worried about code that runs from with-selected-window and
similar macros, about code that selects and then deselects the
mini-window, and other similar "temporary changes" of the selected
window.

> > I think we should find a better way of doing this.
> 
> I was also a bit uneasy with extending another hook at first, so I
> appreciate your scrutiny, and I'd love to consider different ways to
> achieve what we want here.  But so far this is the best way I came up
> with, and after testing it and examining the specifics of the case I
> feel that it's not such a bad solution.
> 
> > How about that idle timer idea we discussed earlier?
> 
> I'm not sure I see how that would solve this issue, because we want to
> dismiss the preview as soon as you switch windows, and I imagine that an
> ideal timer would instead be less prompt to react to such a change.
> What do you have in mind?

Whether a timer will be prompt or not is determined by the time
interval programmed into the timer.  A small enough interval is
indistinguishable from "as soon as".

We pop down other features using timers, for example tooltips.  It
doesn't look bad in practice.

Also, window-selection-change-functions cannot perform redisplay
(because they are called in the middle of redisplay), so the effect
will be seen only upon the next redisplay cycle -- not immediately
anyway.

> I also feel that we shouldn't underrate the ability of the current
> approach to display the preview immediately.  In fact, one user said
> that Completion Preview mode "seems more smooth and efficient" then the
> package he was using before, which I attribute to this exact property of
> showing the preview without delay.

Maybe so, but adding too much stuff to these hooks is inelegant, to
say the least, and basically I'm reluctant to admit more and more of
such features.  We already have too many features on these hooks.  And
users rightfully complain that Emacs feels sluggish, except if you
invoke "emacs -Q".

> I share the concern that you're expressing towards excessive use of
> hooks, but I can't currently think of further cases in which we'll
> /need/ any hook but `post-command-hook` for showing/hiding the preview,
> and I think that we're still in the safe zone with this patch.

Well, please try to find alternative implementation ideas, as I'm very
unhappy about going this way.  Again, I suspect this is not the last
hook you'd want to employ for this feature, we are just starting down
that road.

> Perhaps we should wait a few days to see if other suggestions come up?

Fine by me.