Re: [ELPA] A Setup package

[Stefan Monnier]

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