Implementing :validate-custom using validate.el (feedback please)

[xenodium]

Goal

To implement :validate-custom keyword (same as :custom, but backed by validate.el validation).

I seem to have an initial implementation. I could really use some feedback/guidance. Spot flaws? Potential improvements?

Background

I'm a fan of validate.el. It enforces schema validation when setting custom variables, automatically surfacing incompatibilities (often overlooked), specially when upgrading packages.

Great posts about validate.el by @manuel-uberti and @Malabarba

https://manuel-uberti.github.io/emacs/2016/09/17/validate https://endlessparentheses.com/validate-el-schema-validation-for-emacs-lisp.html

I'm also a fan of use-package's :custom keyword, enabling declarative package configurations.

I'd like to combine :custom-style configuration with validate.el validation. Read on for potential implementation.

Usage

(use-package magit
  :ensure t
  :validate-custom
  (magit-diff-refine-hunk 'all)
  (magit-status-margin '(t "%Y-%m-%d %H:%M " magit-log-margin-width t 18)))

Potential implementation

(use-package validate
  :ensure t
  :config
  (defmacro validate-customize-set-variable (variable value &optional comment)
    "Like `customize-set-variable', but throw an error if validation fails.
VALUE is validated against SYMBOL's custom type.

\(fn [SYM VAL] ...)"
    `(if (boundp ,variable)
         (customize-set-variable ,variable (validate-value ,value (custom-variable-type ,variable)) ,comment)
       (user-error "Trying to validate a variable that's not defined yet: `%s'.\nYou need to require the package before validating" ,variable)))

  (setq use-package-keywords
        (use-package-list-insert :validate-custom
                                 use-package-keywords
                                 :load t))

  (defun use-package-normalize/:validate-custom (_name keyword args)
    "Normalize use-package validate-custom keyword."
    (use-package-as-one (symbol-name keyword) args
      #'(lambda (label arg)
          (unless (listp arg)
            (use-package-error
             (concat label " a (<symbol> <value> [comment])"
                     " or list of these")))
          (if (use-package-non-nil-symbolp (car arg))
              (list arg)
            arg))))

  (defun use-package-handler/:validate-custom (name _keyword args rest state)
    "Generate use-package validate-custom keyword code."
    (use-package-concat
     (mapcar
      #'(lambda (def)
          (let ((variable (nth 0 def))
                (value (nth 1 def))
                (comment (nth 2 def)))
            (unless (and comment (stringp comment))
              (setq comment (format "Customized with use-package %s" name)))
            `(validate-customize-set-variable (quote ,variable) ,value ,comment)))
      args)
     (use-package-process-keywords name rest state))))

[a13]

What a nice idea!

IMO it is not necessary to add an additional keyword here, all we need is a defcustom with a name like use-package-custom-use-validate or smth like that, to let people choose

[xenodium]

Thanks for this.

defcustom with a name like use-package-custom-use-validate

I've run into the odd case where validate.el is unable validate the value I'm setting. For those instances, I use setq (instead of validate-setq). Would be nice to offer :validate-custom and :custom for equivalent flexibility.

For example:

(use-package power-ranger
  :validate-custom
  (happy-foo t)
  (happy-bar "yay")
  :custom
  (grumpy-foo t)
  (grumpy-bar "boo"))

[a13]

I've run into the odd case where validate.el is unable validate the value I'm setting.

looks like an issue with validate.el for me

[Malabarba]

I've run into the odd case where validate.el is unable validate the value I'm setting.

looks like an issue with validate.el for me

Feel free to report any issues, but it can also be a case of the defcustom having a "bad" definition in the package or the package code supports custom values not specified in the actual defcustom (both are surprisingly common).

[xenodium]

Reviving PR/thread in case there's still interest to get this merged into use-package. I've been using :validate-custom in my config for a couple of years now.

it can also be a case of the defcustom having a "bad" definition in the package or the package code supports custom values not specified in the actual defcustom (both are surprisingly common)

These are the problematic scenarios I've typically run into. In these cases, I use :custom. For all others with valid definitions, I use :validate-custom. IMO, this is the benefit if keeping :custom and :validate-custom separate.

[skangas]

@xenodium Perhaps this would work better as a stand-alone GNU ELPA package? We are currently working on getting use-package merged into Emacs core, and I don't think it makes sense to add support for a rather niche third-party library there, even if it is useful.

What do you think?

[xenodium]

The addition has helped maintain some correctness in my config, often flagging breaks that could have gone unseen after upgrading packages. I’m happy to take your guidance or preference on inclusion.

[skangas]

I see your point, but I still think that we either should add something like validate.el to core Emacs, or this should be a separate GNU ELPA package. I haven't looked at validate.el so I can't really give an opinion on which of these are best.

If you say that it's useful, then I'm sure you're right. The question is only how useful and to which proportion of users.

@jwiegley Do you have an opinion about this?

[skangas]

BTW, starting this off as a GNU ELPA package would in any case be a good step forward. Would you be interested in making such a package, perhaps?

[skangas]

Another idea, just to throw it out there: Why not make this part of the validate package itself?

[xenodium]

I'm happy with either of the options. Feel free to close this issue and I can re-open elsewhere or create a new package.

[skangas]

Sounds good, thanks. Closing.