Subject: bug#47957: [External] : bug#47957: diredx-aux doctrings [PATCH INCLUDED]

From: Eli Zaretskii <[email-masked]>
Date: Sun, 25 Apr 2021 11:15:23 +0300

> Date: Sun, 25 Apr 2021 01:22:12 -0400
> From: Boruch Baum <[email-masked]>
> Cc: "[email-masked]" <[email-masked]>
> 
> 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.