From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Spencer Baugh via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#74019: [PATCH] Optionally preserve selected candidate across *Completions* update Date: Mon, 28 Oct 2024 09:51:40 -0400 Message-ID: References: Reply-To: Spencer Baugh Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="37325"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 74019@debbugs.gnu.org, Juri Linkov To: Stefan Monnier Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Oct 28 14:52:46 2024 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 1t5QAn-0009Ux-2z for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 28 Oct 2024 14:52:45 +0100 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1t5QAb-0000Ww-5f; Mon, 28 Oct 2024 09:52:33 -0400 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 1t5QAW-0000Wn-73 for bug-gnu-emacs@gnu.org; Mon, 28 Oct 2024 09:52:28 -0400 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 1t5QAU-0006dz-Hj for bug-gnu-emacs@gnu.org; Mon, 28 Oct 2024 09:52:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=debbugs.gnu.org; s=debbugs-gnu-org; h=MIME-Version:Date:References:In-Reply-To:From:To:Subject; bh=oB1fh6eQ+JvzeJyM9zWrYtXns80pEJboXFD1RtYnrGs=; b=cXlLyIAhzV8v2UD3POLT5STxW6dcc+YC2FAEneCDa1CMIe3KnZ6yajcI6MeD2whwY0XHCGI/f8kEt9hqVqTwm52v9W1zFeWNTwzPfOkmoNetVaGDnpVZLmV8QNl8niCik5T7yNzuQ0aH7oTCCLADLWR34g93LOMlqLdkDGQOpT9npGLwso4/uPgoFKFwbwrGZ6D+exus61glc6udpjFPAIqSLhqHk6JbBlnimqiVznpA/l9MDtDOE2SCdrnYYiylpN+/GgIHKq+DFXvba5k8Qm+N9PbuA/Dbdc10cL1H/bUuKqXR7XnJ3nhsVnePoMght9KRcOGZAfg4PWn9V2RtsQ==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1t5QB4-0001Na-7h for bug-gnu-emacs@gnu.org; Mon, 28 Oct 2024 09:53:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Spencer Baugh Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Mon, 28 Oct 2024 13:53:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 74019 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: patch Original-Received: via spool by 74019-submit@debbugs.gnu.org id=B74019.17301235445240 (code B ref 74019); Mon, 28 Oct 2024 13:53:02 +0000 Original-Received: (at 74019) by debbugs.gnu.org; 28 Oct 2024 13:52:24 +0000 Original-Received: from localhost ([127.0.0.1]:52968 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5QAR-0001MS-JG for submit@debbugs.gnu.org; Mon, 28 Oct 2024 09:52:24 -0400 Original-Received: from mxout5.mail.janestreet.com ([64.215.233.18]:38153) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1t5QAQ-0001MD-Bq for 74019@debbugs.gnu.org; Mon, 28 Oct 2024 09:52:22 -0400 In-Reply-To: (Stefan Monnier's message of "Sat, 26 Oct 2024 10:49:19 -0400") DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1730123500; bh=oB1fh6eQ+JvzeJyM9zWrYtXns80pEJboXFD1RtYnrGs=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=MY1BHRieJPGGarYnF0eTKFKXEuMrDT/e3/hEfW49BJOsQFbeQQCN0lwjK4o08W8d5 HRCzChIRLrwUbxxLipfEkGrRu8vzYQUXcjMQW/UMcapSdU3ioPVsDPtQn3H/VtzVhy PwMfQ4OWi3VCqkPzciWN4VLKHdF7+NaOvqbim2nXGsKiY6/SXfJQ13cq15RMqrad+i 9GhLnblTmfB+fMYtu8bJ++q5cEdkokI9Mc8Bsm1SlOWzCwsQMb9Be88C9LnKSfO0KK KGH2eTcNtff5fmz/nUzfHrccePfWI2ekWzKwY+nGJjRsr2gg0FFftcnK2oYX8M8g4E gjFSTPFkjpF5Q== 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:294421 Archived-At: Stefan Monnier writes: >> Add completion-preserve-selection, a defcustom which allows keeping the >> same selected candidate after *Completions* is updated by >> minibuffer-completion-help. > > IIUC, this is a change that affects only `minibuffer-completion-help`, > which is part of the standard UI's *Completions*, right, the generic > completion infrastructure, right? > >> This works correctly with choose-completion-deselect-if-after: If point >> is after a completion (such that choose-completion-deselect-if-after=3Dt >> means it won't be treated as selected), point will still be after that >> completion after updating the list. > > I can't remember what `choose-completion-deselect-if-after` is about, > sorry, but the above reads like "the code doesn't have one of the bugs > I encountered while implemented the code". Is there more to this paragra= ph? Eh, well, it's just that a completion can be "selected" in two different ways: A. if point is on the completion, then choose-completion will always choose it B. if point is right after the completion, then choose-completion will choose it if choose-completion-deselect-if-after is nil The location of point in the completion is preserved, so these two different behaviors are preserved. It's just to contrast with another possible approach, where I only preserve what completion was selected, by moving point to its start after updating. That wouldn't preserve the same behavior in case B. Will improve commit message in next version. >> This feature is primarily motivated by the fact that some other >> completion UIs (e.g. ido, vertico, etc) effectively have this behavior: >> whenever they update the list of completions, they preserve whatever >> candidate is selected. > > Are there cases where the current behavior is preferable? > IOW, why do we need a config var and why does it default to nil? It's a fair point. We could always add the config var later if we end up wanting it - it's very easy. So I think it would make sense to remove the config var, and always have this preservation behavior, if people are OK with that. That would be less complex. >> * lisp/minibuffer.el (completion-preserve-selection): >> (minibuffer-completion-help): >> (minibuffer-next-completion): >> * lisp/simple.el (choose-completion): >> (completion-setup-function): > > I presume you know that this is incomplete. =F0=9F=99=82 Yes :) Just wanted to get initial feedback. > [ BTW, I really regret not moving the `completion-list-mode` out of > `simple.el`. ] Maybe we could still do that? It would definitely make completion UI changes a lot easier... >> + "If non-nil, `minibuffer-completion-help' preserves the selected comp= letion candidate. >> + >> +If non-nil, and point is on a completion candidate in the displayed >> +*Completions* window, `minibuffer-completion-help' will put point on the >> +same candidate after updating *Completions*." > > I think we should be more clear that it *tries* to preserve it. > After all, the selected candidate may simply be absent from the new set > of candidates. Will update in next version. >> @@ -2624,6 +2634,12 @@ minibuffer-completion-help >> (sort-fun (completion-metadata-get all-md 'display-sort-fu= nction)) >> (group-fun (completion-metadata-get all-md 'group-function= )) >> (mainbuf (current-buffer)) >> + (current-candidate-and-offset >> + (when-let* ((window (get-buffer-window "*Completions*" 0)= )) >> + (with-selected-window window >> + (when-let* ((beg (completions--start-of-candidate-at = (point)))) >> + >> + (cons (get-text-property beg 'completion--string) (= - (point) beg)))))) >> ;; If the *Completions* buffer is shown in a new >> ;; window, mark it as softly-dedicated, so bury-buffer in >> ;; minibuffer-hide-completions will know whether to > > Hmm... are we sure here that the `*Completions*`s content is related to > the current completion session? I don't think we want to preserve the > selection when it came from an unrelated use of completion half an > hour earlier. That's why I'm doing get-buffer-window here - I figure that if *Completions* is currently displayed in a window, it's reasonable to preserve the selected candidate. (The selected candidate in that window, I guess - so maybe I should use window-point here?) It still might not be related to the current completion session, since the user might have just manually switched buffers to *Completions*, but I wasn't sure there was a good way to determine that... any suggestions? >> @@ -4905,8 +4928,6 @@ minibuffer-next-completion >> (interactive "p") >> (let ((auto-choose minibuffer-completion-auto-choose)) >> (with-minibuffer-completions-window >> - (when completions-highlight-face >> - (setq-local cursor-face-highlight-nonselected-window t)) >> (if vertical >> (next-line-completion (or n 1)) >> (next-completion (or n 1))) > [...] >> @@ -10451,6 +10458,8 @@ completion-setup-function >> (let ((base-position completion-base-position) >> (insert-fun completion-list-insert-choice-function)) >> (completion-list-mode) >> + (when completions-highlight-face >> + (setq-local cursor-face-highlight-nonselected-window t)) >> (setq-local completion-base-position base-position) >> (setq-local completion-list-insert-choice-function insert-fun)) >> (setq-local completion-reference-buffer mainbuf) > > Why? Prevuously cursor-face-highlight-nonselected-window was only set by next-completion, but minibuffer-completion-help creates a new *Completions* buffer which doesn't have it set, so the selected completion isn't highlighted. Moving that to completion-setup-function causes it to still be highlighted. This is kind of an unrelated improvement, since this is probably nicer anyway - if the user moves point around manually in *Completions*, IMO that should have the same behavior as minibuffer-next-completion, but currently it doesn't highlight the same way if they leave *Completions* because cursor-face-highlight-nonselected-window doesn't get set.