bug#68815: Unexpected behavior with read-file-name and functional REQUIRE-MATCH argument

[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: 1107 bytes --]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>>> Also, performance can be a concern (in many cases it makes more sense
>>> to make the caller pass the unquoted name rather than force it to quote
>>> the name only for PRED to unquote it).
>>
>> The REQUIRE-MATCH function is only called once when the user attempts to
>> exit the minibuffer.  Would you please explain the performance concern?
>
> Oh, sorry, I got confused.  Indeed, you're wrapping the REQUIRE-MATCH
> arg, not the PRED arg I was ranting about.  Duh!
>
> It would be OK for `emacs-29`, indeed.  Eli?  Stefan?  Any objection?

Great!  I've CC'd Stefan as well in this email.

> In the mean time, could you update the docs to clarify the behavior.
> E.g. currently the docstring of `read-file-name` says:
>
>     - a function, which will be called with the input as the
>       argument.  [...]
>
> which could be understood to suggest it really receives the contents of
> the minibuffer (i.e. the quoted file name) rather than the value we'd
> extract from it (the unquoted file name).

Please see the attached patch.

Thanks!

Joseph


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Pass-unquoted-filename-to-user-supplied-MUSTMATCH-pr.patch --]
[-- Type: text/x-diff, Size: 2753 bytes --]

From 208c7abff97f842057540fa2dfacd99808f17015 Mon Sep 17 00:00:00 2001
From: Joseph Turner <joseph@breatheoutbreathe.in>
Date: Tue, 30 Jan 2024 22:08:50 -0800
Subject: [PATCH] Pass unquoted filename to user-supplied MUSTMATCH predicate

* lisp/minibuffer.el (read-file-name-default): Pass REQUIRE-MATCH
argument through substitute-in-file-name.
* lisp/minibuffer.el (read-file-name): Update docstring.

Resolves bug#68815.
---
 lisp/minibuffer.el | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index faa7f543ece..a9e3ec937f9 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3262,9 +3262,10 @@ Fourth arg MUSTMATCH can take the following values:
   input, but she needs to confirm her choice if she called
   `minibuffer-complete' right before `minibuffer-complete-and-exit'
   and the input is not an existing file.
-- 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.
+- a function, which will be called with a single argument, the
+  input unquoted by `substitute-in-file-name', which see.  If the
+  function returns a non-nil value, the minibuffer is exited with
+  that argument as the value.
 - anything else behaves like t except that typing RET does not exit if it
   does non-null completion.
 
@@ -3353,7 +3354,13 @@ See `read-file-name' for the meaning of the arguments."
     (let ((ignore-case read-file-name-completion-ignore-case)
           (minibuffer-completing-file-name t)
           (pred (or predicate 'file-exists-p))
-          (add-to-history nil))
+          (add-to-history nil)
+          (require-match (if (functionp mustmatch)
+                             (lambda (input)
+                               (funcall mustmatch
+                                        ;; User-supplied MUSTMATCH expects an unquoted filename
+                                        (substitute-in-file-name input)))
+                           mustmatch)))
 
       (let* ((val
               (if (or (not (next-read-file-uses-dialog-p))
@@ -3389,7 +3396,7 @@ See `read-file-name' for the meaning of the arguments."
 				   (read-file-name--defaults dir initial))))
 			  (set-syntax-table minibuffer-local-filename-syntax))
                       (completing-read prompt 'read-file-name-internal
-                                       pred mustmatch insdef
+                                       pred require-match insdef
                                        'file-name-history default-filename)))
                 ;; If DEFAULT-FILENAME not supplied and DIR contains
                 ;; a file name, split it.
-- 
2.41.0