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

[Eshel Yaron]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Eshel Yaron <me@eshelyaron.com>
>>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>> > ...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 have, and ISTM that the buffer-local value of
`window-selection-change-functions` is only ever consulted in
`run_window_change_functions_1`, which calls this functions in turn.
That means that whenever redisplay happened without invoking our hook
function, our hook function did not incur any performance penalty.  Is
there some fast path for when this hook is nil that I'm missing?

> 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.

You'll find none of those here.  My patch only says
`with-current-buffer`, it do any temporary window selections.

>> > 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.

Well, I don't see any delay.

>> 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.

Here's one.  I'm attaching an updated patch (v2) that does the same
thing, except now we only extend `window-selection-change-functions`
when the preview is shown, and we remove our function from the hook
immediately when hiding the preview.  So you can have ten windows
showing buffers with Completion Preview mode and only the selected
buffer will have any `window-selection-change-functions`, and only when
it's showing the preview.

> Again, I suspect this is not the last hook you'd want to employ for
> this feature, we are just starting down that road.

Noted.

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

In the meantime, here's the updated patch:

v2-0001-Hide-completion-preview-when-switching-windows.patch
Description: Text Data