bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Similar to `setopt' introduced in 29, which warns when a variable gets
assigned a value with an incorrect type, I hope `defcustom' can
(optionally) check that the standard value of a customizable variable is
correct.  This would help external as well as internal package authors
to catch errors on the types.

I don't know how to define such optionality though -- can/should it also
be a customizable variable?  Or maybe it should just be a
function/command which checks customizable variables (e.g., a single
variable; a group; all of them; etc.), and warns about all the type
mismatches along the way?

In addition, a "check all variables within this group (and
subgroups)" might be helpful as well.

At the moment, the only via workaround I know of to check existing value
type for any given variable is this:

    (setopt variable variable)

But this introduces the runtime cost of the :set function, which
sometimes do pretty expensive things like (re-) loading a package.

Another workaround I know of, of course, is to run

    M-x customize-variable

on the variable, but that is painfully slow compared to the first
workaround.

If no-one gets to it first, I may start looking into how to implement it
within a week or two.

-- 
Best,


RY

bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

[Drew Adams]

> Similar to `setopt' introduced in 29, which warns when a variable gets
> assigned a value with an incorrect type, I hope `defcustom' can
> (optionally) check that the standard value of a customizable variable is
> correct.  This would help external as well as internal package authors
> to catch errors on the types.

Not sure I understand.  Doesn't it do that already,
showing "mismatch" next to the default value if it
doesn't match the type definition?

bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

>> Similar to `setopt' introduced in 29, which warns when a variable gets
>> assigned a value with an incorrect type, I hope `defcustom' can
>> (optionally) check that the standard value of a customizable variable is
>> correct.  This would help external as well as internal package authors
>> to catch errors on the types.
>
> Not sure I understand.  Doesn't it do that already,
> showing "mismatch" next to the default value if it
> doesn't match the type definition?

Yes, it does, but I think it is not "automated" enough, since you would
have to go over the variables one at a time.

My request is that Emacs should (optionally, maybe behind a defcustom,
or configure, or cli flag, etc.) warn you warn a defcustom's standard
value does not match its declared type.  Since a similar warning is
already in place for `setopt', I don't think it is farfetched to request
`defcustom' to follow suit.

Alternatively, or in addition to the above, maybe provide a
function/command that checks types for a group+subgroups, or for all
defined customizable variables.  Again, this would help package authors
to catch typing errors which would otherwise be quite difficult to
catch.

As I mentioned upthread, using `setopt' directly like the following
snippet is unnecessarily expensive: it calls the :set function of a
customizable variable, where the only thing we want is to check its
type.

```emacs-lisp
(defcustom expensive-var t
  :type 'string
  :set (lambda (n v)
         (dotimes (_ 424242) 'expensive)
         (set n v)))
(setopt expensive-var expensive-var)
;; warns the user that the type is correct, but only after spending a
;; long time in the :set function
```

-----

Let me also explain the reason behind this request.  I have encountered
this scenario multiple times, where when I `setopt' something for a
customizable variable according to its docs, I get a warning saying that
the type is incorrect.  Examples of this scenario include eshell, mu4e,
and pyim.

Seeing this type of warning would prompt me to examine the entire
package in which the warning occurs.  I would have to rgrep the entire
package, and examine the types of each defcustom one-by-one, fixing all
type errors along the way.  When I encounter difficult-to-parse types I
would do the `(setopt var var)' method to save time and brain cells.

And here comes my feature request.  We have the warning in `setopt'; we
have some visual indication in the customization view; and I want to
have one more: to know when a defcustom call defines a wrongly-typed
standard value.

-- 
Best,


RY

bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

[Eli Zaretskii]

> Cc: "63410@debbugs.gnu.org" <63410@debbugs.gnu.org>
> Date: Wed, 10 May 2023 22:51:14 +0800
> From:  Ruijie Yu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> My request is that Emacs should (optionally, maybe behind a defcustom,
> or configure, or cli flag, etc.) warn you warn a defcustom's standard
> value does not match its declared type.

Warn you at what time?  When defcustom is byte-compiled? when it is
evaluated at run time? when the user says "M-x set-variable"?
something else?

> Since a similar warning is already in place for `setopt', I don't
> think it is farfetched to request `defcustom' to follow suit.

setopt says it when you invoke it.  But setopt is designed to be
called as a function at run time, whereas defcustom isn't.

bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: "63410@debbugs.gnu.org" <63410@debbugs.gnu.org>
>> Date: Wed, 10 May 2023 22:51:14 +0800
>> From:  Ruijie Yu via "Bug reports for GNU Emacs,
>>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
>> 
>> My request is that Emacs should (optionally, maybe behind a defcustom,
>> or configure, or cli flag, etc.) warn you warn a defcustom's standard
>> value does not match its declared type.
>
> Warn you at what time?  When defcustom is byte-compiled? when it is
> evaluated at run time? when the user says "M-x set-variable"?
> something else?

I was initially thinking of at runtime, but now that you mention it,
byte-compilation time sounds more reasonable.

So, I would say check the type unconditionally on byte-compilation time,
or when something is toggled on, check the type at runtime if a
defcustom is not compiled.

>> Since a similar warning is already in place for `setopt', I don't
>> think it is farfetched to request `defcustom' to follow suit.
>
> setopt says it when you invoke it.  But setopt is designed to be
> called as a function at run time, whereas defcustom isn't.

In that case, I had my semi-parallel second proposal: provide a command
that checks for a group of variables and see if types of all variables
within the group (or its subgroups) are correct.

-- 
Best,


RY

bug#63410: 30.0.50; [FR] Optionally allow defcustom to check type for standard value

[Ruijie Yu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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


Ruijie Yu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org> writes:

> In that case, I had my semi-parallel second proposal: provide a command
> that checks for a group of variables and see if types of all variables
> within the group (or its subgroups) are correct.

Regarding the second proposal: I have quickly hacked together a
functional example of type checking, see the attachment.  If we think
this should be integrated into Emacs, I can make some minor edits and
propose a patch.  In that case, I would probably put the functions
somewhere in cus-edit.el or custom.el.

Note that at the moment it depends on the internal function
`custom--standard-value', which is a one-line function accessing a
property of the symbol.


[-- Attachment #2: cfg-check-defcustom.el --]
[-- Type: text/plain, Size: 1827 bytes --]

;;; cfg-check-defcustom.el --- Ensure type-strictness of defcustom

;;; Commentary:

;;; Code:
(autoload 'customize-read-group "cus-edit")
(autoload 'custom-variable-prompt "cus-edit")

(defun ccd (variable)
  "Check the standard value of VARIABLE is of the correct type."
  (interactive (custom-variable-prompt))
  (custom-load-symbol variable)
  (when-let ((type (get variable 'custom-type)))
    (let ((value (custom--standard-value variable)))
      (unless (widget-apply (widget-convert type) :match value)
        (warn "Type mismatch on `%s': value `%S'; type %s"
              variable value type)))))

(defun ccd--vars-in-group (group &optional recursive)
  "Return a list of all variable symbols contained in GROUP.
When RECURSIVE is non-nil, the list contains variables in
subgroups recursively."
  (when-let (((symbolp group))
             (gp (get group 'custom-group)))
    (let* ((filter
            (lambda (s)
              (mapcar
               #'car (seq-filter (lambda (c) (eq s (nth 1 c))) gp))))
           (child (funcall filter 'custom-variable))
           (subgroup (and recursive
                          (funcall filter 'custom-group))))
      (nconc child (mapcan #'ccd--vars-in-group-r subgroup)))))

(defun ccd--vars-in-group-r (group)
  "Shorthand for \\(cfg-check-defcustom--vars-in-group GROUP t)."
  (ccd--vars-in-group group t))

(defun ccd-group (group &optional recursive)
  "Check all variables within GROUP.
When RECURSIVE is non-nil, check the group recursively.  See also
`cfg-check-defcustom'."
  (interactive (list (customize-read-group) current-prefix-arg))
  (mapc #'ccd (ccd--vars-in-group (intern group) recursive)))

(provide 'cfg-check-defcustom)
;;; cfg-check-defcustom.el ends here.

;; Local Variables:
;; read-symbol-shorthands: (("ccd" . "cfg-check-defcustom"))
;; End:

[-- Attachment #3: Type: text/plain, Size: 303 bytes --]


Also, note that I added the symbol into the warning text because the
goal of this command is to mass-check an entire group, whereas `setopt'
did not mention the variable symbol name.  Not sure if `setopt' should
show the symbol in trouble, but I understand if we don't want to do
that.

-- 
Best,


RY