bug#26270: help-function-arglist doesn't respect "(fn ARGS...)" in docstring

bug#26270: help-function-arglist doesn't respect "(fn ARGS...)" in docstring

[npostavs]

Severity: minor

From emacs -Q, evaluate:

    (defun foo (&rest args)
      "Do foo.

    \(fn ARG1 ARG2)")

<f1> f foo RET correctly shows (ARG1 ARG2) as the arglist:

    foo is a Lisp function.

    (foo ARG1 ARG2)

    Do foo.

But

    (help-function-arglist 'foo) ;=> (&rest args)

bug#26270: help-function-arglist doesn't respect "(fn ARGS...)" in docstring

[npostavs]

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

npostavs@users.sourceforge.net writes:

>     (defun foo (&rest args)
>       "Do foo.
>
>     \(fn ARG1 ARG2)")
>
>     (help-function-arglist 'foo) ;=> (&rest args)

Here's a patch which puts the check of the docstring before the lambda
list.  I'm not sure what to do with the PRESERVE-NAMES argument of
`help-function-arglist' though.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: patch --]
[-- Type: text/x-diff, Size: 4451 bytes --]

From 0a84e9441d94485ba20c2e44cb9062a7e4259968 Mon Sep 17 00:00:00 2001
From: Noam Postavsky <npostavs@gmail.com>
Date: Fri, 2 Jun 2017 22:34:32 -0400
Subject: [PATCH v1] Prefer docstring as source of arglist with names
 (Bug#26270)

* lisp/help.el (help-arglist-from-docstring): New function, extracted
from `help-function-arglist'.
(help-function-arglist): Use it before looking for a function's lambda
list.
---
 lisp/help.el | 61 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 33 insertions(+), 28 deletions(-)

diff --git a/lisp/help.el b/lisp/help.el
index 361ab2a01e..4d98c1dc81 100644
--- a/lisp/help.el
+++ b/lisp/help.el
@@ -1417,6 +1417,23 @@ (defun help-add-fundoc-usage (docstring arglist)
                   (error "Unrecognized usage format"))
 	      (help--make-usage-docstring 'fn arglist)))))
 
+(defun help-arglist-from-docstring (doc)
+  "Return argument list parsed from the docstring, DOC.
+DOC should end with \"(fn ARGLIST)\", see Info node `(elisp)
+Function Documentation'."
+  (let* ((docargs (if doc (car (help-split-fundoc doc nil))))
+         (arglist (if docargs (cdr (read (downcase docargs)))))
+         (valid t))
+    ;; Check validity.
+    (dolist (arg arglist)
+      (unless (and (symbolp arg)
+                   (let ((name (symbol-name arg)))
+                     (if (eq (aref name 0) ?&)
+                         (memq arg '(&rest &optional))
+                       (not (string-match "\\." name)))))
+        (setq valid nil)))
+    (when valid arglist)))
+
 (defun help-function-arglist (def &optional preserve-names)
   "Return a formal argument list for the function DEF.
 If PRESERVE-NAMES is non-nil, return a formal arglist that uses
@@ -1425,40 +1442,28 @@ (defun help-function-arglist (def &optional preserve-names)
   (if (and (symbolp def) (fboundp def)) (setq def (indirect-function def)))
   ;; If definition is a macro, find the function inside it.
   (if (eq (car-safe def) 'macro) (setq def (cdr def)))
+  ;; FIXME: What does PRESERVE-NAMES really mean??
   (cond
+   ((help-arglist-from-docstring
+     (ignore-errors (documentation def))))
    ((and (byte-code-function-p def) (listp (aref def 0))) (aref def 0))
    ((eq (car-safe def) 'lambda) (nth 1 def))
    ((eq (car-safe def) 'closure) (nth 2 def))
    ((or (and (byte-code-function-p def) (integerp (aref def 0)))
         (subrp def) (module-function-p def))
-    (or (when preserve-names
-          (let* ((doc (condition-case nil (documentation def) (error nil)))
-                 (docargs (if doc (car (help-split-fundoc doc nil))))
-                 (arglist (if docargs
-                              (cdar (read-from-string (downcase docargs)))))
-                 (valid t))
-            ;; Check validity.
-            (dolist (arg arglist)
-              (unless (and (symbolp arg)
-                           (let ((name (symbol-name arg)))
-                             (if (eq (aref name 0) ?&)
-                                 (memq arg '(&rest &optional))
-                               (not (string-match "\\." name)))))
-                (setq valid nil)))
-            (when valid arglist)))
-        (let* ((arity (func-arity def))
-               (max (cdr arity))
-               (min (car arity))
-               (arglist ()))
-          (dotimes (i min)
-            (push (intern (concat "arg" (number-to-string (1+ i)))) arglist))
-          (when (and (integerp max) (> max min))
-            (push '&optional arglist)
-            (dotimes (i (- max min))
-              (push (intern (concat "arg" (number-to-string (+ 1 i min))))
-                    arglist)))
-          (unless (integerp max) (push '&rest arglist) (push 'rest arglist))
-          (nreverse arglist))))
+    (let* ((arity (func-arity def))
+           (max (cdr arity))
+           (min (car arity))
+           (arglist ()))
+      (dotimes (i min)
+        (push (intern (concat "arg" (number-to-string (1+ i)))) arglist))
+      (when (and (integerp max) (> max min))
+        (push '&optional arglist)
+        (dotimes (i (- max min))
+          (push (intern (concat "arg" (number-to-string (+ 1 i min))))
+                arglist)))
+      (unless (integerp max) (push '&rest arglist) (push 'rest arglist))
+      (nreverse arglist)))
    ((and (autoloadp def) (not (eq (nth 4 def) 'keymap)))
     "[Arg list not available until function definition is loaded.]")
    (t t)))
-- 
2.11.1

bug#26270: help-function-arglist doesn't respect "(fn ARGS...)" in docstring

[Lars Ingebrigtsen]

npostavs@users.sourceforge.net writes:

> Severity: minor
>
>>From emacs -Q, evaluate:
>
>     (defun foo (&rest args)
>       "Do foo.
>
>     \(fn ARG1 ARG2)")
>
> <f1> f foo RET correctly shows (ARG1 ARG2) as the arglist:
>
>     foo is a Lisp function.
>
>     (foo ARG1 ARG2)
>
>     Do foo.
>
> But
>
>     (help-function-arglist 'foo) ;=> (&rest args)

I think the intention of help-function-arglist is to really return the
actual arglist -- it's used when composing functions on top of each
other (like in advice.el), I'm not sure that this is something that
should really be fixed?

Do you see the "real" arglist show up in contexts where the "advertised"
arglist should be displayed instead?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#26270: help-function-arglist doesn't respect "(fn ARGS...)" in docstring

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> I think the intention of help-function-arglist is to really return the
> actual arglist -- it's used when composing functions on top of each
> other (like in advice.el), I'm not sure that this is something that
> should really be fixed?

There was no response in seven weeks, so I'm closing this bug report.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no