Subject: bug#77718: 31.0.50; completion styles substring and flex are broken

From: Stephen Berman <[email-masked]>
Date: Fri, 20 Jun 2025 19:19:07 +0200

On Wed, 18 Jun 2025 16:10:11 -0400 Stefan Monnier <[email-masked]> wrote:

>> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
>> index 122459be062..598383e7b51 100644
>> --- a/lisp/minibuffer.el
>> +++ b/lisp/minibuffer.el
>> @@ -4516,6 +4516,14 @@ completion--common-suffix
>>    "Return the common suffix of the strings STRS."
>>    (nreverse (try-completion "" (mapcar #'reverse strs))))
>>  
>> +(defcustom completion-/-not-common-suffix nil
>> +  "If non-nil, ignore \"/\" as common suffix.
>> +With the substring completion style enabled, this prevents displaying
>> +\"/\" as the common substring suffix when all completion candidate end
>> +in \"/\" (that was the only dispay prior to Emacs 31)."
>> +  :version "31.1"
>> +  :type 'boolean)
>
> Like Daniel, I'd much rather find a solution that doesn't involve
> a user config.
>
>>  (defun completion-pcm--merge-completions (strs pattern)
>>    "Extract the commonality in STRS, with the help of PATTERN.
>>  PATTERN can contain strings and symbols chosen among `star', `any', `point',
>> @@ -4622,7 +4630,9 @@ completion-pcm--merge-completions
>>                                  (mapcar (lambda (str) (substring str skip))
>>                                          comps))))))
>>                        (cl-assert (stringp suffix))
>> -                      (unless (equal suffix "")
>> +                      (unless (if completion-/-not-common-suffix
>> +                                  (member suffix '("" "/"))
>> +                                (equal suffix ""))
>>                          (push suffix res))))
>>                    ;; We pushed these wildcards on RES, so we're done with them.
>>                    (setq wildcards nil))
>
> Comments:
>
> - We shouldn't hardcode "/" here, since this code is not specific to
>   file names.  IOW we should use the `completion-boundaries`
>   functionality instead to detect if the suffix is equal to the
>   boundary separator.
> - Maybe we can/should limit this "/" special case to the situation where
>   `prefix` is empty (or where the string before "suffix" also ends in
>   a boundary-separator).

Does the attached patch go in the direction you intended with your
comments?  From my initial testing it gives the same results as my patch
with hardcoded "/", and I haven't seen bad results yet; however, I don't
have a good understanding of `completion-boundaries' and may well have
not used it as you meant.  Also, even if it does do what you suggested,
since it in effect reverts the substring completion display and behavior
involving directories to what it was before Spencer's changes, I guess
he wouldn't accept it, just as he rejected my first patch.  That was why
I proposed to make the behavior configurable, but you don't like that;
why not?

Steve Berman

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 122459be062..64966a7a35c 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -4613,16 +4613,22 @@ completion-pcm--merge-completions
                              ;; If prefix is one of the completions, there's no
                              ;; suffix left to find.
                              (not (assoc-string prefix comps t)))
-                    (let ((suffix
-                           (completion--common-suffix
-                            (if (zerop (length prefix)) comps
-                              ;; Ignore the chars in the common prefix, so we
-                              ;; don't merge '("abc" "abbc") as "ab*bc".
-                              (let ((skip (length prefix)))
-                                (mapcar (lambda (str) (substring str skip))
-                                        comps))))))
+                    (let* ((suffix
+                            (completion--common-suffix
+                             (if (zerop (length prefix)) comps
+                               ;; Ignore the chars in the common prefix, so we
+                               ;; don't merge '("abc" "abbc") as "ab*bc".
+                               (let ((skip (length prefix)))
+                                 (mapcar (lambda (str) (substring str skip))
+                                         comps)))))
+                           (bounds (completion-boundaries
+                                     prefix comps minibuffer-completion-predicate
+                                     suffix))
+                           (sep (substring suffix (max 0 (1- (cdr bounds)))
+                                           (cdr bounds))))
                       (cl-assert (stringp suffix))
-                      (unless (equal suffix "")
+                      (unless (or (equal suffix "")
+                                  (and (equal prefix "") (equal suffix sep)))
                         (push suffix res))))
                   ;; We pushed these wildcards on RES, so we're done with them.
                   (setq wildcards nil))