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

[Dmitry Gutov <dmitry@gutov.dev>]

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

On 11/04/2024 09:55, Eli Zaretskii wrote:
>> Cc: 48356@debbugs.gnu.org, Daniel Mendler <mail@daniel-mendler.de>,
>>   JD Smith <jdtsmith@gmail.com>, Juri Linkov <juri@linkov.net>
>> Date: Thu, 11 Apr 2024 04:00:35 +0300
>> From: Dmitry Gutov <dmitry@gutov.dev>
>>
>> All right, please see the new function completion-base-suffix added in
>> 0288bc6c949. Any docstring improvements (and others) are welcome.
> 
> I tried to do that.
> 
> Is there any reason why this function shouldn't be called
> completion-boundary-suffix instead?
> 
>> I guess we should also mention it in NEWS...
> 
> Yes, please.

Sorry about the trouble, here is the next patch on top which essentially 
had to change the function's semantics to match the name above, except 
it needed just the length. Since that made it a very thin wrapper, I'm 
inlining the code back, no docstring or announcement needed.

What else this patch does:

* Removes the variables completion-use-base-affixes and 
completion-base-affixes, just using the completion-base-position 
variable, although with a marker for the field end.
* Changes 'completion--replace' to preserve the said marker.

The result is that the text outside of the current field boundaries is 
maintained for both minibuffer and in-buffer completion (in particular, 
the suffix).

As one downside, it brings back behavior described in 
https://debbugs.gnu.org/34517#14. That doesn't seem too critical to me, 
but opinions might vary.

So more feedback welcome.

Also Cc'ing Visuwesh who filed bug#49931 (related).

[-- Attachment #2: base-suffix-v3.diff --]
[-- Type: text/x-patch, Size: 10246 bytes --]

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index ad6a0928cda..56827f509aa 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,12 +2584,15 @@ 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))))
+             (field-end
+              (save-excursion
+                (forward-char
+                 (cdr (completion-boundaries (buffer-substring start (point))
+                                             minibuffer-completion-table
+                                             minibuffer-completion-predicate
+                                             (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
@@ -2687,38 +2676,25 @@ 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 (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)
@@ -4877,8 +4853,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 +4891,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))