Re: wdired.el 1.7 --- Rename files editing their names in dired buffers

[Richard Stallman]

wdired.el looks clean in general; here are my specific comments.  I've
cc'd emacs-devel so that they can see the general principles I'm
following.

    ;; Theoretically, you cannot edit anything outside of the names of the
    ;; files in the wdired buffer. The return key, as well as C-j and C-o
    ;; are "unbinded" (but you can still "paste" newlines or use another
    ;; tricks to shoot yourself). If, despite these precautions, you
    ;; change something out of the names, or add or remove lines,
    ;; unpredictable renames would be done. You are warned ;-).

Shouldn't it do something more effective to prevent nonsense editing?
For instance, it could use post-command-hook to notice that newlines
have been inserted and remove them.

It could also assume that any writable newline has been inserted as
part of a file name, and really put the newline in that file name.
Maybe that is really "the right thing".

    ;; Then, you can start editing the names of the files by typing "r"
    ;; (or whatever key you choose, or M-x wdired-change-to-wdired-mode).
    ;; Use C-c C-c when finished or C-c C-k to abort.

Please change the abort command to C-c C-].  That is what rmailedit.el
uses, and it relates to the C-] command.

    (defcustom wdired-advise-functions t
      "*If non-nil some editing commands are advised to work different in
    wdired mode. These commands are `upcase-word' `capitalize-word'
    `downcase-word' `query-replace' `query-replace-regexp'
    `replace-string', and the advice makes them to ignore read-only

The use of advising in wdired.el is not a good idea, both on general
principle, and because it could mess up user functions that call these
advised functions.  I think it would be better to rebind the keys
using substitute-key-bindings.

But other more specific approaches may be better.  For downcase-word
and such, repeating M-l may work fine even though some of the calls
get an error.  So maybe nothing needs to be done for the case commands.
Have you tried it?  What happens?

For query-replace and such, we could make perform-replace understand
read-only text regions and skip them.  That may be useful in general.
Please try editing perform-replace in this way.

          (buffer-disable-undo)    ; Changing properties can be "undo-ed",
          (buffer-enable-undo))))  ; and in this way I kill this possibility.

It would be cleaner to do these jobs by operating directly
on the variable buffer-undo-list.

    (defun wdired-abort-changes ()
      "Abort changes and return to dired mode."
      (interactive)
      (wdired-change-to-dired-mode)
      (revert-buffer))

It might be cleaner to save the old contents and restore them.
That would ensure that the text in the dired buffer after the abort
is the same as it was before starting to edit.  The user may not
want to sync it with the file system at that time.

    (defun wdired-finish-edit ()
      "Finish the edition of the filenames in that buffer, performing the

"Edition" means a published version of a book; you mean "editing".
But I think here it is best to say something like "Actually rename
files based on your editing in the Dired buffer."

    (defcustom wdired-use-interactive-rename nil
      "*If non-nil, confirmation is required before changing the filenames
    edited in a wdired mode. Confirmation is required also for overwriting
    files. If nil, no confirmation is required for change the file names,
    and the variable wdired-is-ok-overwrite is used to see if it is ok
    to overwrite files without asking."

There are two problems in that doc string:

1. The first line should stand on its own.

2. Please write two spaces after the end of a sentence.

These are standard Emacs conventions; please see the Documentation
Tips node of the Emacs Lisp Manual.  Many other doc strings in
wdired.el need to be fixed in the same way.

    (defcustom wdired-is-ok-overwrite nil
      "*If non-nil the renames can overwrite files without asking,
    if wdired-use-interactive-rename is non-nil."

This has problem #1, and another problem: symbol names such as
wdired-use-interactive-rename need to be written inside `...'.
That is also a general rule; the only exceptions are t and nil.