bug#48883: [External] : bug#48883: dired marking bugs

[Boruch Baum <boruch_baum@gmx.com>]

[-- Attachment #1: Type: text/plain, Size: 3934 bytes --]

On 2021-06-06 22:12, Drew Adams wrote:
> >    1.1) The correct and expected behavior of dired is that when one
> >    ...
> >    1.2) Likewise, typing 'u' on that line unmarks all said lines.
>
> (You don't say so, but I think/hope you agree that
> 1.1 and 1.2 are OK, not something improper.)

Agreed.

>
> >    1.3) Now, advance one line, to the line typically presenting a
> >         summary, beginning with the word 'total', and press the sequence
> >         'C-u -1 m'.
> >
> >    1.4) Why is there now a mark on the prior line, the directory line?
> >         Note that the result differs from actually performing 'm' on
> >         that line also in that none of the "real" files in the directory
> >         are marked. Note also that performing 'u' on the line does not
> >         remove the mark, but performing 'U' on the buffer does.

> This change to `dired-repeat-over-lines' seems to
> take care of that OK:
>
> In the second `while' (the one for negative movement),
> change (save-excursion (funcall function)) to this:
> (when (dired-get-filename nil t) ; <=======
>   (save-excursion (funcall function)))

I don't like that change because it allows operations on '.' and '..'
which makes me uncomfortable.

I have a patch in testing, but I want to do some more work before
submitting it (and who knows, some emacs devel may insist none of this
is a bug!).

What I see as possibly the issue is function `dired-between-files'. I'm
not equipped to perform a 'git blame', but the function's comment
indicated a change at some point, and I suspect that's when the bug
arose.

> >    1.5) Attempting to perform an operation on the marked directory line
> >         (eg. 'C' to copy it) returns the message "No files specified"
>
> IMO, that's not a problem.  (I assume here you're
> talking about point being on the dir heading line
> and no (other) files being marked.)  There _are_
> (should be) no files specified.

Agreed. I should have made that clear.

> IMO, there's nothing wrong with marking `.' or `..',
> and nothing wrong with Dired having some operations
> that work on them.

I don't feel comfortable with that. Do you have any specific case in
mind?

> > 3) Improper advancing to (point-max)
> >
> >    3.1) Marking the final entry in a dired buffer advances POINT to a
> >         blank line, which is pretty much never desirable to a user.
>
> I don't see that as a problem.

I agree that it isn't a *problem*, just that it's not friendly.

OK. I've changed my mind, and am submitting my current patch. The reason
I thought not to above is that I might want to propose changing function
`dired-between-files'. However, from performing M-x rzgrep on the lisp
directory, I see it referred to 9 times, and I haven't checked them yet:

./dired.el.gz:3498:      (while (and (not (eobp)) (dired-between-files)) (forward-line 1))
./dired.el.gz:3509:      (while (and (not (bobp)) (dired-between-files)) (forward-line -1))
./dired.el.gz:3515:(defun dired-between-files ()
./dired.el.gz:3570:      (while (and (< (point) end) (dired-between-files))
./dired.el.gz:3652:        (or (dired-between-files)
./files.el.gz:6944:;;		 dired-between-files, (shortcut for (not (dired-move-to-filename)))
./net/ange-ftp.el.gz:5440:;;(or (assq 'vms ange-ftp-dired-between-files-alist)
./net/ange-ftp.el.gz:5441:;;    (setq ange-ftp-dired-between-files-alist
./net/ange-ftp.el.gz:5443:;;		ange-ftp-dired-between-files-alist)))

Also, because dired is such an old feature of emacs, and since it is the
subject of so many third-party[1] extensions, any *fix* to that function,
even if correct, might have undesirable consequences downstream, so I
want to careful about it.


[1] I know I keep saying 'third-party' but I have no idea who is the second-party.

--
hkp://keys.gnupg.net
CA45 09B5 5351 7C11 A9D1  7286 0036 9E45 1595 8BC0

[-- Attachment #2: dired.patch --]
[-- Type: text/x-diff, Size: 1557 bytes --]

(defun dired-repeat-over-lines (arg function)
  ;; This version skips non-file lines.
  (let ((pos (make-marker)))
    (beginning-of-line)
    (cond
     ((> arg 0)
       (while (and (> arg 0) (not (eobp)))
         (setq arg (1- arg))
         (beginning-of-line)
       ;;(while (and (not (eobp)) (dired-between-files)) (forward-line 1))
         (while (and (not (eobp))
                     (condition-case nil
                       (not (dired-get-filename))
                       (error t)))
           (forward-line 1))
         (save-excursion
           (forward-line 1)
           (move-marker pos (1+ (point))))
         (save-excursion (funcall function))
         ;; Advance to the next line--actually, to the line that *was* next.
         ;; (If FUNCTION inserted some new lines in between, skip them.)
         (goto-char pos))
       (when (eobp)
         (forward-line -1)
         (dired-move-to-filename)))
     ((< arg 0)
       (while (and (< arg 0) (not (bobp)))
         (setq arg (1+ arg))
         (forward-line -1)
       ;;(while (and (not (bobp)) (dired-between-files)) (forward-line -1))
         (while (and (not (bobp))
                     (condition-case nil
                       (not (dired-get-filename))
                       (error t)))
           (forward-line -1))
         (beginning-of-line)
         (when (condition-case nil
                 (dired-get-filename)
                 (error nil))
           (save-excursion (funcall function))))
       (move-marker pos nil)
       (dired-move-to-filename)))))