unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
From: Drew Adams <drew.adams@oracle.com>
To: Boruch Baum <boruch_baum@gmx.com>
Cc: "47957@debbugs.gnu.org" <47957@debbugs.gnu.org>
Subject: bug#47957: [External] : bug#47957: diredx-aux doctrings [PATCH INCLUDED]
Date: Sun, 25 Apr 2021 18:21:04 +0000	[thread overview]
Message-ID: <SA2PR10MB4474F9ABEEC70B6FF99A8E01F3439@SA2PR10MB4474.namprd10.prod.outlook.com> (raw)
In-Reply-To: <20210425052212.ao4ugrbsbloxnkon@E15-2016.optimum.net>

> > Thanks for doing this.
> 
> Quite welcome. Let's see what it takes to get merged...
> 
> > A minor suggestion (for consideration) for `dired-do-kill-lines'
> > ...
> > Say "remove", not "kill" (which generally has to do with removing
> > ...
> > (Also, don't forget to prefix lines that start with ( with \.)
> 
> Done. Updated patch attached.

Some feedback, for consideration.  I see now, after writing
all this, that Eli also provided feedback.  HTH, anyway.

1.
+Operates on marked files and refresh their file lines.
                              refreshes

Start each <ARG NAME> is... on its own line, including
OP-SYMBOL and ARG.

2.
+"Process all the files in FILES...

  Process FILES...

+Uses (FUNCALL FUNCTION ARGS... SOME-FILES...).

 Applies FUNCTION to ARGS and SOME-FILES, where SOME-FILES
 is a batch of at most MAX files.

(The comment in the code was wrong.)

3.
+"Make up a shell command line from 

  Return a shell command line derived from

+If ON-EACH is t, COMMAND should be applied to each file, else
+simply concat all files and apply COMMAND to this.
+FILE-LIST's elements will be quoted for the shell.

 By default, apply COMMAND to all files together.
 Non-nil ON-EACH means apply COMMAND to each file instead.

 Quote the files in FILE-LIST for the shell.

+Might be redefined for smarter things and could then use RAW-ARG
+(coming from interactive P and currently ignored) to decide what to do.
+Smart would be a way to access basename or extension of file names."

Remove that - I think it should just remain a code comment.

4.
You indicate that you applied my suggestion to speak of
removing, rather than killing, lines.  But I don't see
that you did that, at all.

5.
+"Compress or uncompress the current file.
+Return nil for success, offending filename else."

  Compress or uncompress the file named on the current line.
  Return nil for success or the file name for failure.

6.
+"Request confirmation from the user for an operation on marked-files.
+The operation is described by OP-SYMBOL. Confirmation consists in
+a y-or-n question with a file list pop-up unless OP-SYMBOL is a
+member of `dired-no-confirm'.

  Request confirmation from user for an operation on the marked files.
  OP-SYMBOL is a symbol describing the operation performed (e.g.
  `compress').

  If `dired-no-confirm' is t or OP-SYMBOL is a member of list
  `dired-no-confirm' then this is a no-op: no confirmation is needed.
  Otherwise, confirmation is requested with `y-or-n-p'.

7.
FWIW: For `dired-map-over-marks-check' I use this (adapted
by removing mention of added &rest arg FUN-ARGS and function
`diredp-map-over-marks-and-report'):

Map FUN over marked lines and report failures.
FUN should return nil for success and non-nil (the offending object,
e.g. the short form of the filename) for a failure.  FUN can log a
detailed error explanation using `dired-log'.

MARK-ARG is as the second argument of `dired-map-over-marks'.

OP-SYMBOL is a symbol describing the operation performed (e.g.
`compress').  It is used with `dired-mark-pop-up' to prompt the user
\(e.g. with `Compress * [2 files]? ') and to display errors (e.g.
`Failed to compress 1 of 2 files - type ? for details (\"foo\")')

SHOW-PROGRESS if non-nil means redisplay Dired after each file.

FUN-ARGS is the list of any remaining args to
`dired-map-over-marks-check'.  Function FUN is applied to these
arguments.

8.
+  "Byte-compile file at POINT.

and

+  "Load file at POINT.

POINT shouldn't be uppercase.  And it shouldn't be mentioned.
The cursor need not be on the file name.

Use something like this instead:

   Byte-compile the file named on the current line.

9.
+If FILE is nil, then just delete the current line. Keeps any
+marks that may be present in column one (doing this here is
+faster than with dired-add-entry's optional arg). Does not update
+other dired buffers. Use dired-relist-entry for that."

 If FILE is nil, then just delete the current line.
 Keep any mark in column one.

 This does not update other dired buffers.
 \(Use `dired-relist-entry' for that.)

10.
+  "Return pos of first file line of DIR.
    Return the position of the first file line of DIR.
    DIR is assumed to be visible, not hidden.

+Header lines and total or wildcard lines are skipped. Important:
+never moves into the next subdir. DIR is assumed to be unhidden."

    This never moves into the next subdir listing.

(No need to say what's skipped, as it says "first file line".)

11.
+  "A fluid var used in dired-handle-overwrite.
+It should be let-bound whenever dired-copy-file etc are called.
+See `dired-create-files' for an example.")

I don't know what this is - it was apparently added after Emacs 27.
But the first line should not say what it says.  The first line
should say what the variable does, is, or represents.  Presumably
its value controls or defines how overwriting is handled.

This doc string is not good, IMO.  Maybe the variable itself
is ill-advised; dunno.  Maybe that info is helpful as a code
comment; dunno.

12.
What happened to a doc string for `dired-rename-subdir'?  That's
more important than doc strings for `*-1' and `*-2'.  The latter
doc strings should say these are helper functions for the main fn.

13.
+"Rename DIR to TO in header lines and dired-subdir-alist, if DIR
+or one of its subdirectories is expanded in this buffer."

The args should have the same names as in `dired-rename-subdir'
(so a code change also).

  Helper for `dired-rename-subdir'.
  Rename only if FROM-DIR or one of its subdirectories is...

(Requires study of the code.  Sorry, no time.)

14.
+  "Update the headerline and dired-subdir-alist element, as well
+as dired-switches-alist element, of directory described by
+alist-element ELT to reflect the moving of DIR to TO. Thus, ELT
+describes either DIR itself or a subdir of DIR."

Start with "Helper for `dired-rename-subdir-1'."

headerline -> header line

   ELT is an element of `dired-subdir-alist'.  It describes
   either FROM-DIR or one of its subdirectories.

15.
+  "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."

    Use `dired-mark-pop-up' to read a file name.
    PROMPT, ARG, and FILES are used for prompting.
     PROMPT is a `format' string that is applied to the result of applying
     `dired-mark-prompt' to ARG and FILES.
    OP-SYMBOL, FILES, DIR, and DEFAULT are passed to `dired-mark-pop-up'.

16.
+  "Return directories from all next visible windows with dired-mode buffers."
    Return a list of directories from visible `next-window' Dired buffers.
    
17.
+  "Return directories from all visible windows with dired-mode
+buffers ordered by most-recently-used."

    Return a list of directories from visible recent Dired buffers.

18.
+  "Try to guess which target directory the user may want.
+If there is a dired buffer displayed in one of the next windows,
+use its current subdir, else use current subdir of this dired buffer."

    Return a directory the user might want as a target.
    Prefer the current subdir of a Dired `next-window' buffer.
    If none, return the current subdir of this buffer.

19.
+  "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
+to exclude from the returned list, for the case when this
+directory name is already presented in initial input.
+For Dired operations that support `dired-dwim-target',
+the argument `target-dir' should have the value returned
+from `dired-dwim-target-directory'."

Change arg FN-LIST to FILES.  Then it can be used in the doc
with no other description.

   Return a list of default values for file-reading functions.
   This can include directories from Dired buffers in other windows.

   If FILES is a singleton list (FILE) then the return value can
    include this or other directories with FILE appended, as well
    as just other directories.
   If FILES is not a singleton list then the return value includes
    only directories.

Sorry, but I've run out of time to spend on this.  For #19,
TARGET-DIR needs to be described properly.  The comment doesn't
do that, AFAICT.

HTH.  Can't spend more time on this.  Hopefully someone else can
take a look.  It's not enough to move the comments to doc strings,
I think.





  parent reply	other threads:[~2021-04-25 18:21 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
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     ` Drew Adams [this message]
2021-04-25 19:38       ` bug#47957: [External] : bug#47957: diredx-aux doctrings [PATCH INCLUDED] 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=SA2PR10MB4474F9ABEEC70B6FF99A8E01F3439@SA2PR10MB4474.namprd10.prod.outlook.com \
    --to=drew.adams@oracle.com \
    --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).