~pkal/public-inbox

Generic implementation of setup-ensure-spec v1 PROPOSED

Trey Peacock: 2
 Generic implementation of setup-ensure-spec
 Generic implementation of setup-ensure-spec

 2 files changed, 61 insertions(+), 34 deletions(-)
"Philip Kaludercic" <philipk@posteo.net> writes:
Next
--
Trey Peacock
treypeacock.com
Next
Trey Peacock <git@treypeacock.com> writes:
Next
Trey Peacock <git@treypeacock.com> writes:
Next
ping?

Philip Kaludercic <philipk@posteo.net> writes:
Export patchset (mbox)
How do I use this?

Copy & paste the following snippet into your terminal to import this patchset into git:

curl -s https://lists.sr.ht/~pkal/public-inbox/patches/39748/mbox | git am -3
Learn more about email & git

[PATCH] Generic implementation of setup-ensure-spec Export this patch

I currently use a modified versions of the keybinding macros that use
`keymap-set` and other functions found in `keymap.el`. This resulted in
an error thrown when trying to use the new `kbd` ensure keyword. "C-="
can be set with `keymap-set`, but it will result an error in
`setup--ensure` with `(kbd "C-=")`.

So, I figured it may be useful to allow for custom ENSURE-SPECs. The
implementation below uses `cl-defgeneric` to allow for custom methods;
however, if you do not want to rely on the `cl-lib` library, I have a
patch that uses an alist variable instead. This seemed a bit cleaner to
me.

For example, this is the spec that I use for by custom `:bind` macros:

(cl-defmethod setup-ensure-spec ((spec (eql key-valid)) arg)
  (if (key-valid-p arg)
      arg
    (setup-ensure-spec 'kbd arg)))

Do you think this is something you would be interested in?

---
 setup.el | 48 +++++++++++++++++++++++++++++++-----------------
 1 file changed, 31 insertions(+), 17 deletions(-)

diff --git a/setup.el b/setup.el
index 4574487..1d4f1de 100644
--- a/setup.el
+++ b/setup.el
@@ -43,6 +43,7 @@

;;; Code:

(require 'cl)
(require 'elisp-mode)

(defvar setup-opts `((quit . ,(make-symbol "setup-quit")))
@@ -144,11 +145,35 @@ (defmacro setup (name &rest body)
;;;###autoload
(put 'setup 'function-documentation '(setup--make-docstring))

(cl-defgeneric setup-ensure-spec (ensure-spec arg)
  "Ensure that ARGS matches the form of ENSURE-SPEC.
Return nil by default."
  nil)

(cl-defmethod setup-ensure-spec ((_ (eql nil)) arg)
  arg)

(cl-defmethod setup-ensure-spec ((_ (eql kbd)) arg)
  "Ensure ARG is valid `kbd' syntax, applying `kbd' if necessary."
  (cond
   ((stringp arg) (kbd arg))
   ((symbolp arg) `(kbd ,arg))
   (arg)))

(cl-defmethod setup-ensure-spec ((_ (eql func)) arg)
  "Ensure ARG is a function, sharp-quoted if necessary."
  (cond
   ((eq (car-safe arg) 'function)
    arg)
   ((eq (car-safe arg) 'quote)
    `#',(cadr arg))
   ((symbolp arg)
    `#',arg)
   (arg)))

(defun setup--ensure (ensure-spec args &optional check-len)
  "Ensure that ARGS matches the form of ENSURE-SPEC.

The symbol `kbd' means to apply the function `kbd' to the
argument.  The symbol `func' means to sharp-quote the argument.
The symbol `&rest' means that the remaining elements of
ENSURE-SPEC are applied repeatedly to the remaining elements of
ARGS.
@@ -188,21 +213,10 @@ (defun setup--ensure (ensure-spec args &optional check-len)
                                        nil))
                  (push ensured result))))
          (let ((arg (pop args)))
            (push (cond ((null ensure) arg) ;Do not modify argument
                        ((eq ensure 'kbd) (cond
                                           ((stringp arg) (kbd arg))
                                           ((symbolp arg) `(kbd ,arg))
                                           (arg)))
                        ((eq ensure 'func) (cond
                                            ((eq (car-safe arg) 'function)
                                             arg)
                                            ((eq (car-safe arg) 'quote)
                                             `#',(cadr arg))
                                            ((symbolp arg)
                                             `#',arg)
                                            (arg)))
                        ((error "Invalid ensure spec %S" ensure)))
                  result)))))
            (push
             (or (setup-ensure-spec ensure arg)
                 (error "Invalid ensure spec %S" ensure))
             result)))))
    (nreverse result)))

(defun setup-define (name fn &rest opts)
--
2.40.0
Hi,

Trey Peacock <git@treypeacock.com> writes:

Re: [PATCH] Generic implementation of setup-ensure-spec Export this patch

"Philip Kaludercic" <philipk@posteo.net> writes:

> I get the impression that cl-defmacro is too great of an overhead here
> -- especially when we consider loading the necessary files during
> initialisation.  `Setup' is relatively fast, and if there is a more
> efficient/lightweight way then I think it would be preferable.
> Dispatching ensure-specs could also be done using an alist mapping
> symbols to functions.

That makes sense. There should be a new patch attached below. I'm not
extremely familiar with the email patch workflow, so I hope that is okay
> That being said, I wonder if this is really so
> much potential for extending `setup--ensure' or if simply extending it
> from time to time is the good enough of a solution?

If `setup--ensure' were to be extended, would you need to require the
`keymap.el' library in this case?
My thoughts if you allow for user customization:
- Remove any future responsibility of adding to `setup--ensure'
- Additonally you could continue to add examples to the Wiki
for popular/commonly used specs.
It would also be quite useful for a few of the `package-vc' macros that
I'm currently working on.
> Especially when we keep in mind that for infrequent objects it is often
> just easier to preform the check yourself in the local macro instead of
> using an ensure-spec.

To your point, I guess it does not matter whether the logic sits in
`setup.el' or the macro itself. I just saw the ability added recently
and thought it would be useful to extend it.

Also, re: the patch. I wasn't sure if `setup-ensure-alist' should be a
`defcustom' or `defvar'.
---
diff --git a/setup.el b/setup.el
index 4574487..5698e5a 100644
--- a/setup.el
+++ b/setup.el
@@ -144,11 +144,32 @@ (defmacro setup (name &rest body)
;;;###autoload
(put 'setup 'function-documentation '(setup--make-docstring))

(defcustom setup-ensure-alist '((kbd . setup--ensure-kbd)
                                (func . setup--ensure-func)
                                (nil . identity))
  "Associative list assigning spec symbol to function.")

(defun setup--ensure-kbd (arg)
  "Ensure ARG is valid `kbd' syntax, applying `kbd' if necessary."
  (cond
   ((stringp arg) (kbd arg))
   ((symbolp arg) `(kbd ,arg))
   (arg)))

(defun setup--ensure-func (arg)
  "Ensure ARG is a function, sharp-quoted if necessary."
  (cond
   ((eq (car-safe arg) 'function)
    arg)
   ((eq (car-safe arg) 'quote)
    `#',(cadr arg))
   ((symbolp arg)
    `#',arg)
   (arg)))

(defun setup--ensure (ensure-spec args &optional check-len)
  "Ensure that ARGS matches the form of ENSURE-SPEC.

The symbol `kbd' means to apply the function `kbd' to the
argument.  The symbol `func' means to sharp-quote the argument.
The symbol `&rest' means that the remaining elements of
ENSURE-SPEC are applied repeatedly to the remaining elements of
ARGS.
@@ -188,21 +209,13 @@ (defun setup--ensure (ensure-spec args &optional check-len)
                                        nil))
                  (push ensured result))))
          (let ((arg (pop args)))
            (push (cond ((null ensure) arg) ;Do not modify argument
                        ((eq ensure 'kbd) (cond
                                           ((stringp arg) (kbd arg))
                                           ((symbolp arg) `(kbd ,arg))
                                           (arg)))
                        ((eq ensure 'func) (cond
                                            ((eq (car-safe arg) 'function)
                                             arg)
                                            ((eq (car-safe arg) 'quote)
                                             `#',(cadr arg))
                                            ((symbolp arg)
                                             `#',arg)
                                            (arg)))
                        ((error "Invalid ensure spec %S" ensure)))
                  result)))))
            (push
             (let ((spec
                    (alist-get ensure setup-ensure-alist)))
               (if (functionp spec)
                   (funcall spec arg)
                 (error "Invalid ensure spec %S" ensure)))
             result)))))
    (nreverse result)))

(defun setup-define (name fn &rest opts)




Trey Peacock <git@treypeacock.com> writes: