From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Eli Zaretskii Newsgroups: gmane.emacs.bugs Subject: bug#67650: [PATCH] ; Hide completion preview when switching windows Date: Wed, 06 Dec 2023 21:02:47 +0200 Message-ID: <83v89b2lyg.fsf@gnu.org> References: <83r0jz4jvd.fsf@gnu.org> <837clr46y9.fsf@gnu.org> Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="39486"; mail-complaints-to="usenet@ciao.gmane.io" Cc: 67650@debbugs.gnu.org To: Eshel Yaron Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Wed Dec 06 20:03:16 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1rAxAy-000A1y-A9 for geb-bug-gnu-emacs@m.gmane-mx.org; Wed, 06 Dec 2023 20:03:16 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1rAxAf-0001wZ-3f; Wed, 06 Dec 2023 14:02:57 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rAxAa-0001vy-Cj for bug-gnu-emacs@gnu.org; Wed, 06 Dec 2023 14:02:53 -0500 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1rAxAY-0005O4-I7 for bug-gnu-emacs@gnu.org; Wed, 06 Dec 2023 14:02:50 -0500 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1rAxAk-00071P-7Q for bug-gnu-emacs@gnu.org; Wed, 06 Dec 2023 14:03:02 -0500 X-Loop: help-debbugs@gnu.org Resent-From: Eli Zaretskii Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Wed, 06 Dec 2023 19:03:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 67650 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 67650-submit@debbugs.gnu.org id=B67650.170188938026983 (code B ref 67650); Wed, 06 Dec 2023 19:03:02 +0000 Original-Received: (at 67650) by debbugs.gnu.org; 6 Dec 2023 19:03:00 +0000 Original-Received: from localhost ([127.0.0.1]:40444 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rAxAi-000719-7b for submit@debbugs.gnu.org; Wed, 06 Dec 2023 14:03:00 -0500 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]:53356) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1rAxAd-00070q-68 for 67650@debbugs.gnu.org; Wed, 06 Dec 2023 14:02:58 -0500 Original-Received: from fencepost.gnu.org ([2001:470:142:3::e]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1rAxAL-0005NS-QD; Wed, 06 Dec 2023 14:02:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=gnu.org; s=fencepost-gnu-org; h=References:Subject:In-Reply-To:To:From:Date: mime-version; bh=G5f1zuwHXhemDyH549Mv2N/PuY3XRXV+8FQaBWIy0Bg=; b=PnA9H1CeyGWT Q/ViBCGqY5Yi8tbtq7k64oHjVQx3wndUIFLdEguu6bZ56JgPED8eTL1kCtUQumqNe5lWLkW/sUrzq X1DE0BJ4L0ffGhXzW7z0Ux9+dOBz1g4liTK+CwHf0GOFUg0M9whrdnmItQbDmsnQshEX/t99j3NzJ +YDejAtzEcSpfU4z7E+yJSa1viDWuMw7Ysboflf1pppo2/lQ4DcUyPiKIzLKSliRPgm8rZcTIkvEW fhG6IAhsGzmT9qMeLOxyuqZnMJaMRnZykHorw0GkI6MvnuSq4gPa/RNqCc2AtodMPhpWtEufs5f0b mhV3ougloyDdPcNVedO0kA==; In-Reply-To: (message from Eshel Yaron on Wed, 06 Dec 2023 19:29:32 +0100) X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:275618 Archived-At: > From: Eshel Yaron > Cc: 67650@debbugs.gnu.org > Date: Wed, 06 Dec 2023 19:29:32 +0100 > > Eli Zaretskii 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.