unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [ELPA] A Setup package
@ 2021-02-04 11:43 Philip K.
  2021-02-04 11:47 ` Philip K.
  2021-02-09 21:56 ` Stefan Monnier
  0 siblings, 2 replies; 13+ messages in thread
From: Philip K. @ 2021-02-04 11:43 UTC (permalink / raw)
  To: emacs-devel

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


Hi,

I would like to propose adding a package to ELPA:


[-- Attachment #2: setup.el --]
[-- Type: text/plain, Size: 9193 bytes --]

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

;; This file is NOT part of Emacs.
;;
;; This file is in the public domain, to the extent possible under law,
;; published under the CC0 1.0 Universal license.
;;
;; For a full copy of the CC0 license see
;; https://creativecommons.org/publicdomain/zero/1.0/legalcode

;;; 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))

;; will be replaced with the functional equivalent of

;;     (global-set-key (kbd "C-c s") #'shell)
;;     (with-eval-after-load 'shell
;;        (define-key shell-mode-map (kbd "C-c s") #'bury-buffer))
;;
;;
;;     (unless (package-install-p 'paredit)
;;       (package-install 'paredit ))
;;     (delq (assq 'paredit-mode minor-mode-alist)
;;           minor-mode-alist)
;;     (add-hook 'scheme-mode-hook #'paredit-mode)
;;     (add-hook 'lisp-mode-hook #'paredit-mode)

;; Additional "keywords" can be defined using `setup-define'.

;;; Code:

(eval-when-compile (require 'cl-macs))

\f
;;; `setup' macros

(defvar setup-macros nil
  "Local macro definitions to be bound in `setup' bodies.")

(defun setup-after-load (body)
  "Wrap BODY in a `with-eval-after-load'."
  ``(with-eval-after-load setup-name ,,body))

;;;###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)))
         (mode (if (string-match-p "-mode\\'" (symbol-name name))
                   name
                 (intern (format "%s-mode" name)))))
    (when (symbolp (car body))
      (pop body))
    `(let ((setup-name ',name)
           (setup-mode ',mode)          ;best guess
           (setup-map ',(intern (format "%s-map" mode)))
          (setup-hook ',(intern (format "%s-hook" mode))))
       (ignore setup-name setup-mode setup-map setup-hook)
       (catch 'setup-exit
         (cl-macrolet ,setup-macros
           ,@body)))))

;;;###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."
  (declare (indent 1))
  (cl-assert (symbolp name))
  (cl-assert (functionp fn))
  (cl-assert (listp opts))
  ;; save metadata
  (put name 'setup-get-name (plist-get opts :name))
  (put name 'setup-documentation (plist-get opts :doc))
  (put name 'setup-signature (plist-get opts :sig))
  (put name 'lisp-indent-function (plist-get opts :indent))
  ;; forget previous definition
  (setq setup-macros (delq (assq name setup-macros)
                           setup-macros))
  ;; 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)
  (set-advertised-calling-convention name (plist-get opts :sig) nil))

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

\f
;;; definitions of `setup' keywords

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

(setup-define 'with-map
  (lambda (map &rest body)
    `(let ((setup-map ',map))
       ,@body))
  :sig '(MAP &body BODY)
  :doc "Change the MAP that BODY will bind to"
  :indent 1)

(setup-define 'with-hook
  (lambda (hook &body body)
    `(let ((setup-hook ',hook))
       ,@body))
  :sig '(HOOK &body BODY)
  :doc "Change the HOOK that BODY will use."
  :indent 1)

(setup-define 'package
  (lambda (package)
    `(unless (package-installed-p ',package)
       (package-install ',package)))
  :sig '(PACKAGE *)
  :doc "Install PACKAGE if it hasn't been installed yet."
  :name #'cadr)

(setup-define 'global
  (lambda (bind)
    (let ((key (car bind))
          (fn (cadr bind)))
      `(global-set-key ,(if (atom key) `(kbd ,key) key) #',fn)))
  :sig '((KEY FUNCTION)*)
  :doc "Globally bind KEY to FUNCTION.
If KEY is an atom, the function `kbd' will be applied.")

(setup-define 'bind
  (lambda (bind)
    (let ((key (car bind))
          (fn (cadr bind)))
      `(define-key (eval setup-map) ,(if (atom key) `(kbd ,key) key) #',fn)))
  :sig '((KEY FUNCTION)*)
  :doc "Bind KEY to FUNCTION in current map.
If KEY is an atom, the function `kbd' will be applied."
  :wrap #'setup-after-load)

(setup-define 'unbind
  (lambda (key)
    `(define-key (eval setup-map) ,(if (atom key) `(kbd ,key) key) nil))
  :sig '(KEY *)
  :doc "Unbind KEY in current map.
If KEY is an atom, the function `kbd' will be applied."
  :wrap #'setup-after-load)

(setup-define 'hook
  (lambda (hook)
    `(add-hook setup-hook #',hook))
  :sig '(FUNCTION *)
  :doc "Add FUNCTION to current hook.")

(setup-define 'hook-into
  (lambda (mode)
    `(add-hook ',(intern (concat (symbol-name mode) "-hook"))
               setup-mode))
  :sig '(HOOK *)
  :doc "Add current mode to HOOK.")

(setup-define 'option
  (lambda (assign)
    (let ((opt (car assign))
          (val (cadr assign)))
      `(progn
         (customize-set-variable ',opt ,val)
         (put ',opt 'saved-value nil))))
  :sig '((NAME VAL) *)
  :doc "Set the option NAME to VAL.")

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

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

(setup-define 'local-hook
  (lambda (entry)
    (let ((hook (car entry))
          (fn (cadr entry)))
      `(add-hook setup-hook
                 (lambda ()
                   (add-hook ,hook #',fn nil t)))))
  :sig '((HOOK FUNCTION) *)
  :doc "Add FUNCTION to HOOK only in buffers of the current mode.")

(setup-define 'needs
  (lambda (binary)
    `(unless (executable-find ,binary)
       (throw 'setup-exit nil)))
  :sig '(PROGRAM *)
  :doc "If PROGRAM is not in the path, stop here.")

(setup-define 'require
  (lambda (feature) `(require ,feature))
  :sig '(FEATURE)
  :doc "Require FEATURE to be loaded."
  :name #'cadr)

(setup-define 'when-loaded
  (lambda (&rest body) `(progn ,@body))
  :sig '(&body BODY)
  :doc "Evaluate BODY after the current feature has been loaded."
  :wrap #'setup-after-load)

(provide 'setup)

;;; setup.el ends here

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


This can be compared to use-package. The difference is that instead of
the keyword-argument structure, it uses local macros that allow
interleaving regular lisp with the configuration syntax. In this sense,
it /might/ be compared to Common Lisp's Loop[0] and Iterate[1] control
structures.

I'll just quote an example from the commentary section to demonstrate
how this looks like:

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

        will be replaced with the functional equivalent of

            (global-set-key (kbd "C-c s") #'shell)
            (with-eval-after-load 'shell
               (define-key shell-mode-map (kbd "C-c s") #'bury-buffer))

            (unless (package-install-p 'paredit)
              (package-install 'paredit ))
            (delq (assq 'paredit-mode minor-mode-alist)
                  minor-mode-alist)
            (add-hook 'scheme-mode-hook #'paredit-mode)
            (add-hook 'lisp-mode-hook #'paredit-mode)


If there are any comments/improvements on the code itself, I'd be glad
to fix them. I am the only author, so there should be no copyright
issues.

[0] http://www.lispworks.com/documentation/HyperSpec/Body/m_loop.htm
[1] https://common-lisp.net/project/iterate/doc/Don_0027t-Loop-Iterate.html

-- 
	Philip K.

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

* Re: [ELPA] A Setup package
  2021-02-04 11:43 [ELPA] A Setup package Philip K.
@ 2021-02-04 11:47 ` Philip K.
  2021-02-09 21:56 ` Stefan Monnier
  1 sibling, 0 replies; 13+ messages in thread
From: Philip K. @ 2021-02-04 11:47 UTC (permalink / raw)
  To: emacs-devel

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


I just noticed that I accidentally inserted the file inline instead of
attaching it. Here's the fixed message:

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

> Hi,
>
> I would like to propose adding a package to ELPA:


[-- Attachment #2: setup.el --]
[-- Type: text/plain, Size: 9193 bytes --]

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

;; This file is NOT part of Emacs.
;;
;; This file is in the public domain, to the extent possible under law,
;; published under the CC0 1.0 Universal license.
;;
;; For a full copy of the CC0 license see
;; https://creativecommons.org/publicdomain/zero/1.0/legalcode

;;; 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))

;; will be replaced with the functional equivalent of

;;     (global-set-key (kbd "C-c s") #'shell)
;;     (with-eval-after-load 'shell
;;        (define-key shell-mode-map (kbd "C-c s") #'bury-buffer))
;;
;;
;;     (unless (package-install-p 'paredit)
;;       (package-install 'paredit ))
;;     (delq (assq 'paredit-mode minor-mode-alist)
;;           minor-mode-alist)
;;     (add-hook 'scheme-mode-hook #'paredit-mode)
;;     (add-hook 'lisp-mode-hook #'paredit-mode)

;; Additional "keywords" can be defined using `setup-define'.

;;; Code:

(eval-when-compile (require 'cl-macs))

\f
;;; `setup' macros

(defvar setup-macros nil
  "Local macro definitions to be bound in `setup' bodies.")

(defun setup-after-load (body)
  "Wrap BODY in a `with-eval-after-load'."
  ``(with-eval-after-load setup-name ,,body))

;;;###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)))
         (mode (if (string-match-p "-mode\\'" (symbol-name name))
                   name
                 (intern (format "%s-mode" name)))))
    (when (symbolp (car body))
      (pop body))
    `(let ((setup-name ',name)
           (setup-mode ',mode)          ;best guess
           (setup-map ',(intern (format "%s-map" mode)))
          (setup-hook ',(intern (format "%s-hook" mode))))
       (ignore setup-name setup-mode setup-map setup-hook)
       (catch 'setup-exit
         (cl-macrolet ,setup-macros
           ,@body)))))

;;;###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."
  (declare (indent 1))
  (cl-assert (symbolp name))
  (cl-assert (functionp fn))
  (cl-assert (listp opts))
  ;; save metadata
  (put name 'setup-get-name (plist-get opts :name))
  (put name 'setup-documentation (plist-get opts :doc))
  (put name 'setup-signature (plist-get opts :sig))
  (put name 'lisp-indent-function (plist-get opts :indent))
  ;; forget previous definition
  (setq setup-macros (delq (assq name setup-macros)
                           setup-macros))
  ;; 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)
  (set-advertised-calling-convention name (plist-get opts :sig) nil))

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

\f
;;; definitions of `setup' keywords

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

(setup-define 'with-map
  (lambda (map &rest body)
    `(let ((setup-map ',map))
       ,@body))
  :sig '(MAP &body BODY)
  :doc "Change the MAP that BODY will bind to"
  :indent 1)

(setup-define 'with-hook
  (lambda (hook &body body)
    `(let ((setup-hook ',hook))
       ,@body))
  :sig '(HOOK &body BODY)
  :doc "Change the HOOK that BODY will use."
  :indent 1)

(setup-define 'package
  (lambda (package)
    `(unless (package-installed-p ',package)
       (package-install ',package)))
  :sig '(PACKAGE *)
  :doc "Install PACKAGE if it hasn't been installed yet."
  :name #'cadr)

(setup-define 'global
  (lambda (bind)
    (let ((key (car bind))
          (fn (cadr bind)))
      `(global-set-key ,(if (atom key) `(kbd ,key) key) #',fn)))
  :sig '((KEY FUNCTION)*)
  :doc "Globally bind KEY to FUNCTION.
If KEY is an atom, the function `kbd' will be applied.")

(setup-define 'bind
  (lambda (bind)
    (let ((key (car bind))
          (fn (cadr bind)))
      `(define-key (eval setup-map) ,(if (atom key) `(kbd ,key) key) #',fn)))
  :sig '((KEY FUNCTION)*)
  :doc "Bind KEY to FUNCTION in current map.
If KEY is an atom, the function `kbd' will be applied."
  :wrap #'setup-after-load)

(setup-define 'unbind
  (lambda (key)
    `(define-key (eval setup-map) ,(if (atom key) `(kbd ,key) key) nil))
  :sig '(KEY *)
  :doc "Unbind KEY in current map.
If KEY is an atom, the function `kbd' will be applied."
  :wrap #'setup-after-load)

(setup-define 'hook
  (lambda (hook)
    `(add-hook setup-hook #',hook))
  :sig '(FUNCTION *)
  :doc "Add FUNCTION to current hook.")

(setup-define 'hook-into
  (lambda (mode)
    `(add-hook ',(intern (concat (symbol-name mode) "-hook"))
               setup-mode))
  :sig '(HOOK *)
  :doc "Add current mode to HOOK.")

(setup-define 'option
  (lambda (assign)
    (let ((opt (car assign))
          (val (cadr assign)))
      `(progn
         (customize-set-variable ',opt ,val)
         (put ',opt 'saved-value nil))))
  :sig '((NAME VAL) *)
  :doc "Set the option NAME to VAL.")

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

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

(setup-define 'local-hook
  (lambda (entry)
    (let ((hook (car entry))
          (fn (cadr entry)))
      `(add-hook setup-hook
                 (lambda ()
                   (add-hook ,hook #',fn nil t)))))
  :sig '((HOOK FUNCTION) *)
  :doc "Add FUNCTION to HOOK only in buffers of the current mode.")

(setup-define 'needs
  (lambda (binary)
    `(unless (executable-find ,binary)
       (throw 'setup-exit nil)))
  :sig '(PROGRAM *)
  :doc "If PROGRAM is not in the path, stop here.")

(setup-define 'require
  (lambda (feature) `(require ,feature))
  :sig '(FEATURE)
  :doc "Require FEATURE to be loaded."
  :name #'cadr)

(setup-define 'when-loaded
  (lambda (&rest body) `(progn ,@body))
  :sig '(&body BODY)
  :doc "Evaluate BODY after the current feature has been loaded."
  :wrap #'setup-after-load)

(provide 'setup)

;;; setup.el ends here

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


> This can be compared to use-package. The difference is that instead of
> the keyword-argument structure, it uses local macros that allow
> interleaving regular lisp with the configuration syntax. In this sense,
> it /might/ be compared to Common Lisp's Loop[0] and Iterate[1] control
> structures.
>
> I'll just quote an example from the commentary section to demonstrate
> how this looks like:
>
>         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))
>
>         will be replaced with the functional equivalent of
>
>             (global-set-key (kbd "C-c s") #'shell)
>             (with-eval-after-load 'shell
>                (define-key shell-mode-map (kbd "C-c s") #'bury-buffer))
>
>             (unless (package-install-p 'paredit)
>               (package-install 'paredit ))
>             (delq (assq 'paredit-mode minor-mode-alist)
>                   minor-mode-alist)
>             (add-hook 'scheme-mode-hook #'paredit-mode)
>             (add-hook 'lisp-mode-hook #'paredit-mode)
>
>
> If there are any comments/improvements on the code itself, I'd be glad
> to fix them. I am the only author, so there should be no copyright
> issues.
>
> [0] http://www.lispworks.com/documentation/HyperSpec/Body/m_loop.htm
> [1] https://common-lisp.net/project/iterate/doc/Don_0027t-Loop-Iterate.html

-- 
	Philip K.

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

* Re: [ELPA] A Setup package
  2021-02-04 11:43 [ELPA] A Setup package Philip K.
  2021-02-04 11:47 ` Philip K.
@ 2021-02-09 21:56 ` Stefan Monnier
  2021-02-09 23:42   ` Philip K.
  1 sibling, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2021-02-09 21:56 UTC (permalink / raw)
  To: Philip K.; +Cc: emacs-devel

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?

> ;;; 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?

> ;;; 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.

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

> ;;;###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.
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`.

> ;;;###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!
- `: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))))

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

>   ;; 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.

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

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

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

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).

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

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


        Stefan




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

* Re: [ELPA] A Setup package
  2021-02-09 21:56 ` Stefan Monnier
@ 2021-02-09 23:42   ` Philip K.
  2021-03-11 16:17     ` Philip Kaludercic
  0 siblings, 1 reply; 13+ messages in thread
From: Philip K. @ 2021-02-09 23:42 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

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 #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 658 bytes --]

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

* Re: [ELPA] A Setup package
  2021-02-09 23:42   ` Philip K.
@ 2021-03-11 16:17     ` Philip Kaludercic
  2021-03-13 19:14       ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Philip Kaludercic @ 2021-03-11 16:17 UTC (permalink / raw)
  To: emacs-devel; +Cc: Stefan Monnier

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

I think all the issues should now be resolved. I just pushed a commit
adding edebug support.  setup-help was moved into setup's docstring, a
lot of the macros were redefined to look more like setq, and all macros
were redefined to use keywords (for now).

The source code can be found here:

    https://git.sr.ht/~zge/setup/tree/master/item/setup.el

-- 
	Philip K.



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

* Re: [ELPA] A Setup package
  2021-03-11 16:17     ` Philip Kaludercic
@ 2021-03-13 19:14       ` Stefan Monnier
  2021-03-13 19:44         ` Philip Kaludercic
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2021-03-13 19:14 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel

> I think all the issues should now be resolved. I just pushed a commit
> adding edebug support.  setup-help was moved into setup's docstring, a
> lot of the macros were redefined to look more like setq, and all macros
> were redefined to use keywords (for now).

[ I have some further suggestions for the code, but they'll come later.  ]

I was about to add it to GNU ELPA, but I noticed two problems:

1- The file is missing a copyright notice and it is using the CC0 license.
   See the patch below to fix those issues (the copyright part reflects
   the fact that you consider the package as being covered by your
   copyright assignment and the license change is because we want to
   distribute all GNU ELPA packages under the GPLv3 license; you're of
   course perfectly free to also distribute this package under any other
   license you want, but we want to use the GPLv3 for the copy we
   distribute).

2- AFAIK you rebased (or force-pushed or something like that) your
   branch, which makes it painful for any"one" tracking your branch, such
   as elpa.git.  In order for the elpa.git branch tracking your
   SourceHut repository to work sanely it'll be important not to rebase
   (or force-push...) in the future.

Let me know when point 1 is fixed so I can add the package to GNU ELPA.


        Stefan


PS: I removed the "This file is NOT part of Emacs" line because this is
a bit problematic for GNU ELPA packages: they're at the same time part
of Emacs (the project) and not part of Emacs (the release tarballs) ;-)


diff --git a/setup.el b/setup.el
index e547243340..510c599d5f 100644
--- a/setup.el
+++ b/setup.el
@@ -1,18 +1,25 @@
 ;;; setup.el --- Helpful Configuration Macro    -*- lexical-binding: t -*-
 
+;; Copyright (C) 2021  Free Software Foundation, Inc.
+
 ;; Author: Philip K. <philipk@posteo.net>
 ;; Maintainer: Philip K. <philipk@posteo.net>
 ;; Version: 0.1.0
 ;; Package-Requires: ((emacs "26.1"))
 ;; Keywords: lisp, local
 
-;; This file is NOT part of Emacs.
-;;
-;; This file is in the public domain, to the extent possible under law,
-;; published under the CC0 1.0 Universal license.
-;;
-;; For a full copy of the CC0 license see
-;; https://creativecommons.org/publicdomain/zero/1.0/legalcode
+;; This package is Free Software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; This package is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.
 
 ;;; Commentary:
 




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

* Re: [ELPA] A Setup package
  2021-03-13 19:14       ` Stefan Monnier
@ 2021-03-13 19:44         ` Philip Kaludercic
  2021-03-14 15:07           ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Philip Kaludercic @ 2021-03-13 19:44 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>> I think all the issues should now be resolved. I just pushed a commit
>> adding edebug support.  setup-help was moved into setup's docstring, a
>> lot of the macros were redefined to look more like setq, and all macros
>> were redefined to use keywords (for now).
>
> [ I have some further suggestions for the code, but they'll come later.  ]
>
> I was about to add it to GNU ELPA, but I noticed two problems:
>
> 1- The file is missing a copyright notice and it is using the CC0 license.
>    See the patch below to fix those issues (the copyright part reflects
>    the fact that you consider the package as being covered by your
>    copyright assignment and the license change is because we want to
>    distribute all GNU ELPA packages under the GPLv3 license; you're of
>    course perfectly free to also distribute this package under any other
>    license you want, but we want to use the GPLv3 for the copy we
>    distribute).

Done.

> 2- AFAIK you rebased (or force-pushed or something like that) your
>    branch, which makes it painful for any"one" tracking your branch, such
>    as elpa.git.  In order for the elpa.git branch tracking your
>    SourceHut repository to work sanely it'll be important not to rebase
>    (or force-push...) in the future.

I actually created a new repository, because it seemed cleaner compared
to pushing all the changes as commits.  But I will keep this in mind for
the future.

-- 
	Philip K.



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

* Re: [ELPA] A Setup package
  2021-03-13 19:44         ` Philip Kaludercic
@ 2021-03-14 15:07           ` Stefan Monnier
  2021-03-14 16:21             ` Philip Kaludercic
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2021-03-14 15:07 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel

>>> I think all the issues should now be resolved. I just pushed a commit
>>> adding edebug support.  setup-help was moved into setup's docstring, a
>>> lot of the macros were redefined to look more like setq, and all macros
>>> were redefined to use keywords (for now).
>>
>> [ I have some further suggestions for the code, but they'll come later.  ]
>>
>> I was about to add it to GNU ELPA, but I noticed two problems:
>>
>> 1- The file is missing a copyright notice and it is using the CC0 license.
>>    See the patch below to fix those issues (the copyright part reflects
>>    the fact that you consider the package as being covered by your
>>    copyright assignment and the license change is because we want to
>>    distribute all GNU ELPA packages under the GPLv3 license; you're of
>>    course perfectly free to also distribute this package under any other
>>    license you want, but we want to use the GPLv3 for the copy we
>>    distribute).
>
> Done.

OK, we now have:

    http://elpa.gnu.org/devel/setup.html

We don't yet have http://elpa.gnu.org/packages/setup.html OTOH because
the 0.1.0 version of the code is missing the copyright changes (the
release is made from the commit in which you bump the version number).
So if you want to make a release based on this code, you'll want to bump
up the version number.

Here's some other suggestions.  Obviously the use of `help--make-usage`
is problematic since the `--` indicates it's not supposed to be used by
"outsiders", so we should change help.el or help-fns.el to provide
a proper function for that (pcase uses `help-fns--signature` instead,
which suffers from the same problem).


        Stefan


diff --git a/.gitignore b/.gitignore
index 507dafa2f6..0096d0bb66 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,3 +1,5 @@
 *.elc
 *~
-\#*\#
\ No newline at end of file
+\#*\#
+/setup-pkg.el
+/setup-autoloads.el
diff --git a/setup.el b/setup.el
index 510c599d5f..b29ef3d0f6 100644
--- a/setup.el
+++ b/setup.el
@@ -88,10 +88,8 @@
     (insert (documentation (symbol-function 'setup) 'raw))
     (dolist (sym (sort (mapcar #'car setup-macros)
                        #'string-lessp))
-      (let ((sig (if (get sym 'setup-signature)
-                     (cons sym (get sym 'setup-signature))
-                   (list sym))))
-        (insert (format " - %s\n\n" sig)
+      (let ((sig (get sym 'setup-signature)))
+        (insert (format " - %s\n\n" (help--make-usage sym sig))
                 (or (get sym 'setup-documentation)
                     "No documentation.")
                 "\n\n")))
@@ -122,55 +120,56 @@ The following local macros are defined in a `setup' body:\n\n"
   "Define `setup'-local macro NAME using function FN.
 The plist OPTS may contain the key-value pairs:
 
-  :name
+  :name 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
+  :indent SPEC
 Change indentation behaviour.  See symbol `lisp-indent-function'.
 
-  :after-loaded
+  :after-loaded BOOL
 Wrap the macro in a `with-eval-after-load' body.
 
-  :repeatable
-Allow macro to be automatically repeated, using FN's arity.
+  :repeatable ARITY
+Allow macro to be automatically repeated.
 
-  :signature
+  :signature SIG
 Give an advertised calling convention.
 
-  :documentation
+  :documentation STRING
 A documentation string.
 
-  :debug
-A edebug specification, see Info node `(elisp)
-Specification List'.  If not given, it is assumed nothing is
-evaluated.  If macro is :repeatable, a &rest will be added before
-the specification."
+  :debug SPEC
+A edebug specification, see Info node `(elisp) Specification List'.
+If not given, it is assumed nothing is evaluated."
   (declare (indent 1))
   (cl-assert (symbolp name))
   (cl-assert (functionp fn))
   (cl-assert (listp opts))
   ;; save metadata
   (put name 'setup-documentation (plist-get opts :documentation))
-  (put name 'setup-signature (plist-get opts :signature))
+  (put name 'setup-signature
+       (or (plist-get opts :signature)
+           (append (help-function-arglist fn 'preserve-names)
+                   (if (plist-get opts :repeatable) '(...)))))
   (put name 'setup-shorthand (plist-get opts :shorthand))
   (put name 'lisp-indent-function (plist-get opts :indent))
   (put name 'setup-indent (plist-get opts :indent))
-  (put name 'setup-repeatable (plist-get opts :repeatable))
+  ;; (put name 'setup-repeatable (plist-get opts :repeatable))
   (put name 'setup-debug (plist-get opts :debug))
   ;; forget previous definition
   (setq setup-macros (delq (assq name setup-macros)
                            setup-macros))
   ;; define macro for `cl-macrolet'
-  (push (let* ((arity (func-arity fn))
-               (body (if (plist-get opts :repeatable)
+  (push (let* ((arity (plist-get opts :repeatable))
+               (body (if arity
                          `(progn
-                            (unless (zerop (mod (length args) ,(car arity)))
+                            (unless (zerop (mod (length args) ,arity))
                               (error "Illegal arguments"))
                             (let (aggr)
                               (while args
-                                (let ((rest (nthcdr ,(car arity) args)))
-                                  (setf (nthcdr ,(car arity) args) nil)
+                                (let ((rest (nthcdr ,arity args)))
+                                  (setf (nthcdr ,arity args) nil)
                                   (push (apply #',fn args) aggr)
                                   (setq args rest)))
                               `(progn ,@(nreverse aggr))))
@@ -180,13 +179,14 @@ the specification."
                       `(with-eval-after-load setup-name ,,body))
             `(,name (&rest args) `,,body)))
         setup-macros)
+  ;; FIXME: Use `&interpose' in Emacs≥28.
   (put 'setup 'edebug-form-spec
        (let (specs)
          (dolist (name (mapcar #'car setup-macros))
            (let ((body (cond ((eq (get name 'setup-debug) 'none) nil)
                              ((get name 'setup-debug) nil)
                              ('(sexp)))))
-             (push (if (get name 'setup-repeatable)
+             (push (if (plist-get opts :repeatable)
                        `(,(symbol-name name) &rest ,@body)
                      `(,(symbol-name name) ,@body))
                    specs)))
@@ -215,7 +215,7 @@ the specification."
            (setup-hook ',(intern (format "%s-hook" mode))))
        (ignore setup-mode setup-map setup-hook)
        ,@body))
-  :signature '(MODE &body BODY)
+  ;; :signature '(MODE &body BODY)
   :documentation "Change the MODE that BODY is configuring."
   :debug '(sexp setup)
   :indent 1)
@@ -224,7 +224,7 @@ the specification."
   (lambda (map &rest body)
     `(let ((setup-map ',map))
        ,@body))
-  :signature '(MAP &body BODY)
+  ;; :signature '(MAP &body BODY)
   :documentation "Change the MAP that BODY will bind to"
   :debug '(sexp setup)
   :indent 1)
@@ -233,7 +233,7 @@ the specification."
   (lambda (hook &rest body)
     `(let ((setup-hook ',hook))
        ,@body))
-  :signature '(HOOK &body BODY)
+  ;; :signature '(HOOK &body BODY)
   :documentation "Change the HOOK that BODY will use."
   :debug '(sexp setup)
   :indent 1)
@@ -242,18 +242,18 @@ the specification."
   (lambda (package)
     `(unless (package-installed-p ',package)
        (package-install ',package)))
-  :signature '(PACKAGE ...)
+  ;; :signature '(PACKAGE ...)
   :documentation "Install PACKAGE if it hasn't been installed yet."
   :shorthand #'cadr
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :require
   (lambda (feature)
     `(require ',feature))
-  :signature '(FEATURE ...)
+  ;; :signature '(FEATURE ...)
   :documentation "Eagerly require FEATURE."
   :shorthand #'cadr
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :global
   (lambda (key fn)
@@ -265,7 +265,7 @@ the specification."
   :signature '(KEY FUNCTION ...)
   :documentation "Globally bind KEY to FUNCTION."
   :debug '(form [&or [symbolp sexp] form])
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :bind
   (lambda (key fn)
@@ -278,7 +278,7 @@ the specification."
   :documentation "Bind KEY to FUNCTION in current map."
   :after-loaded t
   :debug '(form [&or [symbolp sexp] form])
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :unbind
   (lambda (key)
@@ -287,11 +287,11 @@ the specification."
               `(kbd ,key)
           ,key)
        nil))
-  :signature '(KEY ...)
+  ;; :signature '(KEY ...)
   :documentation "Unbind KEY in current map."
   :after-loaded t
   :debug '(form)
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :rebind
   (lambda (key fn)
@@ -306,23 +306,23 @@ the specification."
   :signature '(KEY FUNCTION ...)
   :documentation "Unbind the current key for FUNCTION, and bind it to KEY."
   :after-loaded t
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :hook
-  (lambda (hook)
-    `(add-hook setup-hook #',hook))
-  :signature '(FUNCTION ...)
+  (lambda (function)
+    `(add-hook setup-hook #',function))
+  ;; :signature '(FUNCTION ...)
   :documentation "Add FUNCTION to current hook."
   :debug '(form [&or [symbolp sexp] form])
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :hook-into
   (lambda (mode)
     `(add-hook ',(intern (concat (symbol-name mode) "-hook"))
                setup-mode))
-  :signature '(HOOK ...)
+  ;; :signature '(HOOK ...)
   :documentation "Add current mode to HOOK."
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :option
   (lambda (var val)
@@ -349,7 +349,7 @@ will use the car value to modify the behaviour.  If NAME has the
 form (append VAR), VAL is appended to VAR.  If NAME has the
 form (prepend VAR), VAL is prepended to VAR."
   :debug '(sexp form)
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :hide-mode
   (lambda ()
@@ -370,7 +370,7 @@ form (prepend VAR), VAL is prepended to VAR."
                  val `(cons ,val ,name)))
           ((error "Invalid variable %S" name)))
     `(add-hook setup-hook (lambda () (setq-local ,name ,val))))
-  :signature '(name VAL ...)
+  ;; :signature '(name VAL ...)
   :documentation "Set the value of NAME to VAL in buffers of the current mode.
 
 NAME may be a symbol, or a cons-cell.  If NAME is a cons-cell, it
@@ -378,7 +378,7 @@ will use the car value to modify the behaviour.  If NAME has the
 form (append VAR), VAL is appended to VAR.  If NAME has the
 form (prepend VAR), VAL is prepended to VAR."
   :debug '(sexp form)
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :local-hook
   (lambda (hook fn)
@@ -388,36 +388,38 @@ form (prepend VAR), VAL is prepended to VAR."
   :signature '(HOOK FUNCTION ...)
   :documentation "Add FUNCTION to HOOK only in buffers of the current mode."
   :debug '(symbolp form)
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :also-load
   (lambda (feature)
     `(require ',feature))
-  :signature '(FEATURE ...)
+  ;; :signature '(FEATURE ...)
   :documentation "Load FEATURE with the current body."
-  :repeatable t
+  :repeatable 1
   :after-loaded t)
 
 (setup-define :needs
-  (lambda (binary)
-    `(unless (executable-find ,binary)
+  (lambda (executable)
+    `(unless (executable-find ,executable)
        (throw 'setup-exit nil)))
-  :signature '(PROGRAM ...)
-  :documentation "If PROGRAM is not in the path, stop here."
-  :repeatable t)
+  ;; :signature '(PROGRAM ...)
+  :documentation "If EXECUTABLE is not in the path, stop here."
+  :repeatable 1)
 
 (setup-define :if
   (lambda (condition)
     `(unless ,condition
        (throw 'setup-exit nil)))
-  :signature '(CONDITION ...)
+  ;; :signature '(CONDITION ...)
+  ;; FIXME: I find this semantics odd for this name.  Maybe you should rename
+  ;; it to `:stop-if'?
   :documentation "If CONDITION is non-nil, stop evaluating the body."
   :debug '(form)
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :when-loaded
   (lambda (&rest body) `(progn ,@body))
-  :signature '(&body BODY)
+  ;; :signature '(&body BODY)
   :documentation "Evaluate BODY after the current feature has been loaded."
   :debug '(body)
   :after-loaded t)




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

* Re: [ELPA] A Setup package
  2021-03-14 15:07           ` Stefan Monnier
@ 2021-03-14 16:21             ` Philip Kaludercic
  2021-03-14 20:19               ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Philip Kaludercic @ 2021-03-14 16:21 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

> OK, we now have:
>
>     http://elpa.gnu.org/devel/setup.html

Thank you!

> We don't yet have http://elpa.gnu.org/packages/setup.html OTOH because
> the 0.1.0 version of the code is missing the copyright changes (the
> release is made from the commit in which you bump the version number).
> So if you want to make a release based on this code, you'll want to bump
> up the version number.

What exactly is missing?  My understanding was that I had to update the
header and the COPYING file.

> Here's some other suggestions.

I like most of the suggestions, would you mind me committing them with
you as the author?

> Obviously the use of `help--make-usage` is problematic since the `--`
> indicates it's not supposed to be used by "outsiders", so we should
> change help.el or help-fns.el to provide a proper function for that
> (pcase uses `help-fns--signature` instead, which suffers from the same
> problem).

Why use help--make-usage at all?  Something along the same lines could
be simplified (eg. without default values and unused variables) and
directly implemented in setup.

>         (let (specs)
>           (dolist (name (mapcar #'car setup-macros))
>             (let ((body (cond ((eq (get name 'setup-debug) 'none) nil)
>                               ((get name 'setup-debug) nil)
>                               ('(sexp)))))
> -             (push (if (get name 'setup-repeatable)
> +             (push (if (plist-get opts :repeatable)

This doesn't work! The specification is regenerated for every
setup-define call, and if the implicit rest uses opts, all macros will
be treated the same way.

It might be better to save the specification in a separate variable and
modify this destructively for every setup-define call, so as to avoid
the overhead of redefining the entire specification all the time.

-- 
	Philip K.



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

* Re: [ELPA] A Setup package
  2021-03-14 16:21             ` Philip Kaludercic
@ 2021-03-14 20:19               ` Stefan Monnier
  2021-03-14 23:39                 ` Philip Kaludercic
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2021-03-14 20:19 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel

>> We don't yet have http://elpa.gnu.org/packages/setup.html OTOH because
>> the 0.1.0 version of the code is missing the copyright changes (the
>> release is made from the commit in which you bump the version number).
>> So if you want to make a release based on this code, you'll want to bump
>> up the version number.
> What exactly is missing?

You missed this part:

    (the release is made from the commit in which you bump the version
     number)

the commit in which you changed `Version:` to the current 0.1.0 did not
have a "copyright FSF" line.

> My understanding was that I had to update the
> header and the COPYING file.

Actually, we don't include the COPYING file is most cases.  After all,
the GPLv3 license comes with Emacs, so the user obviously has
a copy already ;-)

>> Here's some other suggestions.
> I like most of the suggestions, would you mind me committing them with
> you as the author?

Fine by me.

>> Obviously the use of `help--make-usage` is problematic since the `--`
>> indicates it's not supposed to be used by "outsiders", so we should
>> change help.el or help-fns.el to provide a proper function for that
>> (pcase uses `help-fns--signature` instead, which suffers from the same
>> problem).
>
> Why use help--make-usage at all?

To avoid reinvent the wheel.

>>         (let (specs)
>>           (dolist (name (mapcar #'car setup-macros))
>>             (let ((body (cond ((eq (get name 'setup-debug) 'none) nil)
>>                               ((get name 'setup-debug) nil)
>>                               ('(sexp)))))
>> -             (push (if (get name 'setup-repeatable)
>> +             (push (if (plist-get opts :repeatable)
>
> This doesn't work! The specification is regenerated for every
> setup-define call,

Indeed, sorry 'bout that.

> It might be better to save the specification in a separate variable and
> modify this destructively for every setup-define call, so as to avoid
> the overhead of redefining the entire specification all the time.

I wouldn't bother, no.  In the patch below I instead did the
spec-processing when setting `setup-debug` (and I also dropped the
`none` support because I don't see what it gains: for a macro that
takes no arguments, `&rest sexp` works as well).
And there is a better option for Emacs-28 and after.


        Stefan


diff --git a/setup.el b/setup.el
index 510c599d5f..eca9f23507 100644
--- a/setup.el
+++ b/setup.el
@@ -88,10 +88,8 @@
     (insert (documentation (symbol-function 'setup) 'raw))
     (dolist (sym (sort (mapcar #'car setup-macros)
                        #'string-lessp))
-      (let ((sig (if (get sym 'setup-signature)
-                     (cons sym (get sym 'setup-signature))
-                   (list sym))))
-        (insert (format " - %s\n\n" sig)
+      (let ((sig (get sym 'setup-signature)))
+        (insert (format " - %s\n\n" (help--make-usage sym sig))
                 (or (get sym 'setup-documentation)
                     "No documentation.")
                 "\n\n")))
@@ -122,55 +120,59 @@ The following local macros are defined in a `setup' body:\n\n"
   "Define `setup'-local macro NAME using function FN.
 The plist OPTS may contain the key-value pairs:
 
-  :name
+  :name 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
+  :indent SPEC
 Change indentation behaviour.  See symbol `lisp-indent-function'.
 
-  :after-loaded
+  :after-loaded BOOL
 Wrap the macro in a `with-eval-after-load' body.
 
-  :repeatable
-Allow macro to be automatically repeated, using FN's arity.
+  :repeatable ARITY
+Allow macro to be automatically repeated.
 
-  :signature
+  :signature SIG
 Give an advertised calling convention.
 
-  :documentation
+  :documentation STRING
 A documentation string.
 
-  :debug
-A edebug specification, see Info node `(elisp)
-Specification List'.  If not given, it is assumed nothing is
-evaluated.  If macro is :repeatable, a &rest will be added before
-the specification."
+  :debug SPEC
+A edebug specification, see Info node `(elisp) Specification List'.
+If not given, it is assumed nothing is evaluated."
   (declare (indent 1))
   (cl-assert (symbolp name))
   (cl-assert (functionp fn))
   (cl-assert (listp opts))
   ;; save metadata
   (put name 'setup-documentation (plist-get opts :documentation))
-  (put name 'setup-signature (plist-get opts :signature))
+  (put name 'setup-signature
+       (or (plist-get opts :signature)
+           (append (help-function-arglist fn 'preserve-names)
+                   (if (plist-get opts :repeatable) '(...)))))
   (put name 'setup-shorthand (plist-get opts :shorthand))
   (put name 'lisp-indent-function (plist-get opts :indent))
   (put name 'setup-indent (plist-get opts :indent))
-  (put name 'setup-repeatable (plist-get opts :repeatable))
-  (put name 'setup-debug (plist-get opts :debug))
+  ;; (put name 'setup-repeatable (plist-get opts :repeatable))
+  (put name 'setup-debug
+       (let ((spec (plist-get opts :debug)))
+         (if (plist-get opts :repeatable)
+             (cons '&rest spec) spec)))
   ;; forget previous definition
   (setq setup-macros (delq (assq name setup-macros)
                            setup-macros))
   ;; define macro for `cl-macrolet'
-  (push (let* ((arity (func-arity fn))
-               (body (if (plist-get opts :repeatable)
+  (push (let* ((arity (plist-get opts :repeatable))
+               (body (if arity
                          `(progn
-                            (unless (zerop (mod (length args) ,(car arity)))
+                            (unless (zerop (mod (length args) ,arity))
                               (error "Illegal arguments"))
                             (let (aggr)
                               (while args
-                                (let ((rest (nthcdr ,(car arity) args)))
-                                  (setf (nthcdr ,(car arity) args) nil)
+                                (let ((rest (nthcdr ,arity args)))
+                                  (setf (nthcdr ,arity args) nil)
                                   (push (apply #',fn args) aggr)
                                   (setq args rest)))
                               `(progn ,@(nreverse aggr))))
@@ -180,16 +182,13 @@ the specification."
                       `(with-eval-after-load setup-name ,,body))
             `(,name (&rest args) `,,body)))
         setup-macros)
+  ;; FIXME: Use `&interpose' in Emacs≥28.
   (put 'setup 'edebug-form-spec
        (let (specs)
          (dolist (name (mapcar #'car setup-macros))
-           (let ((body (cond ((eq (get name 'setup-debug) 'none) nil)
-                             ((get name 'setup-debug) nil)
-                             ('(sexp)))))
-             (push (if (get name 'setup-repeatable)
-                       `(,(symbol-name name) &rest ,@body)
-                     `(,(symbol-name name) ,@body))
-                   specs)))
+           (push `(,(symbol-name name)
+                   ,@(or (get name 'setup-debug) '(&rest sexp)))
+                 specs))
          `(&rest &or [symbolp sexp] ,@specs form))))
 
 \f
@@ -215,7 +214,7 @@ the specification."
            (setup-hook ',(intern (format "%s-hook" mode))))
        (ignore setup-mode setup-map setup-hook)
        ,@body))
-  :signature '(MODE &body BODY)
+  ;; :signature '(MODE &body BODY)
   :documentation "Change the MODE that BODY is configuring."
   :debug '(sexp setup)
   :indent 1)
@@ -224,7 +223,7 @@ the specification."
   (lambda (map &rest body)
     `(let ((setup-map ',map))
        ,@body))
-  :signature '(MAP &body BODY)
+  ;; :signature '(MAP &body BODY)
   :documentation "Change the MAP that BODY will bind to"
   :debug '(sexp setup)
   :indent 1)
@@ -233,7 +232,7 @@ the specification."
   (lambda (hook &rest body)
     `(let ((setup-hook ',hook))
        ,@body))
-  :signature '(HOOK &body BODY)
+  ;; :signature '(HOOK &body BODY)
   :documentation "Change the HOOK that BODY will use."
   :debug '(sexp setup)
   :indent 1)
@@ -242,18 +241,18 @@ the specification."
   (lambda (package)
     `(unless (package-installed-p ',package)
        (package-install ',package)))
-  :signature '(PACKAGE ...)
+  ;; :signature '(PACKAGE ...)
   :documentation "Install PACKAGE if it hasn't been installed yet."
   :shorthand #'cadr
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :require
   (lambda (feature)
     `(require ',feature))
-  :signature '(FEATURE ...)
+  ;; :signature '(FEATURE ...)
   :documentation "Eagerly require FEATURE."
   :shorthand #'cadr
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :global
   (lambda (key fn)
@@ -265,7 +264,7 @@ the specification."
   :signature '(KEY FUNCTION ...)
   :documentation "Globally bind KEY to FUNCTION."
   :debug '(form [&or [symbolp sexp] form])
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :bind
   (lambda (key fn)
@@ -278,7 +277,7 @@ the specification."
   :documentation "Bind KEY to FUNCTION in current map."
   :after-loaded t
   :debug '(form [&or [symbolp sexp] form])
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :unbind
   (lambda (key)
@@ -287,11 +286,11 @@ the specification."
               `(kbd ,key)
           ,key)
        nil))
-  :signature '(KEY ...)
+  ;; :signature '(KEY ...)
   :documentation "Unbind KEY in current map."
   :after-loaded t
   :debug '(form)
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :rebind
   (lambda (key fn)
@@ -306,23 +305,23 @@ the specification."
   :signature '(KEY FUNCTION ...)
   :documentation "Unbind the current key for FUNCTION, and bind it to KEY."
   :after-loaded t
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :hook
-  (lambda (hook)
-    `(add-hook setup-hook #',hook))
-  :signature '(FUNCTION ...)
+  (lambda (function)
+    `(add-hook setup-hook #',function))
+  ;; :signature '(FUNCTION ...)
   :documentation "Add FUNCTION to current hook."
   :debug '(form [&or [symbolp sexp] form])
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :hook-into
   (lambda (mode)
     `(add-hook ',(intern (concat (symbol-name mode) "-hook"))
                setup-mode))
-  :signature '(HOOK ...)
+  ;; :signature '(HOOK ...)
   :documentation "Add current mode to HOOK."
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :option
   (lambda (var val)
@@ -349,14 +348,14 @@ will use the car value to modify the behaviour.  If NAME has the
 form (append VAR), VAL is appended to VAR.  If NAME has the
 form (prepend VAR), VAL is prepended to VAR."
   :debug '(sexp form)
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :hide-mode
   (lambda ()
     `(delq (assq setup-mode minor-mode-alist)
            minor-mode-alist))
   :documentation "Hide the mode-line lighter of the current mode."
-  :debug 'none
+  ;; :debug 'none
   :after-loaded t)
 
 (setup-define :local-set
@@ -370,7 +369,7 @@ form (prepend VAR), VAL is prepended to VAR."
                  val `(cons ,val ,name)))
           ((error "Invalid variable %S" name)))
     `(add-hook setup-hook (lambda () (setq-local ,name ,val))))
-  :signature '(name VAL ...)
+  ;; :signature '(name VAL ...)
   :documentation "Set the value of NAME to VAL in buffers of the current mode.
 
 NAME may be a symbol, or a cons-cell.  If NAME is a cons-cell, it
@@ -378,7 +377,7 @@ will use the car value to modify the behaviour.  If NAME has the
 form (append VAR), VAL is appended to VAR.  If NAME has the
 form (prepend VAR), VAL is prepended to VAR."
   :debug '(sexp form)
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :local-hook
   (lambda (hook fn)
@@ -388,36 +387,38 @@ form (prepend VAR), VAL is prepended to VAR."
   :signature '(HOOK FUNCTION ...)
   :documentation "Add FUNCTION to HOOK only in buffers of the current mode."
   :debug '(symbolp form)
-  :repeatable t)
+  :repeatable 2)
 
 (setup-define :also-load
   (lambda (feature)
     `(require ',feature))
-  :signature '(FEATURE ...)
+  ;; :signature '(FEATURE ...)
   :documentation "Load FEATURE with the current body."
-  :repeatable t
+  :repeatable 1
   :after-loaded t)
 
 (setup-define :needs
-  (lambda (binary)
-    `(unless (executable-find ,binary)
+  (lambda (executable)
+    `(unless (executable-find ,executable)
        (throw 'setup-exit nil)))
-  :signature '(PROGRAM ...)
-  :documentation "If PROGRAM is not in the path, stop here."
-  :repeatable t)
+  ;; :signature '(PROGRAM ...)
+  :documentation "If EXECUTABLE is not in the path, stop here."
+  :repeatable 1)
 
 (setup-define :if
   (lambda (condition)
     `(unless ,condition
        (throw 'setup-exit nil)))
-  :signature '(CONDITION ...)
+  ;; :signature '(CONDITION ...)
+  ;; FIXME: I find this semantics odd for this name.  Maybe you should rename
+  ;; it to `:stop-if'?
   :documentation "If CONDITION is non-nil, stop evaluating the body."
   :debug '(form)
-  :repeatable t)
+  :repeatable 1)
 
 (setup-define :when-loaded
   (lambda (&rest body) `(progn ,@body))
-  :signature '(&body BODY)
+  ;; :signature '(&body BODY)
   :documentation "Evaluate BODY after the current feature has been loaded."
   :debug '(body)
   :after-loaded t)




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

* Re: [ELPA] A Setup package
  2021-03-14 20:19               ` Stefan Monnier
@ 2021-03-14 23:39                 ` Philip Kaludercic
  2021-03-15  4:15                   ` Stefan Monnier
  0 siblings, 1 reply; 13+ messages in thread
From: Philip Kaludercic @ 2021-03-14 23:39 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>>> We don't yet have http://elpa.gnu.org/packages/setup.html OTOH because
>>> the 0.1.0 version of the code is missing the copyright changes (the
>>> release is made from the commit in which you bump the version number).
>>> So if you want to make a release based on this code, you'll want to bump
>>> up the version number.
>> What exactly is missing?
>
> You missed this part:
>
>     (the release is made from the commit in which you bump the version
>      number)
>
> the commit in which you changed `Version:` to the current 0.1.0 did not
> have a "copyright FSF" line.

Oops, my bad. I get it now.

>> Why use help--make-usage at all?
>
> To avoid reinvent the wheel.

I could reduce it to

        (mapcar (lambda (arg)
                  (if (string-match "\\`&" (symbol-name arg))
                      arg
                    (intern (upcase (symbol-name arg)))))
                (get sym 'setup-signature))

which does the job.

>> It might be better to save the specification in a separate variable and
>> modify this destructively for every setup-define call, so as to avoid
>> the overhead of redefining the entire specification all the time.
>
> I wouldn't bother, no.  In the patch below I instead did the
> spec-processing when setting `setup-debug` (and I also dropped the
> `none` support because I don't see what it gains: for a macro that
> takes no arguments, `&rest sexp` works as well).
> And there is a better option for Emacs-28 and after.

The reason I introduced `none` was that :hide-mode is not repeatable and
has no arguments, resulting in the edebug specification

    (":hide-mode" sexp)

However this always fails to match. I am thinking about solving this by
checking for a non-nil signature. The only thing I'm not yet sure of if
this could have unintended side effects.

-- 
	Philip K.



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

* Re: [ELPA] A Setup package
  2021-03-14 23:39                 ` Philip Kaludercic
@ 2021-03-15  4:15                   ` Stefan Monnier
  2021-03-15 10:09                     ` Philip Kaludercic
  0 siblings, 1 reply; 13+ messages in thread
From: Stefan Monnier @ 2021-03-15  4:15 UTC (permalink / raw)
  To: Philip Kaludercic; +Cc: emacs-devel

>>> Why use help--make-usage at all?
>> To avoid reinvent the wheel.
> I could reduce it to
>
>         (mapcar (lambda (arg)
>                   (if (string-match "\\`&" (symbol-name arg))
>                       arg
>                     (intern (upcase (symbol-name arg)))))
>                 (get sym 'setup-signature))
>
> which does the job.

Sounds good (but it still shows there's a need for Emacs to provide
a function that does that).

>>> It might be better to save the specification in a separate variable and
>>> modify this destructively for every setup-define call, so as to avoid
>>> the overhead of redefining the entire specification all the time.
>>
>> I wouldn't bother, no.  In the patch below I instead did the
>> spec-processing when setting `setup-debug` (and I also dropped the
>> `none` support because I don't see what it gains: for a macro that
>> takes no arguments, `&rest sexp` works as well).
>> And there is a better option for Emacs-28 and after.
>
> The reason I introduced `none` was that :hide-mode is not repeatable and
> has no arguments, resulting in the edebug specification
>
>     (":hide-mode" sexp)
>
> However this always fails to match.

Indeed that was wrong.  Same problem for non-repeatable macros with more
than one argument.  But in the code I sent in the last message I fixed
this by always using `&rest sexp` for those Setup macros without debug
spec (just like Edebug for normal macros).


        Stefan




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

* Re: [ELPA] A Setup package
  2021-03-15  4:15                   ` Stefan Monnier
@ 2021-03-15 10:09                     ` Philip Kaludercic
  0 siblings, 0 replies; 13+ messages in thread
From: Philip Kaludercic @ 2021-03-15 10:09 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

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

>>>> Why use help--make-usage at all?
>>> To avoid reinvent the wheel.
>> I could reduce it to
>>
>>         (mapcar (lambda (arg)
>>                   (if (string-match "\\`&" (symbol-name arg))
>>                       arg
>>                     (intern (upcase (symbol-name arg)))))
>>                 (get sym 'setup-signature))
>>
>> which does the job.
>
> Sounds good (but it still shows there's a need for Emacs to provide
> a function that does that).

Yes, this is just a temporary compromise.

>> The reason I introduced `none` was that :hide-mode is not repeatable and
>> has no arguments, resulting in the edebug specification
>>
>>     (":hide-mode" sexp)
>>
>> However this always fails to match.
>
> Indeed that was wrong.  Same problem for non-repeatable macros with more
> than one argument.  But in the code I sent in the last message I fixed
> this by always using `&rest sexp` for those Setup macros without debug
> spec (just like Edebug for normal macros).

I've ended up doing something similar now, just also taking :repeatable
into consideration. Either way, I with this working, I'll bump the
version to 0.1.1, to create an official release.

Thank you for all you help, your comments significantly improved the
package!

-- 
	Philip K.



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

end of thread, other threads:[~2021-03-15 10:09 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-04 11:43 [ELPA] A Setup package Philip K.
2021-02-04 11:47 ` Philip K.
2021-02-09 21:56 ` Stefan Monnier
2021-02-09 23:42   ` Philip K.
2021-03-11 16:17     ` Philip Kaludercic
2021-03-13 19:14       ` Stefan Monnier
2021-03-13 19:44         ` Philip Kaludercic
2021-03-14 15:07           ` Stefan Monnier
2021-03-14 16:21             ` Philip Kaludercic
2021-03-14 20:19               ` Stefan Monnier
2021-03-14 23:39                 ` Philip Kaludercic
2021-03-15  4:15                   ` Stefan Monnier
2021-03-15 10:09                     ` Philip Kaludercic

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).