bug#66187: read-file-name unexpected behavior when MUSTMATCH is a function

[Joseph Turner via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> I had a look and tried this:
>
> [2. text/x-diff; 0001-WIP-Bug-66187.patch]
> From ad895d2df5c69e015c2c7eba5116ab2d440e1fbc Mon Sep 17 00:00:00 2001
> From: Michael Heerdegen <michael_heerdegen@web.de>
> Date: Wed, 27 Sep 2023 03:32:37 +0200
> Subject: [PATCH] WIP: Bug#66187
>
> ---
>  lisp/minibuffer.el | 29 +++++++++++++++--------------
>  1 file changed, 15 insertions(+), 14 deletions(-)
>
> diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
> index 2120e31775e..79a8786fbac 100644
> --- a/lisp/minibuffer.el
> +++ b/lisp/minibuffer.el
> @@ -3061,13 +3061,23 @@ completion-file-name-table
>            (funcall (or pred 'file-exists-p) string)))
>
>         (t
> -        (let* ((name (file-name-nondirectory string))
> +        (let* ((test-directory (lambda (s)
> +                                 (let ((len (length s)))
> +                                   (and (> len 0) (eq (aref s (1- len)) ?/)))))
> +               (should-complete
> +                (and pred
> +                     (if (eq pred 'file-directory-p)
> +                         test-directory
> +                       (lambda (f)
> +                         (or (funcall test-directory f)
> +                             (funcall pred f))))))
> +               (name (file-name-nondirectory string))
>                 (specdir (file-name-directory string))
>                 (realdir (or specdir default-directory)))
>
>            (cond
>             ((null action)
> -            (let ((comp (file-name-completion name realdir pred)))
> +            (let ((comp (file-name-completion name realdir should-complete)))
>                (if (stringp comp)
>                    (concat specdir comp)
>                  comp)))
> @@ -3078,18 +3088,9 @@ completion-file-name-table
>                ;; Check the predicate, if necessary.
>                (unless (memq pred '(nil file-exists-p))
>                  (let ((comp ())
> -                      (pred
> -                       (if (eq pred 'file-directory-p)
> -                           ;; Brute-force speed up for directory checking:
> -                           ;; Discard strings which don't end in a slash.
> -                           (lambda (s)
> -                             (let ((len (length s)))
> -                               (and (> len 0) (eq (aref s (1- len)) ?/))))
> -                         ;; Must do it the hard (and slow) way.
> -                         pred)))
> -                  (let ((default-directory (expand-file-name realdir)))
> -                    (dolist (tem all)
> -                      (if (funcall pred tem) (push tem comp))))
> +                      (default-directory (expand-file-name realdir)))
> +                  (dolist (tem all)
> +                    (if (funcall should-complete tem) (push tem comp)))
>                    (setq all (nreverse comp))))
>
>                all))))))

> I thought this would make Emacs complete as you want.  But: what files
> should be shown when hitting TAB?  I decided to show only matching
> files, plus all directories (seems logical, since these may contain
> matching files).

> But that gives a bad user experience: When I tried this I thought it
> would not work because completion did not accept an existing empty
> directory.  Then I saw that it actually was not empty.  But it contained only
> plain files, no subdirectories, and TAB displayed nothing.  Quite confusing!

I dug a little more and realized that the IMO unexpected behavior
resides in completing-read itself.  According to the completing-read
docstring, its REQUIRE-MATCH argument can be

- a function, which will be called with the input as the
  argument.  If the function returns a non-nil value, the
  minibuffer is exited with that argument as the value.

I incorrectly assumed that this function would prevent matching items
inside COLLECTION for which the function returns nil.  What it actually
does it accept input which is not part of COLLECTION if the
REQUIRE-MATCH function returns non-nil.  IOW, it doesn't narrow the
potential completion options; it widens them.

For example, given:

(completing-read "Prompt: " '("a" "b") nil
                 (lambda (input)
                   (string= "a" input)))

I expected that the prompt would refuse to complete "b". I was wrong.

Since completing-read is such a fundamental part of Emacs, I doubt it
will be possible to change this behavior.  What do you think about the
attached patch to clarify the completing-read docstring?

Thank you!!

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clarify-completing-read-REQUIRE-MATCH-function-docst.patch --]
[-- Type: text/x-diff, Size: 1316 bytes --]

From 10fa6dd5c659bed2b818fb925445cab2cbbb5325 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Tue, 3 Oct 2023 14:40:22 -0700
Subject: [PATCH] Clarify completing-read REQUIRE-MATCH function docstring

* src/minibuf.c (Fcompleting_read): Explicitly note that REQUIRE-MATCH
function does not narrow the completion candidates.
---
 src/minibuf.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/minibuf.c b/src/minibuf.c
index 58adde1bf66..616be7091ee 100644
--- a/src/minibuf.c
+++ b/src/minibuf.c
@@ -1999,8 +1999,11 @@ REQUIRE-MATCH can take the following values:
   `minibuffer-complete' right before `minibuffer-complete-and-exit'
   and the input is not an element of COLLECTION.
 - a function, which will be called with the input as the
-  argument.  If the function returns a non-nil value, the
-  minibuffer is exited with that argument as the value.
+  argument.  If the function returns a non-nil value or if
+  the input is a member of COLLECTION, the minibuffer is
+  exited with that argument as the value.  If the function
+  returns nil and the input is not a member of COLLECTION,
+  then the user is not allowed to exit.
 - anything else behaves like t except that typing RET does not exit if it
   does non-null completion.
 
-- 
2.41.0