bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Drew Adams]

(defcustom foo-file ""
  ""
  :type '(file :must-match t) :group 'convenience)

M-x customize-option foo-file

I don't see anything that prevents you from using a file name that
corresponds to no existing file.  I don't see that completion (`M-TAB') 
enforces it to name an existing file.  And I don't see that simply 
inserting a bad file name is prevented, nor is setting the option to a
bad file name or saving it prevented.

What effect does (file :must-match t) actually have, if any?

If this is just confusion on my part, please consider clarifying the
description of the behavior in the doc: (elisp) `Simple Types'.

(I see the same behavior if `(file :must-match t)' is used within a
complex type, e.g. as the :value-type of an alist entry.)


In GNU Emacs 25.1.1 (x86_64-w64-mingw32)
 of 2016-11-15
Windowing system distributor `Microsoft Corp.', version 6.1.7601
Configured using:
 `configure --without-dbus --without-compress-install 'CFLAGS=-O2
 -static -g3''

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[npostavs]

Drew Adams <drew.adams@oracle.com> writes:

> (defcustom foo-file ""
>   ""
>   :type '(file :must-match t) :group 'convenience)
>
> M-x customize-option foo-file
>
> I don't see anything that prevents you from using a file name that
> corresponds to no existing file.  I don't see that completion (`M-TAB') 
> enforces it to name an existing file.  And I don't see that simply 
> inserting a bad file name is prevented, nor is setting the option to a
> bad file name or saving it prevented.
>
> What effect does (file :must-match t) actually have, if any?

It looks like `widget-file-prompt-value' consults it, but I can't tell
when this function is called.

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Tim Landscheidt]

AFAICT, this setting is only obeyed when executing M-x
customize-set-value RET foo-file RET.

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[H. Dieter Wilhelm]

Tim Landscheidt <tim@tim-landscheidt.de> writes:

> AFAICT, this setting is only obeyed when executing M-x
> customize-set-value RET foo-file RET.

Right, customize-set-value is working but Easy Customizations claims:

(info (emacs) Changing a Variable)

     [State]: SET for current session only.

 You don’t have to worry about specifying a value that is not valid; the
 ‘Set for Current Session’ operation checks for validity and will not
 install an unacceptable value.

Which is clearly not the case in Emacs-26.3

Thanky you

   Dieter

-- 
Best wishes
H. Dieter Wilhelm
Zwingenberg, Germany

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 115 bytes --]

I think it makes sense to expand the impact of :must-match t to
Customization buffers, like in the attached patch.

[-- Attachment #1.2: Type: text/html, Size: 139 bytes --]

[-- Attachment #2: 0001-Respect-must-match-for-file-types-in-customization-b.patch --]
[-- Type: text/x-patch, Size: 1361 bytes --]

From 10896dedfc6861dcc64dd8705818dacc6aaa0a98 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Wed, 26 Aug 2020 12:49:58 -0300
Subject: [PATCH] Respect :must-match for file types in customization buffers

* lisp/wid-edit.el (file widget): Add a :match and a :validate
  function to the 'file widget, to be able to check if the widget
  value is an existent file, when required.  (Bug#25678)
---
 lisp/wid-edit.el | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index ea7e266e0d..1015f010ce 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -3161,6 +3161,15 @@ 'file
   :completions (completion-table-case-fold
                 #'completion-file-name-table
                 (not read-file-name-completion-ignore-case))
+  :match #'(lambda (widget value)
+             (or (not (widget-get widget :must-match))
+                 (file-exists-p value)))
+  :validate #'(lambda (widget)
+                (let ((value (widget-value widget)))
+                  (unless (widget-apply widget :match value)
+                    (widget-put widget
+                                :error (format "File %s does not exist" value))
+                    widget)))
   :prompt-value 'widget-file-prompt-value
   :format "%{%t%}: %v"
   ;; Doesn't work well with terminating newline.
-- 
2.28.0

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I think it makes sense to expand the impact of :must-match t to
> Customization buffers, like in the attached patch.

[...]

> +  :match #'(lambda (widget value)
> +             (or (not (widget-get widget :must-match))
> +                 (file-exists-p value)))

Makes sense to me.  I've now applied this to Emacs 28 (but I removed the
#''s before the lambdas first (they're superfluous)).

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Mauro Aranda]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I think it makes sense to expand the impact of :must-match t to
>> Customization buffers, like in the attached patch.
>
> [...]
>
>> +  :match #'(lambda (widget value)
>> +             (or (not (widget-get widget :must-match))
>> +                 (file-exists-p value)))
>
> Makes sense to me.  I've now applied this to Emacs 28 (but I removed the
> #''s before the lambdas first (they're superfluous)).

Noted.  Thanks for taking the trouble!

[-- Attachment #2: Type: text/html, Size: 795 bytes --]

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Mauro Aranda]

[-- Attachment #1.1: Type: text/plain, Size: 805 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Mauro Aranda <maurooaranda@gmail.com> writes:
>
>> I think it makes sense to expand the impact of :must-match t to
>> Customization buffers, like in the attached patch.
>
> [...]
>
>> +  :match #'(lambda (widget value)
>> +             (or (not (widget-get widget :must-match))
>> +                 (file-exists-p value)))
>
> Makes sense to me.  I've now applied this to Emacs 28 (but I removed the
> #''s before the lambdas first (they're superfluous)).

I found that the :match function will return false positives when
:must-match is nil and value is not a string.  This can happen is
the defcustom is of type:
:type '(choice file (const nil))), and the value is nil, because we
check if nil matches with the widget file first.

Here's a patch to fix it.

[-- Attachment #1.2: Type: text/html, Size: 1096 bytes --]

[-- Attachment #2: 0001-Fix-match-function-for-the-file-widget.patch --]
[-- Type: text/x-patch, Size: 1064 bytes --]

From d6abe15da5ff39996ab4f5ebfbc3428e92017452 Mon Sep 17 00:00:00 2001
From: Mauro Aranda <maurooaranda@gmail.com>
Date: Wed, 9 Sep 2020 14:11:49 -0300
Subject: [PATCH] Fix :match function for the file widget

* lisp/wid-edit.el (file widget): Return nil if value is not a
  string.  (Bug#25678)
---
 lisp/wid-edit.el | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/lisp/wid-edit.el b/lisp/wid-edit.el
index e9799dc00f..bc2afc6a6f 100644
--- a/lisp/wid-edit.el
+++ b/lisp/wid-edit.el
@@ -3162,8 +3162,9 @@ 'file
                 #'completion-file-name-table
                 (not read-file-name-completion-ignore-case))
   :match (lambda (widget value)
-           (or (not (widget-get widget :must-match))
-               (file-exists-p value)))
+           (and (stringp value)
+                (or (not (widget-get widget :must-match))
+                    (file-exists-p value))))
   :validate (lambda (widget)
               (let ((value (widget-value widget)))
                 (unless (widget-apply widget :match value)
-- 
2.28.0

bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work

[Lars Ingebrigtsen]

Mauro Aranda <maurooaranda@gmail.com> writes:

> I found that the :match function will return false positives when
> :must-match is nil and value is not a string.  This can happen is
> the defcustom is of type:
> :type '(choice file (const nil))), and the value is nil, because we
> check if nil matches with the widget file first.
>
> Here's a patch to fix it.

Thanks; applied to the trunk.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no