Subject: Re: bug#48356: 28.0.50; choose-completion discards the suffix after the completion boundary

From: Dmitry Gutov <[email-masked]>
Date: Sat, 20 Apr 2024 03:12:59 +0300

On 18/04/2024 17:25, Spencer Baugh wrote:

>>> But then the user could change the mind
>>> and still select a candidate.  This would interfere with the
>>> contents of the minibuffer.
>>
>> Suppose they do. But the list of completions they're shown is for an
>> outdated input. Does it really make more sense to erase the current
>> input than to insert the completion where it was supposed to go?
> 
> Yeah, this patch changes the behavior in the case of an outdated
> *Completions* buffer, and the new behavior is buggy, but I think the old
> behavior was also buggy and the new behavior is better.
> 
> Example: In emacs -Q, if I type
> 
> C-x C-f ~/src/emacs/emacs-2 <TAB>
> 
> I get completions "emacs-28" and "emacs-29"
> 
> Suppose I then type /src (without hitting tab or updating the
> *Completions* buffer) so that the minibuffer contains:
> 
> ~/src/emacs/emacs-2/src
> 
> Then I click on one of the completions to choose it.
> 
> Before this patch the minibuffer will contain:
> 
> ~/src/emacs/emacs-28/
> 
> and after this patch it will contain
> 
> ~/src/emacs/emacs-28//src
> 
> Both of these are wrong IMO, but IMO the second one is closer to
> correct, and it's more consistent with the normal case (when
> *Completions* is up to date) and with in-buffer completion behavior, so
> I think this patch is an improvement and could be installed.
> 
> Still, maybe we can get it to the point where clicking on an outdated
> completion makes the minibuffer contain
> 
> ~/src/emacs/emacs-28/src
> 
> instead?  Which I think is the correct behavior.
> 
> But I don't think we need to do that before landing this patch.

That gave me an idea. With a bit more spaghetti, we can support both 
this scenario and the others mentioned previously that didn't have field 
boundaries in the "new" input:

Whenever POINT > END, we can re-query the completion boundaries again 
inside completion-list-insert-choice-function and adjust END. In theory, 
I guess we'd ideally also recompute the completion entity first (to pass 
the correct prefix and suffix), but the capf function is not in context 
anymore. As long as the field boundaries simply looks for chars in 
passed string, this should work fine.

The attached v4 adds a new step and also fixes an issue apparently 
introduced in 2021 with 0ce2f591ff9 when making 
minibuffer-completion-table and others buffer-local. The bindings for 
CTABLE and other 2 vars currently always set them to nil because the 
variables are looked up right after the current buffer is changed 
(with-current-buffer standard-output ...).

This can affect the completion--done call at the end, reverting it to 
the previous behavior. But nobody noticed the change, so...

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index ad6a0928cda..61395577035 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -112,20 +112,6 @@ completion-boundaries
     (cons (or (cadr boundaries) 0)
           (or (cddr boundaries) (length suffix)))))
 
-(defun completion-base-suffix (start end collection predicate)
-  "Return suffix of completion of buffer text between START and END.
-COLLECTION and PREDICATE are, respectively, the completion's
-completion table and predicate, as in `completion-boundaries' (which see).
-Value is a substring of buffer text between point and END.  It is
-the completion suffix that follows the completion boundary."
-  (let ((suffix (buffer-substring (point) end)))
-    (substring
-     suffix
-     (cdr (completion-boundaries (buffer-substring start (point))
-                                 collection
-                                 predicate
-                                 suffix)))))
-
 (defun completion-metadata (string table pred)
   "Return the metadata of elements to complete at the end of STRING.
 This metadata is an alist.  Currently understood keys are:
@@ -1377,7 +1363,7 @@ completion--replace
       (setq newtext (substring newtext 0 (- suffix-len))))
     (goto-char beg)
     (let ((length (- end beg)))         ;Read `end' before we insert the text.
-      (insert-and-inherit newtext)
+      (insert-before-markers-and-inherit newtext)
       (delete-region (point) (+ (point) length)))
     (forward-char suffix-len)))
 
@@ -2598,17 +2584,23 @@ minibuffer-completion-help
              (base-size (or (cdr last) 0))
              (prefix (unless (zerop base-size) (substring string 0 base-size)))
              (minibuffer-completion-base (substring string 0 base-size))
-             (base-prefix (buffer-substring (minibuffer--completion-prompt-end)
-                                            (+ start base-size)))
-             (base-suffix (concat (completion-base-suffix start end
-                                                          minibuffer-completion-table
-                                                          minibuffer-completion-predicate)
-                                  (buffer-substring end (point-max))))
+             (ctable minibuffer-completion-table)
+             (cpred minibuffer-completion-predicate)
+             (cprops completion-extra-properties)
+             (field-end
+              (save-excursion
+                (forward-char
+                 (cdr (completion-boundaries (buffer-substring start (point))
+                                             ctable
+                                             cpred
+                                             (buffer-substring (point) end))))
+                (point-marker)))
+             (field-char (and (< field-end end) (char-after field-end)))
              (all-md (completion--metadata (buffer-substring-no-properties
                                             start (point))
                                            base-size md
-                                           minibuffer-completion-table
-                                           minibuffer-completion-predicate))
+                                           ctable
+                                           cpred))
              (ann-fun (completion-metadata-get all-md 'annotation-function))
              (aff-fun (completion-metadata-get all-md 'affixation-function))
              (sort-fun (completion-metadata-get all-md 'display-sort-function))
@@ -2687,38 +2679,31 @@ minibuffer-completion-help
 
                       (with-current-buffer standard-output
                         (setq-local completion-base-position
-                             (list (+ start base-size)
-                                   ;; FIXME: We should pay attention to completion
-                                   ;; boundaries here, but currently
-                                   ;; completion-all-completions does not give us the
-                                   ;; necessary information.
-                                   end))
-                        (setq-local completion-base-affixes
-                                    (list base-prefix base-suffix))
+                                    (list (+ start base-size) field-end))
                         (setq-local completion-list-insert-choice-function
-                             (let ((ctable minibuffer-completion-table)
-                                   (cpred minibuffer-completion-predicate)
-                                   (cprops completion-extra-properties))
                                (lambda (start end choice)
-                                 (if (and (stringp start) (stringp end))
-                                     (progn
-                                       (delete-minibuffer-contents)
-                                       (insert start choice)
-                                       ;; Keep point after completion before suffix
-                                       (save-excursion (insert
-                                                        (completion--merge-suffix
-                                                         choice
-                                                         (1- (length choice))
-                                                         end))))
-                                   (unless (or (zerop (length prefix))
-                                               (equal prefix
-                                                      (buffer-substring-no-properties
-                                                       (max (point-min)
-                                                            (- start (length prefix)))
-                                                       start)))
-                                     (message "*Completions* out of date"))
-                                   ;; FIXME: Use `md' to do quoting&terminator here.
-                                   (completion--replace start end choice))
+                                 (unless (or (zerop (length prefix))
+                                             (equal prefix
+                                                    (buffer-substring-no-properties
+                                                     (max (point-min)
+                                                          (- start (length prefix)))
+                                                     start)))
+                                   (message "*Completions* out of date"))
+                                 (when (> (point) end)
+                                   ;; Completion suffix has changed, have to adapt.
+                                   (setq end (+ end
+                                                (cdr (completion-boundaries
+                                                      (concat prefix choice) ctable cpred
+                                                      (buffer-substring end (point))))))
+                                   ;; Stopped before some field boundary.
+                                   (when (> (point) end)
+                                     (setq field-char (char-after end))))
+                                 (when (and field-char
+                                            (= (aref choice (1- (length choice)))
+                                               field-char))
+                                   (setq end (1+ end)))
+                                 ;; FIXME: Use `md' to do quoting&terminator here.
+                                 (completion--replace start end choice)
                                  (let* ((minibuffer-completion-table ctable)
                                         (minibuffer-completion-predicate cpred)
                                         (completion-extra-properties cprops)
@@ -2729,7 +2714,7 @@ minibuffer-completion-help
                                    ;; completion is not finished.
                                    (completion--done result
                                                      (if (eq (car bounds) (length result))
-                                                         'exact 'finished)))))))
+                                                         'exact 'finished))))))
 
                       (display-completion-list completions nil group-fun)))))
           nil)))
@@ -4877,8 +4862,7 @@ minibuffer-next-completion
           (next-line-completion (or n 1))
         (next-completion (or n 1)))
       (when auto-choose
-        (let ((completion-use-base-affixes t)
-              (completion-auto-deselect nil))
+        (let ((completion-auto-deselect nil))
           (choose-completion nil t t))))))
 
 (defun minibuffer-previous-completion (&optional n)
@@ -4916,8 +4900,7 @@ minibuffer-choose-completion
 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))))
+    (choose-completion nil no-exit no-quit)))
 
 (defun minibuffer-choose-completion-or-exit (&optional no-exit no-quit)
   "Choose the completion from the minibuffer or exit the minibuffer.
diff --git a/lisp/simple.el b/lisp/simple.el
index e4629ce3db7..741cf59f344 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -9856,16 +9856,6 @@ completion-base-position
 where the completion should be inserted and END (if non-nil) is the end
 of the text to replace.  If END is nil, point is used instead.")
 
-(defvar completion-base-affixes nil
-  "Base context of the text corresponding to the shown completions.
-This variable is used in the *Completions* buffer.
-Its value is a list of the form (PREFIX SUFFIX) where PREFIX is the text
-before the place where completion should be inserted, and SUFFIX is the text
-after the completion.")
-
-(defvar completion-use-base-affixes nil
-  "Non-nil means to restore original prefix and suffix in the minibuffer.")
-
 (defvar completion-list-insert-choice-function #'completion--replace
   "Function to use to insert the text chosen in *Completions*.
 Called with three arguments (BEG END TEXT), it should replace the text
@@ -10126,7 +10116,6 @@ choose-completion
   (with-current-buffer (window-buffer (posn-window (event-start event)))
     (let ((buffer completion-reference-buffer)
           (base-position completion-base-position)
-          (base-affixes completion-base-affixes)
           (insert-function completion-list-insert-choice-function)
           (completion-no-auto-exit (if no-exit t completion-no-auto-exit))
           (choice
@@ -10159,13 +10148,7 @@ choose-completion
       (with-current-buffer buffer
         (choose-completion-string
          choice buffer
-         ;; Don't allow affixes to replace the whole buffer when not
-         ;; in the minibuffer.  Thus check for `completion-in-region-mode'
-         ;; to ignore non-nil value of `completion-use-base-affixes' set by
-         ;; `minibuffer-choose-completion'.
-         (or (and (not completion-in-region-mode)
-                  completion-use-base-affixes base-affixes)
-             base-position
+         (or base-position
              ;; If all else fails, just guess.
              (list (choose-completion-guess-base-position choice)))
          insert-function)))))
@@ -10321,11 +10304,9 @@ completion-setup-function
                 (buffer-substring (minibuffer-prompt-end) (point)))))))
     (with-current-buffer standard-output
       (let ((base-position completion-base-position)
-            (base-affixes completion-base-affixes)
             (insert-fun completion-list-insert-choice-function))
         (completion-list-mode)
         (setq-local completion-base-position base-position)
-        (setq-local completion-base-affixes base-affixes)
         (setq-local completion-list-insert-choice-function insert-fun))
       (setq-local completion-reference-buffer mainbuf)
       (if base-dir (setq default-directory base-dir))