Subject: bug#76596: 30.1; dired-movement-style does not honor subdir operations

From: Charles Choi <[email-masked]>
Date: Thu, 27 Feb 2025 10:29:04 -0800

Eli - 

Thanks much for the response. I’ve manually applied the patch to my local dired.el and have verified that the expected behavior for dired-next-line and dired-previous-line to move the point on a subdir works for all values of dired-movement-style.

All my best - 

Charles

—
Charles Y. Choi, Ph.D.
[email-masked]

> On Feb 26, 2025, at 11:53 PM, Eli Zaretskii <[email-masked]> wrote:
> 
>> From: Charles Choi <[email-masked]>
>> Date: Wed, 26 Feb 2025 12:06:14 -0800
>> 
>> 
>> **** Title
>> dired-movement-style does not honor subdir operations.
>> 
>> **** Description
>> 
>> Setting dired-movement-style to a non-nil value prevents moving the point to 
>> 
>> **** Environment
>> GNU Emacs 30.1 (build 2, aarch64-apple-darwin24.3.0, NS appkit-2575.40 Version 15.3.1 (Build 24D70)) of 2025-02-25
>> i--8<---------------cut here---------------end--------------->8---
>> 
>> **** Steps to Reproduce
>> 1. Open Dired on a directory with sub-directories.
>> 2. Open subdir in Dired.
>> 3. M-x customize-variable dired-movement-style
>> 4. Set dired-movement-style value to "Cycle through non-empty lines"
>> 5. Navigate point via n, p, C-p, C-n to subdir
>> 
>> 
>> **** Expected Result
>> Point should land on subdir.
>> 
>> **** Actual Result
>> Point avoids subdir, preventing running operations on it such as removing it (C-u k).
> 
> Juri, this seems to be due to your changes in commit 29d42d9158ae:
> dired-between-files returns non-nil when on a subdirectory line, so
> Dired thinks it's an empty line.
> 
> Charles, does the patch below give good results?
> 
> Juri, do you agree with the patch?  It should go to the release
> branch.
> 
> diff --git a/lisp/dired.el b/lisp/dired.el
> index 90d944c..17c8ba5 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -2888,15 +2888,19 @@ dired--move-to-next-line
>           (setq wrapped t))
>          ;; `bounded': go back to the last non-empty line.
>          (dired-movement-style ; Either 'bounded or anything else non-nil.
> -          (while (and (dired-between-files) (not (zerop arg)))
> +          (while (and (dired-between-files)
> +                      (not (dired-get-subdir))
> +                      (not (zerop arg)))
>             (funcall jumpfun (- moving-down))
>             ;; Point not moving means infinite loop.
>             (if (= old-position (point))
>                 (setq arg 0)
>               (setq old-position (point))))
>           ;; Encountered a boundary, so let's stop movement.
> -          (setq arg (if (dired-between-files) 0 moving-down)))))
> -      (unless (dired-between-files)
> +          (setq arg (if (and (dired-between-files)
> +                             (not (dired-get-subdir)))
> +                        0 moving-down)))))
> +      (unless (and (dired-between-files) (not (dired-get-subdir)))
>         ;; Has moved to a non-empty line.  This movement does
>         ;; make sense.
>         (cl-decf arg moving-down))