GNU bug report logs -
#47957
dired-aux.el: convert comments to doctrings
Previous Next
Reported by: Boruch Baum <boruch_baum <at> gmx.com>
Date: Thu, 22 Apr 2021 19:46:02 UTC
Severity: minor
Tags: wontfix
Done: Lars Ingebrigtsen <larsi <at> gnus.org>
Bug is archived. No further changes may be made.
To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 47957 in the body.
You can then email your comments to 47957 AT debbugs.gnu.org in the normal way.
Toggle the display of automated, internal messages from the tracker.
Report forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Thu, 22 Apr 2021 19:46:02 GMT)
Full text and
rfc822 format available.
Acknowledgement sent
to
Boruch Baum <boruch_baum <at> gmx.com>
:
New bug report received and forwarded. Copy sent to
bug-gnu-emacs <at> gnu.org
.
(Thu, 22 Apr 2021 19:46:02 GMT)
Full text and
rfc822 format available.
Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
File dired-aux.el had many docstrings in comment form, meaning that they
were non discoverable using `describe-function'. The attached patch
fixes that.
--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0
[dired-aux.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Thu, 22 Apr 2021 20:17:01 GMT)
Full text and
rfc822 format available.
Message #8 received at 47957 <at> debbugs.gnu.org (full text, mbox):
Thanks for doing this.
A minor suggestion (for consideration) for
`dired-do-kill-lines', based on my feeling
that "kill" here is a misleading misnomer:
Say "remove", not "kill" (which generally
has to do with removing text and placing it
on the kill ring).
FWIW, I have this doc string for it in
Dired+. Apart from the added optional arg
INIT-COUNT, the behavior is the same.
___
Remove all marked lines, or the next ARG lines.
The files or directories on those lines are _not_ deleted. Only the
Dired listing is affected. To restore the removals, use `M-x revert-buffer'.
With a numeric prefix arg, remove that many lines going forward,
starting with the current line. (A negative prefix arg removes lines
going backward.)
If you use a prefix arg to remove the line for a subdir whose listing
you have inserted into the Dired buffer, then that subdir listing is
also removed.
To remove a subdir listing _without_ removing the subdir's line in its
parent listing, go to the header line of the subdir listing and use
this command with any prefix arg.
When called from Lisp, non-nil INIT-COUNT is added to the number of
lines removed by this invocation, for the reporting message.
___
(Also, don't forget to prefix lines that start with ( with \.)
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Sun, 25 Apr 2021 05:23:01 GMT)
Full text and
rfc822 format available.
Message #11 received at 47957 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 2021-04-22 20:15, Drew Adams wrote:
> 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.
--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0
[dired-aux-2.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Sun, 25 Apr 2021 08:16:02 GMT)
Full text and
rfc822 format available.
Message #14 received at 47957 <at> debbugs.gnu.org (full text, mbox):
> Date: Sun, 25 Apr 2021 01:22:12 -0400
> From: Boruch Baum <boruch_baum <at> gmx.com>
> Cc: "47957 <at> debbugs.gnu.org" <47957 <at> 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Sun, 25 Apr 2021 09:13:01 GMT)
Full text and
rfc822 format available.
Message #17 received at 47957 <at> debbugs.gnu.org (full text, mbox):
On 2021-04-25 11:15, Eli Zaretskii wrote:
> > Date: Sun, 25 Apr 2021 01:22:12 -0400
> > From: Boruch Baum <boruch_baum <at> gmx.com>
> > Cc: "47957 <at> debbugs.gnu.org" <47957 <at> debbugs.gnu.org>
> >
> > Done. Updated patch attached.
>
> Thanks. This still needs some work to make it compatible with our
> conventions.
All your feedback is well-taken, but I'm moving on to other things with
the patch as submitted. If as an improvement over the existing file the
patch isn't good enough to be committed for someone to continue with,
so be it.
--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0
Removed tag(s) patch.
Request was from
Stefan Kangas <stefan <at> marxist.se>
to
control <at> debbugs.gnu.org
.
(Sun, 25 Apr 2021 11:03:02 GMT)
Full text and
rfc822 format available.
Changed bug title to 'dired-aux.el: convert comments to doctrings' from 'diredx-aux doctrings [PATCH INCLUDED]'
Request was from
Stefan Kangas <stefan <at> marxist.se>
to
control <at> debbugs.gnu.org
.
(Sun, 25 Apr 2021 11:03:02 GMT)
Full text and
rfc822 format available.
Severity set to 'minor' from 'normal'
Request was from
Stefan Kangas <stefan <at> marxist.se>
to
control <at> debbugs.gnu.org
.
(Sun, 25 Apr 2021 11:03:02 GMT)
Full text and
rfc822 format available.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Sun, 25 Apr 2021 18:22:02 GMT)
Full text and
rfc822 format available.
Message #26 received at 47957 <at> debbugs.gnu.org (full text, mbox):
> > 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.
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Sun, 25 Apr 2021 19:39:02 GMT)
Full text and
rfc822 format available.
Message #29 received at 47957 <at> debbugs.gnu.org (full text, mbox):
[Message part 1 (text/plain, inline)]
On 2021-04-25 18:21, Drew Adams wrote:
> 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.
Oops. Sent the wrong diff, I guess. Sorry. Here it is now.
> Sorry, but I've run out of time to spend on this.
Me too. At least for now.
> 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.
It was an easy lift to offer a big improvement, but agreed, not perfect.
My opinion is that until the work can be continued, it's desirable over
the prior state, but that's not for me to decide.
--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1 7286 0036 9E45 1595 8BC0
[dired-aux-3.patch (text/x-diff, attachment)]
Information forwarded
to
bug-gnu-emacs <at> gnu.org
:
bug#47957
; Package
emacs
.
(Mon, 03 May 2021 08:29:01 GMT)
Full text and
rfc822 format available.
Message #32 received at 47957 <at> debbugs.gnu.org (full text, mbox):
Boruch Baum <boruch_baum <at> gmx.com> writes:
> All your feedback is well-taken, but I'm moving on to other things with
> the patch as submitted. If as an improvement over the existing file the
> patch isn't good enough to be committed for someone to continue with,
> so be it.
Looking over the changes -- some look good to me, and some don't.
Converting comments to doc strings isn't a mechanical process --
comments should describe what a function does, while internal details
should be left as comments.
So I'm closing this bug report without applying the patch, because in
total I think it's a change for the negative.
--
(domestic pets only, the antidote for overdose, milk.)
bloggy blog: http://lars.ingebrigtsen.no
Added tag(s) wontfix.
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Mon, 03 May 2021 08:29:02 GMT)
Full text and
rfc822 format available.
bug closed, send any further explanations to
47957 <at> debbugs.gnu.org and Boruch Baum <boruch_baum <at> gmx.com>
Request was from
Lars Ingebrigtsen <larsi <at> gnus.org>
to
control <at> debbugs.gnu.org
.
(Mon, 03 May 2021 08:29:02 GMT)
Full text and
rfc822 format available.
bug archived.
Request was from
Debbugs Internal Request <help-debbugs <at> gnu.org>
to
internal_control <at> debbugs.gnu.org
.
(Mon, 31 May 2021 11:24:07 GMT)
Full text and
rfc822 format available.
This bug report was last modified 2 years and 303 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.