> Make checkdoc work with qualified methods
>
> * lisp/emacs-lisp/checkdoc.el (checkdoc--next-docstring): Handle
> cl-defmethod in a case of its own. Check for the presence of
> qualifiers, and skip them accordingly until the docstring.
Any chance we could use the `doc-string-elt` property (which I just
fixed for `cl-defmethod`) for `checkdoc--next-docstring`?
Stefan
Hi Stefan,
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>> Make checkdoc work with qualified methods
>>
>> * lisp/emacs-lisp/checkdoc.el (checkdoc--next-docstring): Handle
>> cl-defmethod in a case of its own. Check for the presence of
>> qualifiers, and skip them accordingly until the docstring.
>
> Any chance we could use the `doc-string-elt` property (which I just
> fixed for `cl-defmethod`) for `checkdoc--next-docstring`?
>
>
> Stefan
Something along these lines?
(defun checkdoc--next-docstring ()
"When looking at a definition with a doc string, find it.
Move to the next doc string after point, and return t. When not
looking at a definition containing a doc string, return nil and
don't move point."
(pcase (save-excursion (condition-case nil
(read (current-buffer))
;; Conservatively skip syntax errors.
(invalid-read-syntax)
;; Don't bug out if the file is empty (or a
;; definition ends prematurely.
(end-of-file)))
(`(,(and
(or 'defun 'defvar 'defcustom 'defmacro 'defconst 'defsubst
'defadvice 'cl-defun 'cl-defgeneric 'cl-defmacro 'cl-defmethod)
def)
,(pred symbolp)
;; Require an initializer, i.e. ignore single-argument `defvar'
;; forms, which never have a doc string.
,_ . ,_)
(down-list)
;; Skip over function or macro name.
(forward-sexp 1)
;; And now skip until the docstring.
(forward-sexp (1- ; We already skipped the function or macro name.
(pcase (function-get def 'doc-string-elt)
((and (pred numberp) num) num)
((and (pred functionp) fn) (funcall fn)))))
(skip-chars-forward " \n\t")
t)))
Note that I need to do (forward-sexp 1) so the requirements of
cl--defmethod-doc-pos are fulfilled. It may get messy if other defining
forms declare a doc-string-elt function that assumes a different point
position.
BTW, I've noticed that I forgot to add the Bug tag to my commit, I'm
sorry. This commit was part of Bug#46918.
> (`(,(and > (or 'defun 'defvar 'defcustom 'defmacro 'defconst 'defsubst > 'defadvice 'cl-defun 'cl-defgeneric 'cl-defmacro 'cl-defmethod) > def) > ,(pred symbolp) > ;; Require an initializer, i.e. ignore single-argument `defvar' > ;; forms, which never have a doc string. > ,_ . ,_) I think you can reduce that to (pred symbolp) and then check that (function-get def 'doc-string-elt) is non-nil. > Note that I need to do (forward-sexp 1) so the requirements of > cl--defmethod-doc-pos are fulfilled. It may get messy if other defining > forms declare a doc-string-elt function that assumes a different point > position. The starting position of `doc-string-elt` is currently defined de-facto by the code in `lisp-string-in-doc-position-p`, so `cl--defmethod-doc-pos` had no choice in the matter ;-) > BTW, I've noticed that I forgot to add the Bug tag to my commit, I'm > sorry. This commit was part of Bug#46918. I think this deserves a harsh punishment. Stefan
Stefan Monnier <monnier@iro.umontreal.ca> writes: >> (`(,(and >> (or 'defun 'defvar 'defcustom 'defmacro 'defconst 'defsubst >> 'defadvice 'cl-defun 'cl-defgeneric 'cl-defmacro 'cl-defmethod) >> def) >> ,(pred symbolp) >> ;; Require an initializer, i.e. ignore single-argument `defvar' >> ;; forms, which never have a doc string. >> ,_ . ,_) > > I think you can reduce that to > > (pred symbolp) > > and then check that (function-get def 'doc-string-elt) is non-nil. I thought about that: it would "add support" to other definitions, and I wasn't sure if it would be desired to check the docstring for all of them. An el-search reports the following (without counting what checkdoc already supports): define-abbrev-table (*) defface defgroup deftheme (*) define-ibuffer-sorter define-ibuffer-op define-ibuffer-filter defimage (*) define-skeleton defvar-local lambda (**) define-widget defmath define-overloadable-function define-mode-local-override define-obsolete-function-alias define-obsolete-variable-alias cl-iter-defun cl-defstruct cl-deftype define-derived-mode define-minor-mode define-globalized-minor-mode easy-menu-define ert-deftest iter-defun define-generic-mode define-inline define-advice pcase-lambda (**) pcase-defmacro define-erc-module define-ccl-program defun-mh defmacro-mh defgroup-mh defcustom-mh defface-mh ediff-defvar-local defun-cvs-mode (*) * the pattern would need tweaking to detect the form ** wouldn't match, because of the second (pred symbolp) Also, for some definitions (e.g., define-erc-module), the report might be different because the file was/wasn't loaded. To sum it up, I feared that just checking for a non-nil doc-string-elt could make checkdoc somewhat annoying, but I'd like to know what other people think. >> Note that I need to do (forward-sexp 1) so the requirements of >> cl--defmethod-doc-pos are fulfilled. It may get messy if other defining >> forms declare a doc-string-elt function that assumes a different point >> position. > > The starting position of `doc-string-elt` is currently defined de-facto > by the code in `lisp-string-in-doc-position-p`, so > `cl--defmethod-doc-pos` had no choice in the matter ;-) I didn't know that. No worries then. >> BTW, I've noticed that I forgot to add the Bug tag to my commit, I'm >> sorry. This commit was part of Bug#46918. > > I think this deserves a harsh punishment. :-(
>> and then check that (function-get def 'doc-string-elt) is non-nil.
>
> I thought about that: it would "add support" to other definitions, and I
> wasn't sure if it would be desired to check the docstring for all of them.
No idea either, but it seems worth a try.
Stefan
Stefan Monnier <monnier@iro.umontreal.ca> writes:
>>> and then check that (function-get def 'doc-string-elt) is non-nil.
>>
>> I thought about that: it would "add support" to other definitions, and I
>> wasn't sure if it would be desired to check the docstring for all of them.
>
> No idea either, but it seems worth a try.
Very well, I've made the change to checkdoc--next-docstring to check for
a non-nil doc-string-elt property and use it.
> Very well, I've made the change to checkdoc--next-docstring to check for
> a non-nil doc-string-elt property and use it.
Thanks!
Stefan