emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [ELPA] A Setup package


From: Philip K.
Subject: Re: [ELPA] A Setup package
Date: Wed, 10 Feb 2021 00:42:39 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux)

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> Hi Philip,
>
> Philip K. [2021-02-04 12:43:11] wrote:
>> I would like to propose adding a package to ELPA:
>
> Have you already signed the copyright paperwork?
> I see a possible matching "Philip K..." in the list, but its email is at
> warpmail rather than posteo.  Is that you?

Yes, sorry about that.  I changed providers last year, so it would be
better if that could be updated.

>> ;;; setup.el --- Helpful Configuration Macro    -*- lexical-binding: t -*-
>>
>> ;; Author: Philip K. <philipk@posteo.net>
>> ;; Maintainer: Philip K. <philipk@posteo.net>
>> ;; Version: 0.1.0
>> ;; Package-Requires: ((emacs "25.1"))
>> ;; Keywords: lisp, local
>
> Is this file available in a Git repository somewhere?

I created one yesterday: https://git.sr.ht/~zge/setup. I'll push the
improvements to this repository, and notify this thread when I am done.

>> ;;; Commentary:
>>
>> ;; The `setup' macro simplifies repetitive configuration patterns.
>> ;; For example, these macros:
>>
>> ;;     (setup shell
>> ;;       (let ((key "C-c s"))
>> ;;         (global (key shell))
>> ;;         (bind (key bury-buffer))))
>> ;;
>> ;;
>> ;;     (setup (package paredit)
>> ;;       (hide-lighter)
>> ;;       (hook-into scheme-mode lisp-mode))
>
> Ah, so it's to `use-package` a bit like `iterate` is to `loop`?
> I like that.
>
>> (eval-when-compile (require 'cl-macs))
>
> Please use (require 'cl-lib) rather than (require 'cl-macs) because the
> division of labor between those files is an internal detail.

Ok.

>> (defun setup-after-load (body)
>>   "Wrap BODY in a `with-eval-after-load'."
>>   ``(with-eval-after-load setup-name ,,body))
>
> Hmm... that's a fairly subtle semantics.

I'm not proud of it, but I haven't found a better solution yet.

>> ;;;###autoload
>> (defmacro setup (&rest body)
>>   "Configure a feature or subsystem.
>> BODY may contain special forms defined by `setup-define', but
>> will otherwise just be evaluated as is."
>>   (declare (indent defun))
>>   (let* ((name (if (and (listp (car body))
>>                         (get (caar body) 'setup-get-name))
>>                    (funcall (get (caar body) 'setup-get-name)
>>                             (car body))
>>                  (car body)))
>
> Hmm.. why do you use (&rest body) instead of (name &rest body)?
>
> AFAICT this `setup-get-name` is so that you can say
>
>     (setup (require shell)
>       ...)
>
> which like (setup shell ...) except that it `require`s shell eagerly.
> And similarly
>
>     (setup (package ivy)
>       ...)
>
> will try and install `ivy` if it's not installed yet.

Yes, that is the idea, though it wanted, the signature might just as
well be (name &rest body).

> Is it worth this complexity?  It seems you could get similar result
> without this trick using a syntax like:
>
>     (setup ivy
>      (:get-package)
>      (:require)
>      ...)
>
> which would save you this `setup-get-name` here and the corresponding
> `:name` in `setup-define`.

I don't know how to say if it is worth the complexity or not. I like it,
because it is more compact, but I might be biased.

>> ;;;###autoload
>> (defun setup-define (name fn &rest opts)
>>   "Define `setup'-local macro NAME using function FN.
>> The plist OPTS may contain the key-value pairs:
>>
>>   :name
>> Specify a function to use, for extracting the feature name of a
>> NAME entry, if it is the first element in a setup macro.
>>
>>   :indent
>> Set the symbol property `lisp-indent-function' for NAME.
>>
>>   :wrap
>> Specify a function used to wrap the results of a NAME entry.
>>
>>   :sig
>> Give an advertised calling convention.
>>
>>   :doc
>> A documentation string."
>
> Here are some of the problem I see:
> - You should add support for Edebug!

I have only ever used Edebug, never added support for it. I'll look into
this.

> - `:indent` will set the `lisp-indent-function` property on the symbol,
>   making it apply globally rather than only within `setup`.
>   [ The same problem will plague the `:sig` and the Edebug support, BTW.  ]
>   This is the reason why in `define-inline` I decided to prefix all the
>   local macros with `inline-` :-(
>   I don't have a really good solution for that (clearly, we'd like to
>   allow `lisp-indent-function` (and Edebug's equivalent) to take some
>   context into account, but that's not currently available.
>   Patches welcome).
>   But maybe to reduce the problem, you could use local macros with names
>   that start with a `:`, as in:
>
>             (setup shell
>               (let ((key "C-c s"))
>                 (:global (key shell))
>                 (:bind (key bury-buffer))))

This was a problem I was thinking about, but couldn't solve. The idea to
use keywords might work though.

> - I'm not sure the generality of `:wrap` is worthwhile, since it seems
>   it's only ever used for eval-after-load.

It might be worth dropping for a first version, and replacing it with a
:after-loaded keyword.

>>   ;; define macro for `cl-macrolet'
>>   (push (let ((arity (func-arity fn)))
>>           (cl-flet ((wrap (result)
>>                           (if (plist-get opts :wrap)
>>                               (funcall (plist-get opts :wrap) result)
>>                             result)))
>>             (cond ((eq (cdr arity) 'many)
>>                    `(,name (&rest args) ,(wrap `(apply #',fn args))))
>>                   ((eq (cdr arity) 0)
>>                    `(,name () ,(wrap `(funcall #',fn))))
>>                   ((= (car arity) (cdr arity))
>>                    `(,name (&rest args)
>>                            (unless (zerop (mod (length args) ,(car arity)))
>>                              (error "Illegal arguments"))
>>                            (let ((aggr (list 'progn)))
>>                              (while args
>>                                (let ((rest (nthcdr ,(car arity) args)))
>>                                  (setf (nthcdr ,(car arity) args) nil)
>>                                  (push (apply #',fn args) aggr)
>>                                  (setq args rest)))
>>                              ,(wrap `(nreverse aggr)))))
>>                   ((error "Illegal function")))))
>>         setup-macros)
>
> IIUC this expands calls that have "more args" into repeated calls, right?
> I suggest you make that behavior optional via some `:repeatable` keyword
> argument, which will be an opportunity to document that feature.
> That will also make it possible to accept `fn` values that allow optional
> arguments.

Will try.

>> (defun setup-help ()
>>   "Generate and display a help buffer for the `setup' macro."
>>   (interactive)
>>   (with-help-window (help-buffer)
>>     (princ "The `setup' macro defines the following local macros:\n\n")
>>     (dolist (sym (mapcar #'car setup-macros))
>>       (let ((doc (or (get sym 'setup-documentation)
>>                      "No documentation."))
>>             (sig (if (get sym 'setup-signature)
>>                      (cons sym (get sym 'setup-signature))
>>                    (list sym))))
>>         (princ (format "- %s\n\n%s\n\n" sig doc))))))
>
> Take a look at the definition of `pcase` to see how you can merge the
> above right into the normal function's doc, so you don't need a separate
> command and `C-h o setup` will directly give you that info.

Will do, thank you for the pointer.

>> (setup-define 'with-mode
>>   (lambda (mode &rest body)
>>     `(let ((setup-mode ',mode)
>>            (setup-map ',(intern (format "%s-map" mode)))
>>            (setup-hook ',(intern (format "%s-hook" mode))))
>>        (ignore setup-mode setup-map setup-hook)
>>        ,@body))
>>   :sig '(MODE &body BODY)
>>   :doc "Change the MODE that BODY is configuring."
>>   :indent 1)
>
> It would be nice to use this macro in the `setup` macro, to reduce the
> code duplication between the two.

Can do.

>> (setup-define 'global
>>   (lambda (bind)
>>     (let ((key (car bind))
>>           (fn (cadr bind)))
>>       `(global-set-key ,(if (atom key) `(kbd ,key) key) #',fn)))
>
> `atom`?  I think you only want to use `kbd` is `key` is a string, and
> not if it's a vector, for example.

I had to take atom, because otherwise the example

             (setup shell
               (let ((key "C-c s"))
                 (global (key shell))
                 (bind (key bury-buffer))))

wouldn't work, as "key" is a symbol. The alternative would be

         (or (stringp key) (symbolp key))

that might be better, because I forgot that vectors are atoms (which
doesn't make sense, IMO).

> I find the syntax
>
>     (bind (key bury-buffer))
>
> a bit confusing from a Lisp point of view.  It would be nice to make it
> into a *function* (or at least use the syntax that corresponds to
> a function) so there's no confusion with a call to function `key`:
>
>     (bind key 'bury-buffer)
>
> An alternative would be to go the other way and use
>
>     (bind ((key bury-buffer))

I think the first alternative would be better, because it reminds me of
setq, but I get your point.

> BTW, I think it'd be good to allow `setup` to have not only local macros
> but also local functions (I think we should generally refrain from using
> macros when functions work about as well; it simplifies the overall
> system, leads to simpler semantics, and helps when debugging).

The idea behind using macros is that setup doesn't have to be loaded,
and can be byte-compiled away. Functions would either have to be
re-defined every time or aliased locally, which would require loading
setup. I'm not sure how I feel about this...

>> (setup-define 'hide-lighter
>>   (lambda ()
>>     `(delq (assq setup-mode minor-mode-alist)
>>            minor-mode-alist))
>>   :doc "Hide the mode-line lighter of the current mode."
>>   :body 'after-load)
>
> I don't see `:body` handled anywhere.

Thank your for the node, that was an old definition I forgot to update.

>> (setup-define 'local-set
>>   (lambda (assign)
>>     (let ((var (car assign))
>>           (val (cadr assign)))
>>       `(add-hook setup-hook (lambda () (setq-local ,var ,val)))))
>>   :sig '((VAR VAL) *)
>>   :doc "Set the value of VAR to VAL in buffers of the current mode."
>>   :wrap #'setup-after-load)
>
> Why after-load?

It is probably not necessary, but I'm not sure if there were some issues
with byte compilation if I didn't add it.

>         Stefan

-- 
        Philip K.

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]