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#70968: 29.2.50; choose-completion on an emacs22-style completion deletes text after point Date: Mon, 16 Sep 2024 15:54:58 -0400 Message-ID: References: <86bk56jhsp.fsf@gnu.org> <377f815c-52d2-4770-ae85-55e096e104b0@gutov.dev> <8634qhipgj.fsf@gnu.org> <7e05fd14-3499-4811-b4bc-b53186b15408@gutov.dev> <86ed5vzzru.fsf@gnu.org> <86o74qh9wv.fsf@gnu.org> Reply-To: Spencer Baugh Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="2223"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: dmitry@gutov.dev, 70968@debbugs.gnu.org, monnier@iro.umontreal.ca, juri@linkov.net To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Mon Sep 16 21:56:16 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 1sqHpX-0000JS-G2 for geb-bug-gnu-emacs@m.gmane-mx.org; Mon, 16 Sep 2024 21:56:15 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1sqHp9-0008WJ-4j; Mon, 16 Sep 2024 15:55:51 -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 1sqHp7-0008Vy-9f for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2024 15:55:49 -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 1sqHp6-0000uZ-LM for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2024 15:55:49 -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=DZOzKxRvmnWVXEQE2P/pMvw0pnaQ+1qHzPd6EsLV5O0=; b=V/gYU/SXktMEBz48r1TvUC3hoFR+BpjyyNMZDp1k2xgPwVIDiDo17lCCZvhWj1JBs0nfv31U5lUwapAbz/GeTmThrXp4rZnISjtNTVHpWH98pcikLOLKjaVGu6I8+qCIXUzQE0GK8yndjZ9FVHWtVL7Lv+ytQWHMrn7Xpeha5MJey+EJYPMucEbBH8PCpsLmn/LFzmj7CxNF6paNLSB1C8KBh6AQ3rYdcey2qYaVGE2//rosHileXFuDxjX0v42OqW1g0+38LKkYxGngoblSsUB0aoBKy8vnV2bd3bb22xcDLe74KiF9qu2PUnouvgyHiUHUdB278g9ohaKjOhQ0bg==; Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1sqHpK-0004pA-9o for bug-gnu-emacs@gnu.org; Mon, 16 Sep 2024 15:56: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, 16 Sep 2024 19:56:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 70968 X-GNU-PR-Package: emacs Original-Received: via spool by 70968-submit@debbugs.gnu.org id=B70968.172651652018172 (code B ref 70968); Mon, 16 Sep 2024 19:56:02 +0000 Original-Received: (at 70968) by debbugs.gnu.org; 16 Sep 2024 19:55:20 +0000 Original-Received: from localhost ([127.0.0.1]:53347 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sqHod-0004j1-AU for submit@debbugs.gnu.org; Mon, 16 Sep 2024 15:55:20 -0400 Original-Received: from mxout2.mail.janestreet.com ([38.105.200.79]:45419) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1sqHob-0004fl-6C for 70968@debbugs.gnu.org; Mon, 16 Sep 2024 15:55:18 -0400 In-Reply-To: <86o74qh9wv.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 14 Sep 2024 12:17:04 +0300") DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=janestreet.com; s=waixah; t=1726516498; bh=DZOzKxRvmnWVXEQE2P/pMvw0pnaQ+1qHzPd6EsLV5O0=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=NIxLyvjZ07dkuFYmMKxgOPFwwkUHdGhEX/F3jhe5D+6ymUwldbmWr5Gcc6csELbnC MGWmkttoD5I9+HUiU2jy0Ip0FoNPf1+nmTLjY0kj1NScay3NNlZVIfRWobmWpyWleQ 5YGrsW63YxZjQiNgLD/0Ejxi9ywZ3gsKXMSMxA6fCUnuWlPtJu0/dsZqpx6k5dvtDy f7J+uFipNIBvDjXLgASL5ynE8LVpLR3y2n0zmxMCXAfHbVN7vA45/V75KfNZNw+KVK GV4KZIEV45bbNiSHDCOX0xKeyzKIiyOy5lnwwl5koBasarKqgnFV396/uRz/ha0UhN 8q41CoREMcuqA== 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:291913 Archived-At: --=-=-= Content-Type: text/plain Eli Zaretskii writes: > I'm okay with adding a new style, per B, but why do we need to > deprecate emacs22 at the same time? Let users who want this new > behavior customize their completion styles to use this new style > instead of emacs22. That'd be fully backward compatible, and will > solve the problem for those who don't like the current behavior. That's fair enough, we don't need to deprecate emacs22 at the same time, we can wait until the new style has been battle-tested. I think a new style should replace emacs22 in the default completion-styles eventually, but that can wait. And after working on this bug for a while, I now am convinced that the new style approach is straightforward, and is the best way. (Although it would also work to make these changes in the emacs22 style) Attached is a patch which adds a new ignore-after-point style. The fix for this bug is entirely contained in the differences between completion-ignore-after-point-all-completions (c-iap-a-c) and completion-emacs22-all-completions (c-e22-a-c). c-e22-a-c always omits the text after point, which means choose-completion always deletes that text, causing this bug. c-iap-a-c includes the text after point in some cases. Whenever the text after point is included, it's grayed out with a new completions-ignored face to indicate it was ignored. The cases where c-iap-a-c includes the text after point are those where the user will have further chances to edit or complete with that text: - When we're doing minibuffer completion and choose-completion will immediately exit the minibuffer, the text after point is not included, since the user won't get any further changes to use it, and it's probably garbage. - When we're doing completion-at-point (or certain kinds of minibuffer completion, e.g. completing a directory in filename completion), the text after point is included. In such cases, the user can always delete it themselves later, or might want to actually use it. To make this work consistently with completion-try-completion (which keeps point before the ignored suffix in both the ignore-after-point and emacs22 styles), choose-completion-string now checks a new 'completion-position-after-insert text property, and moves point to that position. (There are two other changes which are general improvements unrelated to this bug: - The behavior of keeping point before the ignored suffix is more consistent. - Instead of hardcoding try-completion and all-completion, ignore-after-point uses the configured completion styles.) Stefan, Dmitry, any comments? --=-=-= Content-Type: text/x-patch Content-Disposition: inline; filename=0001-Add-ignore-after-point-completion-style.patch >From cdc7ce4b9958a0ef36e911066ecce82a6da09c02 Mon Sep 17 00:00:00 2001 From: Spencer Baugh Date: Mon, 16 Sep 2024 15:15:57 -0400 Subject: [PATCH] Add ignore-after-point completion style * lisp/minibuffer.el (completion--twq-all): Use the original completion faces where possible. (completion-styles-alist, completion-ignore-after-point--force-nil) (completions-ignored, completion-ignore-after-point-try-completion) (completion-ignore-after-point-all-completions): Add ignore-after-point completion style. (bug#70968) * lisp/simple.el (choose-completion-string--should-exit): Add. (choose-completion-string): Call choose-completion-string--should-exit. --- lisp/minibuffer.el | 142 ++++++++++++++++++++++++++++++++++++--------- lisp/simple.el | 47 +++++++++------ 2 files changed, 142 insertions(+), 47 deletions(-) diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el index 72ee5d02002..e5d85ed6fc8 100644 --- a/lisp/minibuffer.el +++ b/lisp/minibuffer.el @@ -674,34 +674,42 @@ completion--twq-all ;; requote them, so that *Completions* can show nicer unquoted values ;; which only get quoted when needed by choose-completion. (nconc - (mapcar (lambda (completion) - (cl-assert (string-prefix-p prefix completion 'ignore-case) t) - (let* ((new (substring completion (length prefix))) - (qnew (funcall qfun new)) - (qprefix - (if (not completion-ignore-case) - qprefix - ;; Make qprefix inherit the case from `completion'. - (let* ((rest (substring completion - 0 (length prefix))) - (qrest (funcall qfun rest))) - (if (string-equal-ignore-case qprefix qrest) - (propertize qrest 'face - 'completions-common-part) - qprefix)))) - (qcompletion (concat qprefix qnew))) - ;; FIXME: Similarly here, Cygwin's mapping trips this - ;; assertion. - ;;(cl-assert - ;; (string-equal-ignore-case - ;; (funcall unquote - ;; (concat (substring string 0 qboundary) - ;; qcompletion)) - ;; (concat (substring ustring 0 boundary) - ;; completion)) - ;; t) - qcompletion)) - completions) + (mapcar + (if (string-equal qprefix prefix) + ;; There's no quoting in the prefix; quoting in the completions + ;; can be simpler and preserve the existing faces. + (lambda (completion) + (concat + (substring completion 0 (length prefix)) + (funcall qfun (substring completion (length prefix))))) + (lambda (completion) + (cl-assert (string-prefix-p prefix completion 'ignore-case) t) + (let* ((new (substring completion (length prefix))) + (qnew (funcall qfun new)) + (qprefix + (if (not completion-ignore-case) + qprefix + ;; Make qprefix inherit the case from `completion'. + (let* ((rest (substring completion + 0 (length prefix))) + (qrest (funcall qfun rest))) + (if (string-equal-ignore-case qprefix qrest) + (propertize qrest 'face + 'completions-common-part) + qprefix)))) + (qcompletion (concat qprefix qnew))) + ;; FIXME: Similarly here, Cygwin's mapping trips this + ;; assertion. + ;;(cl-assert + ;; (string-equal-ignore-case + ;; (funcall unquote + ;; (concat (substring string 0 qboundary) + ;; qcompletion)) + ;; (concat (substring ustring 0 boundary) + ;; completion)) + ;; t) + qcompletion))) + completions) qboundary)))) ;;; Minibuffer completion @@ -1038,6 +1046,12 @@ completion-styles-alist "Prefix completion that only operates on the text before point. I.e. when completing \"foo_bar\" (where _ is the position of point), it will consider all completions candidates matching the glob +pattern \"foo*\" and will add back \"bar\" to the end of it.") + (ignore-after-point + completion-ignore-after-point-try-completion completion-ignore-after-point-all-completions + "Prefix completion that only operates on the text before point. +I.e. when completing \"foo_bar\" (where _ is the position of point), +it will consider all completions candidates matching the glob pattern \"foo*\" and will add back \"bar\" to the end of it.") (basic completion-basic-try-completion completion-basic-all-completions @@ -3692,6 +3706,78 @@ completion-emacs22-all-completions point (car (completion-boundaries beforepoint table pred ""))))) +;;; ignore-after-point completion style. + +(defvar completion-ignore-after-point--force-nil nil + "When non-nil, the ignore-after-point style always returns nil.") + +(defface completions-ignored + '((t (:inherit shadow))) + "Face for text which was ignored by the completion style.") + +(defun completion-ignore-after-point-try-completion (string table pred point) + "Run `completion-try-completion' ignoring the part of STRING after POINT. + +We add the part of STRING after POINT back to the result." + (let ((prefix (substring string 0 point)) + (suffix (substring string point))) + (when-let ((completion + (unless completion-ignore-after-point--force-nil + (let ((completion-ignore-after-point--force-nil t)) + (completion-try-completion prefix table pred point))))) + ;; Add SUFFIX back to COMPLETION. However, previous completion styles failed and + ;; this one succeeded by ignoring SUFFIX. The success of future completion depends + ;; on ignoring SUFFIX. We mostly do that by keeping point right before SUFFIX. + (if (eq completion t) + ;; Keep point in the same place, right before SUFFIX. + (cons string point) + (let ((newstring (car completion)) + (newpoint (cdr completion))) + (cond + ((= (length newstring) newpoint) + ;; NEWPOINT is already right before SUFFIX. + (cons (concat newstring suffix) newpoint)) + ((minibufferp completion-reference-buffer) + ;; Don't allow moving point, keep it right before SUFFIX. + (cons (concat newstring suffix) (length newstring))) + (t + ;; If we're not in a minibuffer, then we're using `completion-at-point', which + ;; calculates a completion region to complete over. We can allow point to + ;; move and still cause SUFFIX to be omitted from the completion region, by + ;; including a space right before SUFFIX. + (cons (concat newstring + ;; Don't add another space if SUFFIX already starts with one. + (when (/= (aref suffix 0) ? ) " ") suffix) + newpoint)))))))) + +(defun completion-ignore-after-point-all-completions (string table pred point) + "Run `completion-all-completions' ignoring the part of STRING after POINT." + (let ((prefix (substring string 0 point)) + (suffix (propertize (substring string point) 'face 'completions-ignored))) + (when-let ((completions + (unless completion-ignore-after-point--force-nil + (let ((completion-ignore-after-point--force-nil t)) + (completion-all-completions prefix table pred point))))) + ;; Add SUFFIX back to each completion. COMPLETIONS may be an improper list (with + ;; the base position in its last cdr) so we can't use `mapcar'. + (let ((tail completions)) + (while (consp tail) + (let* ((completion (car tail)) + (choose-completion-will-exit + (and (minibufferp completion-reference-buffer) + (choose-completion-string--should-exit completion)))) + ;; Include the suffix if, after `choose-completion' runs on COMPLETION, the + ;; user is still able to use and edit the suffix. + (unless choose-completion-will-exit + (let ((end-of-real-completion (length completion))) + (setcar tail (concat completion suffix)) + ;; When chosen, point should go before SUFFIX. + (put-text-property + 0 1 'completion-position-after-insert end-of-real-completion + (car tail))))) + (setq tail (cdr tail)))) + completions))) + ;;; Basic completion. (defun completion--merge-suffix (completion point suffix) diff --git a/lisp/simple.el b/lisp/simple.el index 1dd6bfe5b22..fe68f23c4da 100644 --- a/lisp/simple.el +++ b/lisp/simple.el @@ -10083,6 +10083,20 @@ choose-completion-string-functions If all functions in the list return nil, that means to use the default method of inserting the completion in BUFFER.") +(defun choose-completion-string--should-exit (result) + "Should `choose-completion-string' exit the minibuffer if RESULT is chosen?" + (and + (not completion-no-auto-exit) + minibuffer-completion-table + ;; If this is reading a file name, and the file name chosen + ;; is a directory, don't exit the minibuffer. + (let ((bounds (completion-boundaries + result minibuffer-completion-table + minibuffer-completion-predicate ""))) + ;; The completion chosen leads to a new set of completions + ;; (e.g. it's a directory): don't exit the minibuffer yet. + (not (eq (car bounds) (length result)))))) + (defun choose-completion-string (choice &optional buffer base-position insert-function) "Switch to BUFFER and insert the completion choice CHOICE. @@ -10116,10 +10130,13 @@ choose-completion-string ;; comes from buffer-substring-no-properties. ;;(remove-text-properties 0 (length choice) '(mouse-face nil) choice) ;; Insert the completion into the buffer where it was requested. - (funcall (or insert-function completion-list-insert-choice-function) - (or (car base-position) (point)) - (or (cadr base-position) (point)) - choice) + (let ((beg (or (car base-position) (point))) + (end (or (cadr base-position) (point)))) + (funcall (or insert-function completion-list-insert-choice-function) + beg end choice) + (unless (string-empty-p choice) + (when-let ((pos (get-text-property 0 'completion-position-after-insert choice))) + (goto-char (+ pos beg))))) ;; Update point in the window that BUFFER is showing in. (let ((window (get-buffer-window buffer t))) (set-window-point window (point))) @@ -10127,21 +10144,13 @@ choose-completion-string (and (not completion-no-auto-exit) (minibufferp buffer) minibuffer-completion-table - ;; If this is reading a file name, and the file name chosen - ;; is a directory, don't exit the minibuffer. - (let* ((result (buffer-substring (field-beginning) (point))) - (bounds - (completion-boundaries result minibuffer-completion-table - minibuffer-completion-predicate - ""))) - (if (eq (car bounds) (length result)) - ;; The completion chosen leads to a new set of completions - ;; (e.g. it's a directory): don't exit the minibuffer yet. - (let ((mini (active-minibuffer-window))) - (select-window mini) - (when minibuffer-auto-raise - (raise-frame (window-frame mini)))) - (exit-minibuffer)))))))) + (if (choose-completion-string--should-exit + (buffer-substring (field-beginning) (point))) + (exit-minibuffer) + (let ((mini (active-minibuffer-window))) + (select-window mini) + (when minibuffer-auto-raise + (raise-frame (window-frame mini)))))))))) (define-derived-mode completion-list-mode nil "Completion List" "Major mode for buffers showing lists of possible completions. -- 2.39.3 --=-=-=--