unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Eli Zaretskii <eliz@gnu.org>
To: Boruch Baum <boruch_baum@gmx.com>
Cc: 47957@debbugs.gnu.org
Subject: bug#47957: [External] : bug#47957: diredx-aux doctrings [PATCH INCLUDED]
Date: Sun, 25 Apr 2021 11:15:23 +0300	[thread overview]
Message-ID: <83y2d6lqzo.fsf@gnu.org> (raw)
In-Reply-To: <20210425052212.ao4ugrbsbloxnkon@E15-2016.optimum.net> (message from Boruch Baum on Sun, 25 Apr 2021 01:22:12 -0400)

> Date: Sun, 25 Apr 2021 01:22:12 -0400
> From: Boruch Baum <boruch_baum@gmx.com>
> Cc: "47957@debbugs.gnu.org" <47957@debbugs.gnu.org>
> 
> Done. Updated patch attached.

Thanks.  This still needs some work to make it compatible with our
conventions.  First, some general issues will almost all of the doc
strings in the patch:

  . The first line of a doc string should be a complete sentence
    mentioning the mandatory arguments.
  . Two spaces between sentences, per US English conventions.
  . An opening parenthesis or grave accent in column zero should be
    escaped by a backslash.

A few specific comments:

>  (defun dired-map-dired-file-lines (fun)
> -  ;; Perform FUN with point at the end of each non-directory line.
> -  ;; FUN takes one argument, the absolute filename.
> +"Perform FUN with point at the end of each non-directory line.

"Perform FUN" sounds awkward.  I suggest "Call FUN" instead.

>  (defun dired-compress ()
> -  ;; Compress or uncompress the current file.
> -  ;; Return nil for success, offending filename else.

This and other uses of "offending" are not really a good idea.  We are
not talking about files that cause some trouble.  Suggest to find a
better word.

> -;; Read arguments for a marked-files command that wants a file name,
> -;; perhaps popping up the list of marked files.
> -;; ARG is the prefix arg and indicates whether the files came from
> -;; marks (ARG=nil) or a repeat factor (integerp ARG).
> -;; If the current file was used, the list has but one element and ARG
> -;; does not matter. (It is non-nil, non-integer in that case, namely '(4)).
> -;; DEFAULT is the default value to return if the user just hits RET;
> -;; if it is omitted or nil, then the name of the directory is used.
> -
>  (defun dired-mark-read-file-name (prompt dir op-symbol arg files
>  					 &optional default)
> +  "Read arguments for a marked-files command that wants a file name,
> +perhaps popping up the list of marked files. ARG is the prefix
> +arg and indicates whether the files came from marks (ARG=nil) or
> +a repeat factor (integerp ARG). If the current file was used, the
> +list has but one element and ARG does not matter. (It is non-nil,
> +non-integer in that case, namely '(4)). DEFAULT is the default
> +value to return if the user just hits RET; if it is omitted or
> +nil, then the name of the directory is used."

Some of the comments that you converted to doc strings are
descriptions of the implementation.  the above is one example.  I'm
not sure it is a good idea to have doc strings which do that, because
understanding how to use this function in Lisp programs is not easy
given such low-level details and nothing else.  Maybe just leave them
as comments?

> +  "Return a list of default values for file-reading functions in Dired.
> +This list may contain directories from Dired buffers in other windows.
> +`fn-list' is a list of file names used to build a list of defaults.
> +When nil or more than one element, a list of defaults will
> +contain only directory names.  `target-dir' is a directory name

This uses non-standard way of naming the function's arguments: they
should be up-cased and not quoted.

> +FILE-CREATOR and OPERATION as in dired-create-files.
> +ARG as in dired-get-marked-files.

These two sentences lack the verb ("are", I guess).

>  (defun dired-alist-sort ()
> -  ;; Keep the alist sorted on buffer position.
> +  "Keep the alist sorted on buffer position."

This references some alist without naming it.

>  (defun dired-insert-subdir-newpos (new-dir)
> -  ;; Find pos for new subdir, according to tree order.
> +  "Find pos for new subdir, according to tree order."

This doesn't mention the function's argument.





  reply	other threads:[~2021-04-25  8:15 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-22 19:45 bug#47957: diredx-aux doctrings [PATCH INCLUDED] Boruch Baum
2021-04-22 20:15 ` bug#47957: [External] : " Drew Adams
2021-04-25  5:22   ` Boruch Baum
2021-04-25  8:15     ` Eli Zaretskii [this message]
2021-04-25  9:12       ` Boruch Baum
2021-05-03  8:27         ` bug#47957: dired-aux.el: convert comments to doctrings Lars Ingebrigtsen
2021-04-25 18:21     ` bug#47957: [External] : bug#47957: diredx-aux doctrings [PATCH INCLUDED] Drew Adams
2021-04-25 19:38       ` Boruch Baum

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

  List information: https://www.gnu.org/software/emacs/

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

  git send-email \
    --in-reply-to=83y2d6lqzo.fsf@gnu.org \
    --to=eliz@gnu.org \
    --cc=47957@debbugs.gnu.org \
    --cc=boruch_baum@gmx.com \
    /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 public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).