unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
@ 2017-02-10 18:32 Drew Adams
  2017-02-17 23:57 ` npostavs
  2020-08-29 15:33 ` Mauro Aranda
  0 siblings, 2 replies; 9+ messages in thread
From: Drew Adams @ 2017-02-10 18:32 UTC (permalink / raw)
  To: 25678

(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''





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2017-02-10 18:32 bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work Drew Adams
@ 2017-02-17 23:57 ` npostavs
  2018-01-14 23:07   ` Tim Landscheidt
  2020-08-29 15:33 ` Mauro Aranda
  1 sibling, 1 reply; 9+ messages in thread
From: npostavs @ 2017-02-17 23:57 UTC (permalink / raw)
  To: Drew Adams; +Cc: 25678

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.





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2017-02-17 23:57 ` npostavs
@ 2018-01-14 23:07   ` Tim Landscheidt
  2020-03-21 17:36     ` H. Dieter Wilhelm
  0 siblings, 1 reply; 9+ messages in thread
From: Tim Landscheidt @ 2018-01-14 23:07 UTC (permalink / raw)
  To: npostavs; +Cc: 25678

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2018-01-14 23:07   ` Tim Landscheidt
@ 2020-03-21 17:36     ` H. Dieter Wilhelm
  0 siblings, 0 replies; 9+ messages in thread
From: H. Dieter Wilhelm @ 2020-03-21 17:36 UTC (permalink / raw)
  To: Tim Landscheidt; +Cc: 25678, npostavs

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2017-02-10 18:32 bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work Drew Adams
  2017-02-17 23:57 ` npostavs
@ 2020-08-29 15:33 ` Mauro Aranda
  2020-08-30 13:56   ` Lars Ingebrigtsen
  1 sibling, 1 reply; 9+ messages in thread
From: Mauro Aranda @ 2020-08-29 15:33 UTC (permalink / raw)
  To: 25678; +Cc: dieter, Noam Postavsky, tim


[-- 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2020-08-29 15:33 ` Mauro Aranda
@ 2020-08-30 13:56   ` Lars Ingebrigtsen
  2020-08-30 15:22     ` Mauro Aranda
  2020-09-09 17:21     ` Mauro Aranda
  0 siblings, 2 replies; 9+ messages in thread
From: Lars Ingebrigtsen @ 2020-08-30 13:56 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: dieter, 25678, Noam Postavsky, tim

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2020-08-30 13:56   ` Lars Ingebrigtsen
@ 2020-08-30 15:22     ` Mauro Aranda
  2020-09-09 17:21     ` Mauro Aranda
  1 sibling, 0 replies; 9+ messages in thread
From: Mauro Aranda @ 2020-08-30 15:22 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: dieter, 25678, Noam Postavsky, tim

[-- 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 --]

^ permalink raw reply	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2020-08-30 13:56   ` Lars Ingebrigtsen
  2020-08-30 15:22     ` Mauro Aranda
@ 2020-09-09 17:21     ` Mauro Aranda
  2020-09-10 13:13       ` Lars Ingebrigtsen
  1 sibling, 1 reply; 9+ messages in thread
From: Mauro Aranda @ 2020-09-09 17:21 UTC (permalink / raw)
  To: Lars Ingebrigtsen; +Cc: dieter, 25678, Noam Postavsky, tim


[-- 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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work
  2020-09-09 17:21     ` Mauro Aranda
@ 2020-09-10 13:13       ` Lars Ingebrigtsen
  0 siblings, 0 replies; 9+ messages in thread
From: Lars Ingebrigtsen @ 2020-09-10 13:13 UTC (permalink / raw)
  To: Mauro Aranda; +Cc: dieter, 25678, Noam Postavsky, tim

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





^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-09-10 13:13 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-10 18:32 bug#25678: 25.1; defcustom type `(file :must-match t)' seems not to work Drew Adams
2017-02-17 23:57 ` npostavs
2018-01-14 23:07   ` Tim Landscheidt
2020-03-21 17:36     ` H. Dieter Wilhelm
2020-08-29 15:33 ` Mauro Aranda
2020-08-30 13:56   ` Lars Ingebrigtsen
2020-08-30 15:22     ` Mauro Aranda
2020-09-09 17:21     ` Mauro Aranda
2020-09-10 13:13       ` Lars Ingebrigtsen

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).