bug#41705: 28.0.50; minibuffer completion of ".../*/*" shouldn't be "...//"

[Pip Cet]

"Basil L. Contovounesios" <contovob@tcd.ie> writes:
> Eli Zaretskii <eliz@gnu.org> writes:
>>> From: Pip Cet <pipcet@gmail.com>
>>> Date: Thu, 04 Jun 2020 09:35:12 +0000
>>> 
>>> ./emacs -Q
>>> enter: C-x C-f * / * <tab>
>>> 
>>> expected result: a list of sub-subdirectories of the emacs src dir. A
>>> "Find file: .../emacs/src/*/" prompt.
>>> 
>>> actual result: prompt says "Find file: .../emacs/src//", further tab
>>> completion uses the root directory.
>>> 
>>> I'd noticed this for a while now, but thought it was a local
>>> configuration issue. It happens in emacs -Q, too, though.
>>> 
>>> For master, we should fix minibuffer.el properly, but I'm not sure what
>>> to do on emacs-27. It's a regression (from Emacs 26) that might annoy
>>> many users of tab completion in the minibuffer.
>>
>> Can you bisect to find which commit broke this?
>
> I selectively applied commits onto emacs-26 rather than bisecting, and
> it seems to point to [1].  Paging Stefan.

Thanks for that!

I'd actually suspected the same file as well, and started playing around
with the code a little. I'm suspicious of the code in
completion-pcm--optimize-pattern which turns a '(star) pattern into
nil. That's correct as far as which strings qualify as matches, but it's
incorrect because it doesn't survive the completion-pcm--pattern->string
round trip.

I think we get something closer to Emacs-26 behavior back with this
patch.

>From b21734a42860b153a381ac92e5c17f863da4c8da Mon Sep 17 00:00:00 2001
From: Pip Cet <pipcet@gmail.com>
Date: Thu, 4 Jun 2020 18:21:42 +0000
Subject: [PATCH] Don't optimize away star patterns in minibuffer file name
 completion

* lisp/minibuffer.el (completion-pcm--optimize-pattern): Keep
'star in the pattern.  (Bug#41705)
---
 lisp/minibuffer.el | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index d2c3f9045e..15deccc1c3 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3112,12 +3112,12 @@ completion-pcm--optimize-pattern
     (while p
       (pcase p
         (`(,(or 'any 'any-delim) point . ,rest) (setq p `(point . ,rest)))
-        ;; This is not just a performance improvement: it also turns
-        ;; a terminating `point' into an implicit `any', which
-        ;; affects the final position of point (because `point' gets
-        ;; turned into a non-greedy ".*?" regexp whereas we need
-        ;; it the be greedy when it's at the end, see bug#38458).
-        (`(,(pred symbolp)) (setq p nil)) ;Implicit terminating `any'.
+        ;; This is not just a performance improvement: it turns a
+        ;; terminating `point' into an implicit `any', which affects
+        ;; the final position of point (because `point' gets turned
+        ;; into a non-greedy ".*?" regexp whereas we need it to be
+        ;; greedy when it's at the end, see bug#38458).
+        (`(point) (setq p nil)) ;Implicit terminating `any'.
         (_ (push (pop p) n))))
     (nreverse n)))
 
-- 
2.27.0.rc0

But it's fairly obvious this code is both tricky and should be worked on
some more, which probably precludes inclusion in emacs-27 (without
further testing) and master, respectively.