bug#62700: 29.0.60; minibuffer-{previous,next,choose}-completion behave unintuitively when point is not at end of buffer

[sbaugh@catern.com]

[-- Attachment #1: Type: text/plain, Size: 2341 bytes --]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: sbaugh@catern.com
>> Date: Fri, 21 Apr 2023 18:56:35 +0000 (UTC)
>> Cc: Spencer Baugh <sbaugh@janestreet.com>, 62700@debbugs.gnu.org,
>> 	juri@linkov.net
>> 
>> >> Ah, I thought Eli still wanted a backport version because this changes
>> >> code which has been on Emacs 29 for over a year.
>> >
>> > Indeed, that's what I would like to see on the release branch.  Mainly
>> > because even if this is deemed a bug, it happens in a relatively rare
>> > situation, so I'd like to avoid risking breakage in code which affects
>> > other situations.
>> 
>> Here's the backport for the release branch.
>
> Thanks, but I'd like to make this still safer for the release branch:
>
>> --- a/lisp/minibuffer.el
>> +++ b/lisp/minibuffer.el
>> @@ -4464,13 +4464,21 @@ minibuffer-next-completion
>>  When `minibuffer-completion-auto-choose' is non-nil, then also
>>  insert the selected completion to the minibuffer."
>>    (interactive "p")
>> -  (let ((auto-choose minibuffer-completion-auto-choose))
>> +  (let* ((auto-choose minibuffer-completion-auto-choose)
>> +         ;; Backported fix for bug#62700
>> +         (md (completion--field-metadata (minibuffer--completion-prompt-end)))
>> +         (base-suffix
>> +          (if (eq (alist-get 'category (cdr md)) 'file)
>> +              (buffer-substring (save-excursion (search-forward "/" nil t) (point))
>> +                                (point-max))
>> +            "")))
>>      (with-minibuffer-completions-window
>>        (when completions-highlight-face
>>          (setq-local cursor-face-highlight-nonselected-window t))
>>        (next-completion (or n 1))
>>        (when auto-choose
>> -        (let ((completion-use-base-affixes t))
>> +        (let ((completion-use-base-affixes t)
>> +              (completion-base-affixes (list (car completion-base-affixes) base-suffix)))
>>            (choose-completion nil t t))))))
>
> Here, the values used only when minibuffer-completion-auto-choose is
> non-nil should be computed only when that variable is non-nil,
> preferably inside the '(when auto-choose' clause.

OK, here's the patch with this change.

(As discussed elsewhere in the thread, the patch includes changes to
minibuffer-choose-completion because that function also is affected by
the bug and also needs to be fixed)


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-point-not-at-EOB-in-minibuffer-choose-complet.patch --]
[-- Type: text/x-patch, Size: 3363 bytes --]

From d446bec7d59944e25f478a63bd6c980ca7ce48d6 Mon Sep 17 00:00:00 2001
From: Spencer Baugh <sbaugh@catern.com>
Date: Fri, 21 Apr 2023 14:55:00 -0400
Subject: [PATCH] Handle point not at EOB in minibuffer-choose-completion

Without this change, only the minibuffer contents before point are
cleared when a completion is chosen, which results in stray text when
point is in the middle of the minibuffer.

After this change, we heuristically decide either to clear the whole
buffer or only part of it, taking into account the location of point.

This is a backport for the Emacs 29 release branch of a simpler fix in
minibuffer-completion-help.

* lisp/minibuffer.el (minibuffer-next-completion):
(minibuffer-choose-completion):
Recalculate completion-base-affixes with point
---
 lisp/minibuffer.el | 30 +++++++++++++++++++++++++-----
 1 file changed, 25 insertions(+), 5 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 21d4607e7cf..f457ecfcf7d 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4460,13 +4460,25 @@ minibuffer-next-completion
 When `minibuffer-completion-auto-choose' is non-nil, then also
 insert the selected completion to the minibuffer."
   (interactive "p")
-  (let ((auto-choose minibuffer-completion-auto-choose))
+  (let ((auto-choose minibuffer-completion-auto-choose)
+         (buf (current-buffer)))
     (with-minibuffer-completions-window
       (when completions-highlight-face
         (setq-local cursor-face-highlight-nonselected-window t))
       (next-completion (or n 1))
       (when auto-choose
-        (let ((completion-use-base-affixes t))
+        (let* ((completion-use-base-affixes t)
+               ;; Backported fix for bug#62700
+               (md
+                (with-current-buffer buf
+                  (completion--field-metadata (minibuffer--completion-prompt-end))))
+               (base-suffix
+                (if (eq (alist-get 'category (cdr md)) 'file)
+                    (with-current-buffer buf
+                      (buffer-substring (save-excursion (search-forward "/" nil t) (point))
+                                        (point-max)))
+                  ""))
+              (completion-base-affixes (list (car completion-base-affixes) base-suffix)))
           (choose-completion nil t t))))))
 
 (defun minibuffer-previous-completion (&optional n)
@@ -4485,9 +4497,17 @@ minibuffer-choose-completion
 If NO-QUIT is non-nil, insert the completion at point to the
 minibuffer, but don't quit the completions window."
   (interactive "P")
-  (with-minibuffer-completions-window
-    (let ((completion-use-base-affixes t))
-      (choose-completion nil no-exit no-quit))))
+  ;; Backported fix for bug#62700
+  (let* ((md (completion--field-metadata (minibuffer--completion-prompt-end)))
+         (base-suffix
+          (if (eq (alist-get 'category (cdr md)) 'file)
+              (buffer-substring (save-excursion (search-forward "/" nil t) (point))
+                                (point-max))
+            "")))
+    (with-minibuffer-completions-window
+      (let ((completion-use-base-affixes t)
+            (completion-base-affixes (list (car completion-base-affixes) base-suffix)))
+        (choose-completion nil no-exit no-quit)))))
 
 (defun minibuffer-complete-history ()
   "Complete the minibuffer history as far as possible.
-- 
2.38.0