all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Alex Branham <alex.branham@gmail.com>
Cc: 21074@debbugs.gnu.org, mbork@mbork.pl
Subject: bug#21074: [PATCH] Add docs for two tabulated-list functions
Date: Sat, 02 Feb 2019 18:33:01 +0200	[thread overview]
Message-ID: <83h8dmw3pe.fsf@gnu.org> (raw)
In-Reply-To: <87y36y9nzg.fsf@gmail.com> (message from Alex Branham on Sat, 02 Feb 2019 10:03:31 -0600)

> From: Alex Branham <alex.branham@gmail.com>
> Cc: mbork@mbork.pl, 21074@debbugs.gnu.org
> Date: Sat, 02 Feb 2019 10:03:31 -0600
> 
> Thanks. I think the attached patch incorporates all your comments, let
> me know if I missed something.

Looks good.  A couple of minor gotchas, though.

> * doc/lispref/modes.texi: Add documentation for
>   'tabulated-list-delete-entry', 'tabulated-list-get-id',
>   'tabulated-list-get-entry', 'tabulated-list-header-overlay-p',
>   'tabulated-list-put-tag', and 'tabulated-list-set-col'.

This should mention the bug number.

> +@defun tabulated-list-delete-entry
> +This function deletes the entry at point.
> +
> +It returns a list @code{(id cols)} where @var{id} is the ID of the
                                     ^
A comma is missing there.

> +delete entry and @var{cols} is a vector of its column descriptors.
               ^
And here.

Also, the "id" and "cols" inside the list should also have the @var
markup.

> +It moves point to the beginning of the deleted entry.

The last sentence is confusing: if the entry is deleted, how can we
move to its beginning?

> +@defun tabulated-list-header-overlay-p &optional POS
> +This @code{defsubst} returns non-nil if there is a fake header at
> +@var{pos}.

We should explain, in a single sentence if possible, what is a "fake
header".  It is never explained in this section.

> +@defun tabulated-list-put-tag tag &optional advance
> +This function puts @var{tag} in the padding area of the current line.

And this should explain what is the padding area, for the same reason.

> +@var{tag} should be a string, with a length less than or equal to
> +@code{tabulated-list-padding}.

Every variable mentioned in the manual should be indexed.  So please
add

 @vindex tabulated-list-padding

before the @defun.

> +If @var{change-entry-data} is non-nil, this function modifies the
> +underlying entry data by setting the appropriate slot of the vector
> +originally used to print this entry.  If @code{tabulated-list-entries}
> +has a list value, this is the vector stored within it.

This paragraph is confusing, I cannot understand what that argument
does just by reading the above text.  (The doc string says the same,
so it's of no help.)  The confusing parts are "appropriate slot" and
"originally used to print".  The code simply modifies a component of
the vector returned by tabulated-list-get-entry, so I wonder why the
description needs to be that complicated.

Thanks again for working on this.





  reply	other threads:[~2019-02-02 16:33 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-16  9:05 bug#21074: 25.0.50; Incomplete docs for tabulated-list-mode Marcin Borkowski
2019-01-15 18:19 ` bug#21074: [PATCH] Add docs for two tabulated-list functions Alex Branham
2019-01-15 19:06   ` Eli Zaretskii
2019-01-15 19:41     ` Alex Branham
2019-01-19  8:25       ` Eli Zaretskii
2019-01-21 16:12         ` Alex Branham
2019-01-21 16:32           ` Eli Zaretskii
2019-01-22 21:03         ` Alex Branham
2019-02-01  9:28           ` Eli Zaretskii
2019-02-02 16:03             ` Alex Branham
2019-02-02 16:33               ` Eli Zaretskii [this message]
2019-02-02 17:28                 ` Alex Branham
2019-02-02 18:06                   ` Eli Zaretskii
2019-02-05 20:08                     ` Alex Branham
2019-02-05 20:26                       ` Eli Zaretskii
2019-02-05 20:50                         ` Alex Branham
2019-02-06  3:32                           ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83h8dmw3pe.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=21074@debbugs.gnu.org \
    --cc=alex.branham@gmail.com \
    --cc=mbork@mbork.pl \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.