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: 2989 bytes --]

On 20/03/2022 22:34, Juri Linkov wrote:
>>>>> IMHO, the root of the problem is in completion-all-completions
>>>>> that returns in the last `cdr' only the start of completions,
>>>>> but not the end of completions.
>>>> Agreed.
>>> If I recall correctly, someone wrote a patch which fixed the root cause.
>> The only patch that removes these comments
>>
>>          ;; FIXME: We need to additionally return the info needed for the
>>          ;; second part of completion-base-position.
>>
>>          ;; FIXME: We should pay attention to completion
>>          ;; boundaries here, but currently
>>          ;; completion-all-completions does not give us the
>>          ;; necessary information.
>>
>> ishttps://lists.gnu.org/archive/html/emacs-devel/2021-08/msg00412.html
>> in bug#47711 and bug#48841.
> I wonder what is the fate of this patch?  There was a long discussion
> without a clear outcome.  Maybe it's possible to split the patch
> to smaller parts where a separate patch would add the feature needed here
> to return the end position of the completion boundaries?

It seems there is a range of solutions we could take here.

On the one side would be a replacement of the -all-completions API with 
something like completion-filter-completions as proposed by Daniel. It's 
still a reasonable choice, but a rather breaking one, and the patch 
would need to be majorly rewritten anyway, given the amount of changes 
in the area since it was submitted.

On the other, we could preserve the current convention again, and add a 
new dynamic variable which would be assigned in every relevant 
completion style, to store the rightmost-position (the length of 
suffix). Then the callers would fetch it from that variable. Similar to 
what we do with completion-lazy-hilit-fn now.

Or some variations of that.

But what I don't quite see yet, is why wouldn't the caller be able to 
compute the bounds cheaply enough? We could offer an accessor function. 
See the new logic in the attached patch (but imagine it extracted to a 
named function).

Note that it doesn't work too well now, because in the example like

   ~/v/e-|/src

the completions include the trailing slash. And base-suffix includes a 
starting slash as well (according to boundaries returned by the 
completion table). So when I choose one of the completions using 
minibuffer-next-completion, the minibuffer contents look like

   ~/v/emacs-master//src

...which translates to "/" because of the double slash -- the filesystem 
root directory (*). But that's the same data which would be used by any 
other proposed solution, too. So maybe it should be either be fixed in 
the completion table (avoid adding trailing slash when the last boundary 
is already followed by slash?), or the insertion code should do some 
additional post-processing of the completion string.

(*) And then, when I press tab while point is between slashes -- /|/ -- 
it jumps to the beginning of the input, but that's a secondary problem.

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

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 0a844c538b4..33c175aa3c6 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -2582,16 +2582,13 @@ minibuffer-completion-help
              (minibuffer-completion-base (substring string 0 base-size))
              (base-prefix (buffer-substring (minibuffer--completion-prompt-end)
                                             (+ start base-size)))
-             (base-suffix
-              (if (or (eq (alist-get 'category (cdr md)) 'file)
-                      completion-in-region-mode-predicate)
-                  (buffer-substring
-                   (save-excursion
-                     (if completion-in-region-mode-predicate
-                         (point)
-                       (or (search-forward "/" nil t) (point-max))))
-                   (point-max))
-                ""))
+             (base-suffix (let ((suffix (buffer-substring (point) end)))
+                            (substring
+                             suffix
+                             (cdr (completion-boundaries string
+                                                         minibuffer-completion-table
+                                                         minibuffer-completion-predicate
+                                                         suffix)))))
              (all-md (completion--metadata (buffer-substring-no-properties
                                             start (point))
                                            base-size md