bug#21074: [PATCH] Add docs for two tabulated-list functions

[Eli Zaretskii]

> From: Alex Branham <alex.branham@gmail.com>
> Cc: 21074@debbugs.gnu.org
> Date: Tue, 15 Jan 2019 13:41:28 -0600
> 
> On Tue 15 Jan 2019 at 13:06, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> >> From: Alex Branham <alex.branham@gmail.com>
> >> Date: Tue, 15 Jan 2019 12:19:13 -0600
> >>
> >> This patch adds documentation for tabulated-list-get-{id, entry} to
> >> doc/lispref/modes.texi.
> >
> > Thanks, but what is the rationale for adding these two, specifically?
> 
> I was surprised to see they weren't in there the other day when I was
> playing around with tabulated-list-mode. "How do I get the ID of the
> entry at point" is a pretty common question, I'd guess. Since I saw this
> bug on the bug tracker today I figured I'd submit a patch. If it doesn't
> belong in the manual feel free to ignore the patch (but then this bug
> should probably be closed).

Ah, I wasn't aware there was a previous context for this bug.  (Please
in the future say something to put things in their context, otherwise
one needs to deduce from the bug number that it might have been
discussed previously, and go look up that discussion.  I usually just
ignore the bug number.)

After looking into this for some time, I think we should document
functions of tabulated-mode that are frequently used by its clients,
of which there are some in the Emacs tree.  The above 2 functions
aren't the only ones undocumented in the manual.  So either we
document them all, or we'd need more serious rationale for documenting
just those 2.  Would you like to see what other tabulated-mode
functions are candidates for addition to the manual?

In any case, a few minor comments for the patch:

> +@defun tabulated-list-get-id &optional pos

In the code, this is a defsubst, not a defun.

> +This function returns the entry ID of the entry at @var{pos},
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Just "... returns the ID of the entry" is more clear and concise.

> +                                                               which
> +defaults to point if omitted or @code{nil}.

This "which" makes the text ambiguous: it could refer to the ID or to
POS.  So I suggest to write a separate sentence:

  If omitted or @code{nil}, @var{pos} defaults to position of point.

> +                                             This will be an ID object
> +from @var{tabulated-list-entries}.

An element of tabulated-list-entries could be a function, and what
does this return in that case?  This should be spelled out in the
manual, otherwise the reader will be left out wondering.

> +@defun tabulated-list-get-entry &optional pos
> +This function returns the entry at @var{pos}, which defaults to point
> +if omitted or @code{nil}.  This will be a vector for the ID at
> +@var{pos} from @var{tabulated-list-entries}

This is not detailed enough.  E.g., it says nothing about what will be
the elements of the vector.  The doc string is more detailed, but it
is too terse, and the manual should expand on that.  The description
should also make clear what happens if the corresponding element of
tabulated-list-entries is a function.

> +                                            . If there is no entry at
                                               ^^
Two spaces between sentences, please.

Thanks.