Re: Poor quality documentation in edebug.el, and recursive documentation.

[Alan Mackenzie]

Hello, Stefan.

Sorry it's been a long time since your post.  Some of my repo copies got
corrupted, and sorting them out was a priority.  I managed it, thanks to
my back up copies.

On Tue, May 05, 2020 at 18:18:26 -0400, Stefan Monnier wrote:
> >     Access slot "def-name" of `edebug--frame' struct CL-X.

> > Huh?  Do I really care about whether it has a compiler macro?  I
> > certainly care about what this function does, and the one liner is
> > gibberish.

> Not sure what you mean by gibberish.

That it's meaning is clear only to those who already know it.

> The function itself is a one-liner, all it does is (as the docstring
> says) is that it accesses the slot called "def-name" of the struct
> CL-X which is presumed to be of type `edebug--frame`.

No, that is not what it does.  It GETs the value in that slot and
RETURNS IT.  That last is the critical thing.  Your sentence,
containing as it does verbs and articles, is clear.  The contents of the
doc string is anything but.

> > What significance does it have in edebug?

> The "--" indicates it's an internal function.  These are quite often
> not documented.

Or they are documented by comments.  An otherwise obscure object
requires one or the other.  Here it has neither.

> > What is the return value of this function?  All of these points are
> > left open by this alleged doc string.  What does CL-X mean?

> It's the name of the argument, as always.

A typical useful doc string would give information about the semantics
of the argument, saying, say,  "where CL-X is a list".

> See the earlier line:

>     (edebug--frame-def-name CL-X)

> The choice of `CL-X` as argument name is indeed poor.  It should
> probably be ARG or V or X instead.  The use of a `cl-` prefix is
> a remnant of the time where we didn't have lexical scoping, IIRC.

> > It carries on.  If you put point over the `edebug.el' and type <CR>, it
> > doesn't go to the defining function, throwing an error instead.

> Indeed, that's unfortunate.  We should improve either `cl-defstruct` or
> `find-function` to fix this.

Thanks, I agree with that.

> > It caries on further, and if anything, gets steadily worse: put point
> > over `edebug-frame' and hit <CR>.  It leads to another vacuous doc
> > string.

> This one can be improved by the patch below (which is the kind of patch
> you reject in CC-mode, ironically enough).

> > ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> > So, what is a cl-structure-class?

> You said so yourself: Since `edebug--frame` is a type of type
> `cl-structure-class`, then `cl-structure-class` is a "type of types".
> IOW a metaclass.

If I said, to a normal person, "a foo is a baz of a baz" they would be
left none the wiser, and might retort "but what IS a foo?".  That's
where I was at the beginning of the week.  I don't know what a
"metaclass" is, I'm still not sure what a cl-structure-class is and the
latter needs documenting coherently.

> >      "cl-structure-class is a type (of kind `cl-structure-class')"

> > embedded in approximately 26 lines, none of which shed any light on what
> > a cl-structure-class is, does, or represents.

> There actual docstring says: "The type of CL structs descriptors."
> The rest describes the fields of those CL struct descriptors (aka class
> objects).

It most assuredly does not.  It _lists_ those fields - it does not
describe them.  One of these fields, for example, is called named.  What
does named do?  What does it represent?  What's it for?  None of these
questions is answered.  named is undocumented, as are all the other
fields.  Why?

> Since we're in the realm of metaclasses here, it's no wonder that the
> info you'll find here won't be very helpful to understand
> `edebug--frame-def-name`.

> > Following the link just redisplays the current doc string,
> > ad infinitum.

> Indeed, this metaclass is its own metaclass (that's the usual way to
> stop the recursion).

A better way would be actually to document it.  It consists of a lot of
fields.  Each of them need documenting for this doc string to be useful,
together with some explanation of what "CL structs descriptors" are.

On the whole, the semantics of Emacs is described in its doc strings.
In that for cl-structure-class, and probably most of its friends, only
syntax is described.  That is a Bad Thing.

>         Stefan


> diff --git a/lisp/emacs-lisp/edebug.el b/lisp/emacs-lisp/edebug.el
> index a376067443a..1b4bbb54e59 100644
> --- a/lisp/emacs-lisp/edebug.el
> +++ b/lisp/emacs-lisp/edebug.el
> @@ -4169,12 +4169,12 @@ edebug-instrumented-backtrace-frames
>    "Stack frames of the current Edebug Backtrace buffer with instrumentation.
>  This should be a list of `edebug---frame' objects.")
 
> -;; Data structure for backtrace frames with information
> -;; from Edebug instrumentation found in the backtrace.
>  (cl-defstruct
>      (edebug--frame
>       (:constructor edebug--make-frame)
>       (:include backtrace-frame))
> +  "Data structure for backtrace frames with information
> +from Edebug instrumentation found in the backtrace."
>    def-name before-index after-index)
 
>  (defun edebug-pop-to-backtrace ()

That just moves text from a comment to a doc string.  It doesn't help
find out what edebug--frame-def-name does.

-- 
Alan Mackenzie (Nuremberg, Germany).