unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#65621: [PATCH] `dired-next-line' go to meaningful line
@ 2023-08-30 13:02 Shynur Xie
  2023-08-30 13:36 ` Eli Zaretskii
  2023-08-30 14:54 ` Drew Adams
  0 siblings, 2 replies; 34+ messages in thread
From: Shynur Xie @ 2023-08-30 13:02 UTC (permalink / raw)
  To: 65621

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

Cursor in Dired buffer sometimes go to a line which is not an item
line.  For example, the 1st line and the last line:

   > d:/Desktop/.emacs.d/site-lisp:
       drwxrwxrwx shynur 4096 08/30 18:15 .
       drwxrwxrwx shynur 4096 08/30 18:07 ..
       -rw-rw-rw- shynur  304 08/09  3:01 subdirs.el
   > \Newline here.

Avoiding these lines may improve the experience when moving cursor.
The attaching patch implements this.

If there is no visible item line, move the cursor as Dired used to do.

[-- Attachment #2: 0001-dired-next-line-go-to-meaningful-line.patch --]
[-- Type: application/octet-stream, Size: 2625 bytes --]

From c580e0f502eaa9180a5b97bb3f57d8ee4aed51db Mon Sep 17 00:00:00 2001
From: shynur <one.last.kiss@outlook.com>
Date: Wed, 30 Aug 2023 19:47:52 +0800
Subject: [PATCH] `dired-next-line' go to meaningful line

---
 lisp/dired.el | 42 ++++++++++++++++++++++++++++++++----------
 1 file changed, 32 insertions(+), 10 deletions(-)

diff --git a/lisp/dired.el b/lisp/dired.el
index e96b85a..3b5753e 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2666,22 +2666,44 @@ Otherwise, toggle `read-only-mode'."
       (wdired-change-to-wdired-mode)
     (read-only-mode 'toggle)))
 
+(defun dired-filename-line-p ()
+  "Return t if the current line is a filename line."
+  (save-excursion
+    (dired-move-to-filename)
+    (get-char-property (point) 'dired-filename)))
+
 (defun dired-next-line (arg)
   "Move down lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
+Optional prefix ARG says how many lines to move; default is one line.
+
+Point won't go to the dired-header line or the last empty line.  If
+you really want to move there, use `next-line' instead."
   (interactive "^p")
-  (let ((line-move-visual)
-	(goal-column))
-    (line-move arg t))
-  ;; We never want to move point into an invisible line.
-  (while (and (invisible-p (point))
-	      (not (if (and arg (< arg 0)) (bobp) (eobp))))
-    (forward-char (if (and arg (< arg 0)) -1 1)))
-  (dired-move-to-filename))
+  (let ((old-line-has-filename (dired-filename-line-p)))
+    (let ((line-move-visual)
+          (goal-column))
+      (line-move arg t))
+    ;; We never want to move point into an invisible line.
+    (while (and (invisible-p (point))
+                (not (if (and arg (< arg 0)) (bobp) (eobp))))
+      (forward-char (if (and arg (< arg 0)) -1 1)))
+    (dired-move-to-filename)
+    ;; If there's a line (or one of its succeeding lines) that we can
+    ;; go back to,
+    (when old-line-has-filename
+      ;; and the current line doesn't contain a filename,
+      (unless (dired-filename-line-p)
+        ;; then let's move back.
+        (dired-next-line (if (natnump arg)
+                             -1
+                           1))))))
 
 (defun dired-previous-line (arg)
   "Move up lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
+Optional prefix ARG says how many lines to move; default is one line.
+
+Point won't go to the dired-header line or the last empty line.  If
+you really want to move there, use `previous-line' instead."
   (interactive "^p")
   (dired-next-line (- (or arg 1))))
 
-- 
2.41.0.windows.3


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 13:02 bug#65621: [PATCH] `dired-next-line' go to meaningful line Shynur Xie
@ 2023-08-30 13:36 ` Eli Zaretskii
  2023-08-30 13:50   ` Shynur Xie
  2023-08-30 14:54 ` Drew Adams
  1 sibling, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-08-30 13:36 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 65621

> From: Shynur Xie <one.last.kiss@outlook.com>
> Date: Wed, 30 Aug 2023 13:02:43 +0000
> 
> Cursor in Dired buffer sometimes go to a line which is not an item
> line.  For example, the 1st line and the last line:
> 
>    > d:/Desktop/.emacs.d/site-lisp:
>        drwxrwxrwx shynur 4096 08/30 18:15 .
>        drwxrwxrwx shynur 4096 08/30 18:07 ..
>        -rw-rw-rw- shynur  304 08/09  3:01 subdirs.el
>    > \Newline here.
> 
> Avoiding these lines may improve the experience when moving cursor.
> The attaching patch implements this.
> 
> If there is no visible item line, move the cursor as Dired used to do.

Thanks, but I personally fail to see why people would like this.  It
basically means that if I want to copy those "forbidden" lines to the
kill ring or the clipboard, I can't (except by disabling this
feature).  Is it really a good idea?





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 13:36 ` Eli Zaretskii
@ 2023-08-30 13:50   ` Shynur Xie
  2023-08-30 14:20     ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-08-30 13:50 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 65621@debbugs.gnu.org

> It basically means that if I want to copy those "forbidden" lines to
> the kill ring or the clipboard, I can't (except by disabling this
> feature).

Just use mouse, `{forward,backward}-{char,word}',
`{previous,next}-line', etc.  Anyway, I just changed the definition of
`dired-next-line', so actually users have many ways to do what they
want.

> I personally fail to see why people would like this.

Isn't it great that no matter how you move up or down, you never leave
the item line?  Without this, if you move the cursor to the last line
accidentally, to enter one of those files, you need to press one more
keystroke.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 13:50   ` Shynur Xie
@ 2023-08-30 14:20     ` Eli Zaretskii
  2023-08-30 19:14       ` Stefan Kangas
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-08-30 14:20 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 65621

> From: Shynur Xie <one.last.kiss@outlook.com>
> CC: "65621@debbugs.gnu.org" <65621@debbugs.gnu.org>
> Date: Wed, 30 Aug 2023 13:50:34 +0000
> msip_labels:
> 
> > It basically means that if I want to copy those "forbidden" lines to
> > the kill ring or the clipboard, I can't (except by disabling this
> > feature).
> 
> Just use mouse, `{forward,backward}-{char,word}',
> `{previous,next}-line', etc.  Anyway, I just changed the definition of
> `dired-next-line', so actually users have many ways to do what they
> want.
> 
> > I personally fail to see why people would like this.
> 
> Isn't it great that no matter how you move up or down, you never leave
> the item line?  Without this, if you move the cursor to the last line
> accidentally, to enter one of those files, you need to press one more
> keystroke.

It doesn't bother me, but I will let others chime in and speak their
opinions.  If enough people like this feature, we could install it,
but I think even then it will need to be an opt-in feature.






^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 13:02 bug#65621: [PATCH] `dired-next-line' go to meaningful line Shynur Xie
  2023-08-30 13:36 ` Eli Zaretskii
@ 2023-08-30 14:54 ` Drew Adams
  2023-08-30 15:39   ` Shynur Xie
  1 sibling, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-08-30 14:54 UTC (permalink / raw)
  To: Shynur Xie, 65621@debbugs.gnu.org

Be sure to consider also the behavior when 
subdir listings are inserted in the buffer.
(Didn't try your patch; just sayin'.)
___

FWIW, I think that the right way to deal
with this is what's done in `dired+.el':

1. There's an option, `diredp-wrap-around-flag',
which controls wrapping around to the buffer
beginning/end when using `n'/`p'.

That is, it says whether moving down when
there's no file/dired/header line further down
wraps to the beginning (first such line in the
buffer), and similarly when moving up when
there's no such line above.

2. Commands `dired-(next|previous)-line' have
their keys remapped to commands that respect
the option (`diredp-(next|previous)-line').

Simple, useful, IMO.  (Likely suggested
before to `emacs-devel' but rejected.)
___

Commands `diredp-(next|previous)-line' also
have the improvement that they respect var
`goal-column': If non-nil then they put the
cursor at that column.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 14:54 ` Drew Adams
@ 2023-08-30 15:39   ` Shynur Xie
  2023-08-30 15:55     ` Drew Adams
  0 siblings, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-08-30 15:39 UTC (permalink / raw)
  To: Drew Adams, Eli Zaretskii; +Cc: 65621@debbugs.gnu.org

> Dradams:
> Be sure to consider also the behavior when  subdir listings are
> inserted in the buffer.  (Didn't try your patch; just sayin'.)

Thanks.

> Dradams:
> I think that the right way to deal with this is what's done in
> `dired+.el'.

Sounds good; I'll try it (and add it to my configuration after I learn
how to fetch a package from a URL to integrate third-party package
with my package management system).

> Eli:
> If enough people like this feature, we could install it, but I think
> even then it will need to be an opt-in feature.

It's OK.  If they do like it, I'll rewrite the patch.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 15:39   ` Shynur Xie
@ 2023-08-30 15:55     ` Drew Adams
  0 siblings, 0 replies; 34+ messages in thread
From: Drew Adams @ 2023-08-30 15:55 UTC (permalink / raw)
  To: Shynur Xie, Eli Zaretskii; +Cc: 65621@debbugs.gnu.org

> > I think that the right way to deal with this is what's done in
> > `dired+.el'.
> 
> Sounds good; I'll try it (and add it to my configuration after I learn
> how to fetch a package from a URL to integrate third-party package
> with my package management system).

Just save file `dired+.el' locally, byte-compile
it (optional), put the directory where you saved
it into your `load-path`, and `(require 'dired+)'.

Or just `M-x load-library' or `M-x load-file', to
try it out.

Or just copy the definitions of the option and
commands and try them out.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 14:20     ` Eli Zaretskii
@ 2023-08-30 19:14       ` Stefan Kangas
  2023-08-30 20:58         ` Drew Adams
  2023-08-31  5:45         ` Shynur Xie
  0 siblings, 2 replies; 34+ messages in thread
From: Stefan Kangas @ 2023-08-30 19:14 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: Shynur Xie, 65621

Eli Zaretskii <eliz@gnu.org> writes:

> > Isn't it great that no matter how you move up or down, you never leave
> > the item line?  Without this, if you move the cursor to the last line
> > accidentally, to enter one of those files, you need to press one more
> > keystroke.
>
> It doesn't bother me, but I will let others chime in and speak their
> opinions.  If enough people like this feature, we could install it,
> but I think even then it will need to be an opt-in feature.

I'd probably enable it if we had such an option, FWIW.

But Drew's point about inserted subdirs would need to be addressed.
My ideal behavior in that case is that it jumps to the file in the
next/previous directory.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 19:14       ` Stefan Kangas
@ 2023-08-30 20:58         ` Drew Adams
  2023-08-30 21:34           ` Stefan Kangas
  2023-08-31  5:45         ` Shynur Xie
  1 sibling, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-08-30 20:58 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii; +Cc: Shynur Xie, 65621@debbugs.gnu.org

> I'd probably enable it if we had such an option, FWIW.
> 
> But Drew's point about inserted subdirs would need to be addressed.
> My ideal behavior in that case is that it jumps to the file in the
> next/previous directory.

IMO, _definitely not_.  At most it should move
to the subdir header line, skipping only the
blank line before it.

The dired+.el optional wraparound behavior I
described moves, like the vanilla behavior, to
the next line.  It just wraps around, so it
doesn't move to the last line of the buffer
(which is empty).

It could be argued that the blank lines that
precede headings of inserted subdirs could be
skipped over.  But it should _definitely_ move
to the header lines themselves.

There are lots of Dired operations that you can
use on directory header lines.  `m', for example,
marks each line shown in the subdir listing.



^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 20:58         ` Drew Adams
@ 2023-08-30 21:34           ` Stefan Kangas
  2023-08-30 22:36             ` Drew Adams
  0 siblings, 1 reply; 34+ messages in thread
From: Stefan Kangas @ 2023-08-30 21:34 UTC (permalink / raw)
  To: Drew Adams; +Cc: Eli Zaretskii, Shynur Xie, 65621@debbugs.gnu.org

Drew Adams <drew.adams@oracle.com> writes:

> > But Drew's point about inserted subdirs would need to be addressed.
> > My ideal behavior in that case is that it jumps to the file in the
> > next/previous directory.
>
> IMO, _definitely not_.  At most it should move
> to the subdir header line, skipping only the
> blank line before it.

OK, makes sense.

> The dired+.el optional wraparound behavior I
> described moves, like the vanilla behavior, to
> the next line.  It just wraps around, so it
> doesn't move to the last line of the buffer
> (which is empty).

I find wraparound rather disorienting myself, but I can see why one
might find it useful.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 21:34           ` Stefan Kangas
@ 2023-08-30 22:36             ` Drew Adams
  0 siblings, 0 replies; 34+ messages in thread
From: Drew Adams @ 2023-08-30 22:36 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Eli Zaretskii, Shynur Xie, 65621@debbugs.gnu.org

> > > But Drew's point about inserted subdirs would need to be addressed.
> > > My ideal behavior in that case is that it jumps to the file in the
> > > next/previous directory.
> >
> > IMO, _definitely not_.  At most it should move
> > to the subdir header line, skipping only the
> > blank line before it.
> 
> OK, makes sense.
> 
> > The dired+.el optional wraparound behavior I
> > described moves, like the vanilla behavior, to
> > the next line.  It just wraps around, so it
> > doesn't move to the last line of the buffer
> > (which is empty).
> 
> I find wraparound rather disorienting myself,
> but I can see why one might find it useful.

It's an _option_.  That said, I can't imagine
wanting to go to the last (empty) line in the
buffer.  

^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-30 19:14       ` Stefan Kangas
  2023-08-30 20:58         ` Drew Adams
@ 2023-08-31  5:45         ` Shynur Xie
  2023-08-31 15:35           ` Drew Adams
  1 sibling, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-08-31  5:45 UTC (permalink / raw)
  To: Stefan Kangas, Eli Zaretskii, Drew Adams; +Cc: 65621@debbugs.gnu.org

> Stefan:
> I'd probably enable it if we had such an option, FWIW.

Thank you!

> Stefan:
> But Drew's point about inserted subdirs would need to be addressed.
> My ideal behavior in that case is that it jumps to the file in the
> next/previous directory.

> Dradams:
> At most it should move to the subdir header line, skipping only the
> blank line before it.

Let's make thing simple:

    I'll fix the inserting subdirs case, and make this an optional
    feature which is disabled by default.

> Dradams:
> It could be argued that the blank lines that precede headings of
> inserted subdirs could be skipped over.  But it should _definitely_
> move to the header lines themselves.

You can move anywhere you want _easily_ no matter whether this feature
is enabled.  As I said before:

> shynur:
> Just use mouse, `{forward,backward}-{char,word}',
> `{previous,next}-line', etc.  Anyway, I just changed the definition
> of `dired-next-line', so actually users have many ways to do what
> they want.

___

I've started rewriting; this shouldn't be too difficult.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31  5:45         ` Shynur Xie
@ 2023-08-31 15:35           ` Drew Adams
  2023-08-31 15:46             ` Shynur Xie
  0 siblings, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-08-31 15:35 UTC (permalink / raw)
  To: Shynur Xie, Stefan Kangas, Eli Zaretskii; +Cc: 65621@debbugs.gnu.org

> > Dradams:
> > It could be argued that the blank lines that precede headings of
> > inserted subdirs could be skipped over.  But it should _definitely_
> > move to the header lines themselves.
> 
> You can move anywhere you want _easily_ no matter whether
> this feature is enabled.  As I said before:
> 
> > Just use mouse, `{forward,backward}-{char,word}',
> > `{previous,next}-line', etc.  Anyway, I just changed the definition
> > of `dired-next-line', so actually users have many ways to do what
> > they want.

No, no, no.  `dired-(next|previous)-line' should
move to header lines, as well as to file/dir lines.
This is important.

As I said, users can perform actions (I gave the
example of `m') on header lines, just as they can
on file/dir lines.  And in many cases, they can
invoke the _same_ commands (e.g. `dired-mark',
bound to `m'), often with the meaning of applying
to all files/dirs in the listing for that header.

Let's not change Dired willy nilly.  Let's please
learn it well enough to take into account its
existing (and longstanding) behavior in some area,
before opting to change it.

In this case: there's a reason we have `n' and `p'
bound to Dired-specific commands.  Navigation
destination should generally be somewhere you can
do something Dired-specific.  Put differently, it
should _at least include_ places where you can do
something Dired-specific.

Same thing for other Dired navigation commands,
such as `>', `C-M-n', `^', `C-M-u', and `i'.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 15:35           ` Drew Adams
@ 2023-08-31 15:46             ` Shynur Xie
  2023-08-31 15:55               ` Drew Adams
  0 siblings, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-08-31 15:46 UTC (permalink / raw)
  To: Drew Adams, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

> Dradams
> there's a reason we have `n' and `p' bound to Dired-specific
> commands.

Make sense.
Now, we agree that some lines should be skipped, and we disagree on
whether header lines should be skipped.

My patch is almost done.  I think it will make you happy - it doesn't
control only _whether_ to skip lines, instead it says how to skip line
(e.g., move circularly like you mentioned before).  If you really want
to go to the header line:
  1. set it to nil;
  2. add an new meaningful value for this option.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 15:46             ` Shynur Xie
@ 2023-08-31 15:55               ` Drew Adams
  2023-08-31 18:15                 ` Shynur Xie
  0 siblings, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-08-31 15:55 UTC (permalink / raw)
  To: Shynur Xie, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

> > there's a reason we have `n' and `p' bound to Dired-specific
> > commands.
> 
> Make sense.
> Now, we agree that some lines should be skipped, and we disagree on
> whether header lines should be skipped.
> 
> My patch is almost done.  I think it will make you happy - it doesn't
> control only _whether_ to skip lines, instead it says how to skip line
> (e.g., move circularly like you mentioned before).  If you really want
> to go to the header line:
>   1. set it to nil;
>   2. add an new meaningful value for this option.

If you really DON'T want `n'|`p' to go to
header lines then maybe set some option.

The _default_ behavior should go to any
Dired line that you can act on, which
includes header lines.

Otherwise, this is a real step backward,
not forward.  Currently `n'|`p' take you
to any line, including a header line.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 15:55               ` Drew Adams
@ 2023-08-31 18:15                 ` Shynur Xie
  2023-08-31 19:10                   ` Drew Adams
  2023-08-31 21:35                   ` Stefan Kangas
  0 siblings, 2 replies; 34+ messages in thread
From: Shynur Xie @ 2023-08-31 18:15 UTC (permalink / raw)
  To: Eli Zaretskii, Drew Adams, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

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

The final patch is attached.  (See the end for how to test it.)

> Eli:
> it will need to be an opt-in feature.

By default, it's disabled.

> Dradams:
> Be sure to consider also the behavior when subdir listings are
> inserted in the buffer.

Considered.

> Stefan:
> But Drew's point about inserted subdirs would need to be addressed.

Addressed.

> Stefan
> I'd probably enable it if we had such an option, FWIW.

Stefan likes my original patch, so please set
`dired-cursor-goto-meaningful-line' to `bounded' and
`dired-headerline-is-meaningful' to nil.

> Dradams:
> it should move to the subdir header line, skipping only the blank
> line before it.
> `dired-(next|previous)-line' should move to header lines, as well as
> to file/dir lines.
> If you really DON'T want `n'|`p' to go to header lines then maybe
> set some option.

Have taken your advice.  You like what your Dired+ does, so please set
`dired-cursor-goto-meaningful-line' to `cycle' and
`dired-headerline-is-meaningful' to t.

___

- To Test It:

There is totally 4 (not 6) kinds of combinations of the 2 new options:

  1. (setq dired-cursor-goto-meaningful-line 'bounded
           dired-headerline-is-meaningful nil)
  2. (setq dired-cursor-goto-meaningful-line 'cycle
           dired-headerline-is-meaningful nil)
  3. (setq dired-cursor-goto-meaningful-line 'bounded
           dired-headerline-is-meaningful t)
  4. (setq dired-cursor-goto-meaningful-line 'cycle
           dired-headerline-is-meaningful t)

Test them in 3 kinds of dired buffers:

  1. regular dired buffer
  2. buffer inserted subdirs
  3. the option `dired-listing-switches' is an empty string, the
     directory is empty, and there is no subdir inserted.  I.e., there
     is no filename lines.  Only empty lines or header lines.

Move cursor from anywhere.

[-- Attachment #2: 0001-dired-next-line-go-to-meaningful-line.patch --]
[-- Type: application/octet-stream, Size: 5554 bytes --]

From 8ddfb99a9213414e438fc4946ce3935b21b6ca3d Mon Sep 17 00:00:00 2001
From: shynur <one.last.kiss@outlook.com>
Date: Fri, 1 Sep 2023 02:12:25 +0800
Subject: [PATCH] `dired-next-line' go to meaningful line

---
 lisp/dired.el | 95 +++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 88 insertions(+), 7 deletions(-)

diff --git a/lisp/dired.el b/lisp/dired.el
index e96b85a..3d7d6a6 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -495,6 +495,30 @@ to nil: a pipe using `zcat' or `gunzip -c' will be used."
                  (string :tag "Switches"))
   :version "29.1")
 
+(defcustom dired-cursor-goto-meaningful-line nil
+  "Non-nil means moving cursor only to a pathname line.
+This option makes `dired-next-line' and `dired-previous-line' move
+cursor to:
+ - a line containing filename if such a line does exist.
+ - or, a header line if `dired-headerline-is-meaningful' is non-nil.
+
+Possible non-nil values:
+ *   `cycle': the next/previous line of the last/first line is the
+              first/last line.
+ * `bounded': cursor cannot move up/down if the current line is the
+              first/last line.
+ * any other symbol: the effect is the same as `bounded', but it is
+              not recommended for use."
+  :type '(choice (const :tag "Move to any line" nil)
+                 (const :tag "Loop through pathname lines" cycle)
+                 (const :tag "Only to pathname line" bounded))
+  :group 'dired)
+
+(defcustom dired-headerline-is-meaningful t
+  "Non-nil means never skip header line when moving cursor."
+  :type 'boolean
+  :group 'dired)
+
 (defcustom dired-hide-details-preserved-columns nil
   "List of columns which are not hidden in `dired-hide-details-mode'."
   :type '(repeat integer)
@@ -2666,22 +2690,79 @@ Otherwise, toggle `read-only-mode'."
       (wdired-change-to-wdired-mode)
     (read-only-mode 'toggle)))
 
-(defun dired-next-line (arg)
-  "Move down lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
-  (interactive "^p")
+(defun dired--trivial-next-line (arg)
+  "Move down ARG lines then position at filename."
   (let ((line-move-visual)
-	(goal-column))
+    (goal-column))
     (line-move arg t))
   ;; We never want to move point into an invisible line.
   (while (and (invisible-p (point))
-	      (not (if (and arg (< arg 0)) (bobp) (eobp))))
+          (not (if (and arg (< arg 0)) (bobp) (eobp))))
     (forward-char (if (and arg (< arg 0)) -1 1)))
   (dired-move-to-filename))
 
+(defun dired--meaningful-line-p ()
+  "Return t if the current line contains a filename, or is a header
+line if `dired-headerline-is-meaningful' is non-nil."
+  (save-excursion
+    ;; Move to BOL or filename.
+    (dired-move-to-filename)
+    (or (get-char-property (point) 'dired-filename)
+        (when dired-headerline-is-meaningful
+          (skip-chars-forward "[[:blank:]]")
+          (eq (get-text-property (point) 'face) 'dired-header)))))
+
+(defun dired-next-line (arg)
+  "Move down lines then position at filename.
+Optional prefix ARG says how many lines to move; default is one line.
+
+Cursor won't go to empty line when `dired-cursor-goto-meaningful-line'
+is non-nil; further, it also skips dired header lines if
+`dired-headerline-is-meaningful' is nil."
+  (interactive "^p")
+  (if dired-cursor-goto-meaningful-line
+      (let* ((old-position (progn
+                             ;; It's always true that we should move
+                             ;; to the filename when possible.
+                             (dired-move-to-filename)
+                             (point)))
+             ;; Up/Down indicates the direction.
+             (moving-down (if (natnump arg)
+                              1   ; means Down.
+                            -1))  ; means Up.
+             ;; The following while-loop runs this many times at
+             ;; most, unless there is not even a meaningful line,
+             ;; but this is almost impossible.
+             (total-moves (* 3 arg moving-down)))
+        ;; Line by line in case we forget to skip meaningless lines.
+        (while (and (not (zerop arg))
+                    ;; If there's no meaningful lines, stop the
+                    ;; endless while-loop.
+                    (natnump total-moves))
+          (cl-decf total-moves)
+          (dired--trivial-next-line moving-down)
+          (when (= old-position (point))
+            ;; Now cursor is at beginning/end of buffer,
+            ;; but the cursor still wants to move farther.
+            (if (eq dired-cursor-goto-meaningful-line 'cycle)
+                (goto-char (if (= 1 moving-down)
+                               (point-min)
+                             (point-max)))
+              (unless (dired--meaningful-line-p)
+                (dired--trivial-next-line (- moving-down))
+                (setq arg moving-down))))
+          (setq old-position (point))
+          (when (dired--meaningful-line-p)
+            (cl-decf arg moving-down))))
+    (dired--trivial-next-line arg)))
+
 (defun dired-previous-line (arg)
   "Move up lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
+Optional prefix ARG says how many lines to move; default is one line.
+
+Cursor won't go to empty line when `dired-cursor-goto-meaningful-line'
+is non-nil; further, it also skips dired header lines if
+`dired-headerline-is-meaningful' is nil."
   (interactive "^p")
   (dired-next-line (- (or arg 1))))
 
-- 
2.41.0.windows.3


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 18:15                 ` Shynur Xie
@ 2023-08-31 19:10                   ` Drew Adams
  2023-08-31 19:17                     ` Drew Adams
  2023-08-31 19:44                     ` Shynur Xie
  2023-08-31 21:35                   ` Stefan Kangas
  1 sibling, 2 replies; 34+ messages in thread
From: Drew Adams @ 2023-08-31 19:10 UTC (permalink / raw)
  To: Shynur Xie, Eli Zaretskii, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

> Stefan likes my original patch, so please set
> `dired-cursor-goto-meaningful-line' to `bounded' and
> `dired-headerline-is-meaningful' to nil.
> 
> > Dradams:
> > it should move to the subdir header line, skipping
> > only the blank line before it.
> > `dired-(next|previous)-line' should move to header
> > lines, as well as to file/dir lines.
> > If you really DON'T want `n'|`p' to go to header
> > lines then maybe set some option.
> 
> Have taken your advice.

No, you haven't.  Not as far as I can see.

> You like what your Dired+ does, so please set
> `dired-cursor-goto-meaningful-line' to `cycle'
> and `dired-headerline-is-meaningful' to t.

No thanks.

Based on your description and the definition
of `dired-cursor-goto-meaningful-line' in your
patch, I disagree strongly with this change.

Currently users need not do anything to be able
to move to file, directory, and dir-header lines
with `n' and `p'.  Those lines are as actionable
as any others; they should not be skipped over.

That behavior should remain the default.  Users
should not need to do anything to have `n' and
`p' move to header lines.  Making users customize
an option just to get the longstanding (and only
useful) behavior would be a big step backward.

And it would be for no gain.  AFAIK, no reason
has been given why `n' and `p' should skip over
header lines.

`dired-cursor-goto-meaningful-line' must default
to `t'.

From my point of view it need not (should not)
even be offered as an option: the `t' behavior
should just be all there is.

Have you - has anyone - given any reason at all
why `n' and `p' should ever skip header lines?
I don't think so.

(And there's no need for "cursor" in the option
name.  That's implicit in going to a line.)





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 19:10                   ` Drew Adams
@ 2023-08-31 19:17                     ` Drew Adams
  2023-08-31 19:44                     ` Shynur Xie
  1 sibling, 0 replies; 34+ messages in thread
From: Drew Adams @ 2023-08-31 19:17 UTC (permalink / raw)
  To: Drew Adams, Shynur Xie, Eli Zaretskii, Stefan Kangas
  Cc: 65621@debbugs.gnu.org

It's bad enough that some users (apparently
including some folks writing in this thread)
don't know about operating on a whole dir
listing through its header line.

It would be far worse if Emacs were to make
discovery of such a great feature more
difficult.  The fact that `n' and `p' move
to header lines - especially if they skip
over blank lines - invites users to discover
that they can act there on a whole listing.






^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 19:10                   ` Drew Adams
  2023-08-31 19:17                     ` Drew Adams
@ 2023-08-31 19:44                     ` Shynur Xie
  2023-08-31 21:51                       ` Drew Adams
  1 sibling, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-08-31 19:44 UTC (permalink / raw)
  To: Drew Adams, Eli Zaretskii, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

Your words:

> Drew:
> If you really DON'T want `n'|`p' to go to header lines then maybe
> set some option.

Yours, too:

> Drew:
> Those lines are as actionable as any others; they should not be
> skipped over.
> `dired-cursor-goto-meaningful-line' must default to `t'.  From my
> point of view it need not (should not) even be offered as an option:
> the `t' behavior should just be all there is.

Can you please not be inconsistent?

Again, that is an optional feature!

That's all I can say, and that patch is the final patch.
If you're still not satisfied, I'm sorry that there's nothing more I
can do, so please don't reply to me anymore.

Sorry again.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 18:15                 ` Shynur Xie
  2023-08-31 19:10                   ` Drew Adams
@ 2023-08-31 21:35                   ` Stefan Kangas
  2023-08-31 21:37                     ` Stefan Kangas
  2023-09-01 17:29                     ` Shynur Xie
  1 sibling, 2 replies; 34+ messages in thread
From: Stefan Kangas @ 2023-08-31 21:35 UTC (permalink / raw)
  To: Shynur Xie; +Cc: Eli Zaretskii, Drew Adams, 65621@debbugs.gnu.org

Shynur Xie <one.last.kiss@outlook.com> writes:

> The final patch is attached.  (See the end for how to test it.)

Thanks.

> There is totally 4 (not 6) kinds of combinations of the 2 new options:
>
>   1. (setq dired-cursor-goto-meaningful-line 'bounded
>            dired-headerline-is-meaningful nil)
>   2. (setq dired-cursor-goto-meaningful-line 'cycle
>            dired-headerline-is-meaningful nil)
>   3. (setq dired-cursor-goto-meaningful-line 'bounded
>            dired-headerline-is-meaningful t)
>   4. (setq dired-cursor-goto-meaningful-line 'cycle
>            dired-headerline-is-meaningful t)

I'll review your patch in detail as soon as I can find some time.
Just two initial comments:

I like the option `dired-cursor-goto-meaningful-line', which seems to
both add wrap-around and make the feature as a whole optional.  But in
Emacs, we say "point" rather than "cursor", so we'd have to change
that.  I suggest naming it `dired-movement-style', or something along
those lines.

But is `dired-headerline-is-meaningful' really needed?  It seems like
an unnecessary complication.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 21:35                   ` Stefan Kangas
@ 2023-08-31 21:37                     ` Stefan Kangas
  2023-09-01 17:29                     ` Shynur Xie
  1 sibling, 0 replies; 34+ messages in thread
From: Stefan Kangas @ 2023-08-31 21:37 UTC (permalink / raw)
  To: Shynur Xie; +Cc: Eli Zaretskii, Drew Adams, 65621@debbugs.gnu.org

Stefan Kangas <stefankangas@gmail.com> writes:

> I'll review your patch in detail as soon as I can find some time.
> Just two initial comments:

This would also need an entry in etc/NEWS.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 19:44                     ` Shynur Xie
@ 2023-08-31 21:51                       ` Drew Adams
  0 siblings, 0 replies; 34+ messages in thread
From: Drew Adams @ 2023-08-31 21:51 UTC (permalink / raw)
  To: Shynur Xie, Eli Zaretskii, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

1. I see now that the default value of
`dired-headerline-is-meaningful' is t.
For some reason I thought you had it as
nil.  I thought you were changing the
default behavior so it skipped header
lines.  My bad; sorry.

2. Doc nits:

The doc of `dired-headerline-is-meaningful' 
should say that it affects only commands
`dired-(next|previous)-line', instead of
giving the impression that it affects
navigation in general ("when moving cursor").
There are many ways to move the cursor, and
there are even several other Dired-specific
ways.

The doc of `dired-(next|previous)-line'
should refer to the behavior variants or
to `dired-cursor-goto-meaningful-line'.

3. FWIW, for vanilla Dired, lines `.'
and `..' are generally "meaningless"
in your terms: you generally can't act
on them.  (In Dired+ you can.)  Dunno
whether you want to include optionally
skipping over them with `n'|`p', i.e.,
via `dired-cursor-goto-meaningful-line.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-08-31 21:35                   ` Stefan Kangas
  2023-08-31 21:37                     ` Stefan Kangas
@ 2023-09-01 17:29                     ` Shynur Xie
  2023-09-01 18:46                       ` Drew Adams
  1 sibling, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-09-01 17:29 UTC (permalink / raw)
  To: Stefan Kangas, Drew Adams; +Cc: 65621@debbugs.gnu.org

> Stefan:
> I like the option `dired-cursor-goto-meaningful-line', which seems
> to both add wrap-around and make the feature as a whole optional.

Thx!  Yes, to use the `wrap-around' you mentioned, just set the option
to `cycle'.

> Stefan:
> But in Emacs, we say "point" rather than "cursor", so we'd have to
> change that.  I suggest naming it `dired-movement-style'.

OK.

> Drew:
> there's no need for "cursor" in the option name.

I'll try not to mention cursor/point in docstring when possible.

> Drew:
> The doc should say that it affects only commands
> `dired-(next|previous)-line'.

Yeah, docstring should state it.

> Drew:
> FWIW, for vanilla Dired, lines `.' and `..' are generally
> "meaningless".

You can discard them by setting `dired-listing-switches' to an
appropriate value if you don't like them.  The value is passed to the
program `ls'.  On MS-Windows things seem to be a little different, not
sure.

___

To sum up, new patch will provide an option which controls the style
of moving point.

--
Responses may be delayed as I have classes during the day
(UTC+8, not 0, contrary to what my email program indicates).




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-01 17:29                     ` Shynur Xie
@ 2023-09-01 18:46                       ` Drew Adams
  2023-09-01 20:51                         ` Shynur Xie
  0 siblings, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-09-01 18:46 UTC (permalink / raw)
  To: Shynur Xie, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

> You can discard them by setting `dired-listing-switches' to an
> appropriate value if you don't like them.  The value is passed to the
> program `ls'.  On MS-Windows things seem to be a little different, not
> sure.

Sorry; I wasn't clear.  It wasn't about not
listing `.' and `..'.  I meant that vanilla
Emacs sometimes doesn't let you apply actions
to those lines.

For example, `dired-toggle-marks' doesn't act
on them.  (Dired+ has an option to decide
whether to do so.)

Anyway, this isn't important here - vanilla
`n' and `p' don't skip over `.' and `..'.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-01 18:46                       ` Drew Adams
@ 2023-09-01 20:51                         ` Shynur Xie
  2023-09-01 21:06                           ` Drew Adams
  0 siblings, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-09-01 20:51 UTC (permalink / raw)
  To: Drew Adams, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

Let's use this option to settle our differences:

    (defcustom dired-movement-style '((move . bounded)
                                      (skip-pathname
                                       . (empty-line ...
                                          "regexp1" "regexp2" ...)))
      "..."
      ...)
    ;; 1. change `bounded' to `cycle' to use the 'wrap-around'.
    ;; 2. symbol (empty-line or hearder) appears in the second CDR
    ;;    means skip the corresponding lines.
    ;; 3. if the pathname matches any regexp, that line will be
    ;;    skipped.

Drew wants to skip the current directory and the parent directory;
someone (one of my classmates) said I should make it skip LICENSE
because this file won't be touched again in the subsequent development
process;
I speculate that some people will also suggest that ".git" should be
skipped.  They may say most people will never delve into this folder,
though it needs to be displayed to indicate that the current directory
is a Git repository (although there are other ways to remind of this).

So I decided to withdraw from the argument.  Making decisions for
users has no benefits for me.  People have their own thoughts; just
let them set the option themselves.

Any suggestion, Stefan and Drew?




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-01 20:51                         ` Shynur Xie
@ 2023-09-01 21:06                           ` Drew Adams
  2023-09-02 12:05                             ` Shynur Xie
  0 siblings, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-09-01 21:06 UTC (permalink / raw)
  To: Shynur Xie, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

> Drew wants to skip the current directory and the parent directory;

No, he doesn't.  Not at all.  Definitely not.
I think maybe you misread what I wrote.

What I said was that for _vanilla_ Emacs some
actions aren't allowed on `.' and `..'.  And
so based on that, you or someone else (NOT I),
might want those lines, in addition to blank
lines, to be skipped over as "meaningless".

My main point in this thread is that directory
header lines (main dir and subdirs) are _not_
meaningless.  You _can_ perform actions on them,
so they should not be skipped when navigating.

(If you must make it possible to skip them as
an option, so be it.  But they shouldn't be
skipped by default, IMO.)

FWIW, Dired+ doesn't skip any lines, except
the final empty line at eob.

> someone (one of my classmates) said I should make it skip LICENSE
> because this file won't be touched again in the subsequent development
> process;
> I speculate that some people will also suggest that ".git" should be
> skipped.  They may say most people will never delve into this folder,
> though it needs to be displayed to indicate that the current directory
> is a Git repository (although there are other ways to remind of this).
> 
> So I decided to withdraw from the argument.  Making decisions for
> users has no benefits for me.  People have their own thoughts; just
> let them set the option themselves.
> 
> Any suggestion, Stefan and Drew?

In addition to the clarification I offered above,
I'd say that none of the complications you've
offered with this latest suggestion are helpful.
Let's keep it simple.  If users file enhancement
requests to allow other behaviors those can be
considered later.

I'd suggest that skipping over blank lines, if
you want to do that, is enough.  And offering
cycling is a nice-to-have, but is orthogonal to
the bug report.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-01 21:06                           ` Drew Adams
@ 2023-09-02 12:05                             ` Shynur Xie
  2023-09-02 12:13                               ` Eli Zaretskii
  2023-09-03 21:47                               ` Drew Adams
  0 siblings, 2 replies; 34+ messages in thread
From: Shynur Xie @ 2023-09-02 12:05 UTC (permalink / raw)
  To: Drew Adams, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

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

New patch is done.  Only 2 features: skipping empty lines; cycling.

___

> Drew:
> > Drew wants to skip the current directory and the parent directory;
> No, he doesn't.  Not at all.  Definitely not.  I think maybe you
> misread what I wrote.

You really confused me.  If you don't want it, why mentioned it twice?

You proposed this idea:

> Drew:
> What I said was that for _vanilla_ Emacs some actions aren't allowed
> on `.' and `..'.  And so based on that, you or someone else (NOT I),
> might want those lines, in addition to blank lines, to be skipped
> over as "meaningless".

and then rejected it:

> Drew:
> I'd say that none of the complications you've offered with this
> latest suggestion are helpful.

Inconsistent.

If you don't want it and you don't want anyone else to use it, then
do not mention it in the first place.

[-- Attachment #2: 0001-dired-next-line-movement-style.patch --]
[-- Type: application/octet-stream, Size: 5098 bytes --]

From 4ec5ac2e9e868fef3112b000661e2b5cb7eb9a25 Mon Sep 17 00:00:00 2001
From: shynur <one.last.kiss@outlook.com>
Date: Fri, 1 Sep 2023 02:12:25 +0800
Subject: [PATCH] `dired-next-line' movement style

Point will skip empty lines and optionally goto the other
end when encountering a boundary.
* lisp/dired.el (dired-movement-style): Control whether to
skip empty lines and whether to cycle through non-empty
lines.
* lisp/dired.el (dired-next-line): Add new movement styles
controlled by `dired-movement-style'.
---
 lisp/dired.el | 75 ++++++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 68 insertions(+), 7 deletions(-)

diff --git a/lisp/dired.el b/lisp/dired.el
index e96b85a..8c1320d 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -495,6 +495,20 @@ to nil: a pipe using `zcat' or `gunzip -c' will be used."
                  (string :tag "Switches"))
   :version "29.1")
 
+(defcustom dired-movement-style nil
+  "Non-nil means point skips empty lines when moving.
+This affects only `dired-next-line' and `dired-previous-line'.
+
+Possible non-nil values:
+ *   `cycle': the next/previous line of the last/first line is the
+              first/last line.
+ * `bounded': cursor cannot move up/down if the current line is the
+              first/last line."
+  :type '(choice (const :tag "Move to any line" nil)
+                 (const :tag "Loop through non-empty lines" cycle)
+                 (const :tag "Only to non-empty line" bounded))
+  :group 'dired)
+
 (defcustom dired-hide-details-preserved-columns nil
   "List of columns which are not hidden in `dired-hide-details-mode'."
   :type '(repeat integer)
@@ -2666,22 +2680,69 @@ Otherwise, toggle `read-only-mode'."
       (wdired-change-to-wdired-mode)
     (read-only-mode 'toggle)))
 
-(defun dired-next-line (arg)
-  "Move down lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
-  (interactive "^p")
+(defun dired--trivial-next-line (arg)
+  "Move down ARG lines then position at filename."
   (let ((line-move-visual)
-	(goal-column))
+    (goal-column))
     (line-move arg t))
   ;; We never want to move point into an invisible line.
   (while (and (invisible-p (point))
-	      (not (if (and arg (< arg 0)) (bobp) (eobp))))
+          (not (if (and arg (< arg 0)) (bobp) (eobp))))
     (forward-char (if (and arg (< arg 0)) -1 1)))
   (dired-move-to-filename))
 
+(defun dired-next-line (arg)
+  "Move down lines then position at filename.
+Optional prefix ARG says how many lines to move; default is one line.
+
+Whether to skip empty lines and how to move when encountering a
+boundary are controlled by `dired-movement-style'."
+  (interactive "^p")
+  (if dired-movement-style
+      (let ((old-position (progn
+                            ;; It's always true that we should move
+                            ;; to the filename when possible.
+                            (dired-move-to-filename)
+                            (point)))
+            ;; Up/Down indicates the direction.
+            (moving-down (if (cl-plusp arg)
+                             1    ; means Down.
+                           -1)))  ; means Up.
+        ;; Line by line in case we forget to skip empty lines.
+        (while (not (zerop arg))
+          (dired--trivial-next-line moving-down)
+          (when (= old-position (point))
+            ;; Now point is at beginning/end of movable area,
+            ;; but it still wants to move farther.
+            (if (eq dired-movement-style 'cycle)
+                ;; `cycle': go to the other end.
+                (goto-char (if (cl-plusp moving-down)
+                               (point-min)
+                             (point-max)))
+              ;; `bounded': go back to the last non-empty line.
+              (while (string-match-p "\\`[[:blank:]]*\\'"
+                                     (buffer-substring-no-properties
+                                      (line-beginning-position)
+                                      (line-end-position)))
+                (dired--trivial-next-line (- moving-down)))
+              ;; Encountered a boundary, so let's stop movement.
+              (setq arg moving-down)))
+          (when (not (string-match-p "\\`[[:blank:]]*\\'"
+                                     (buffer-substring
+                                      (line-beginning-position)
+                                      (line-end-position))))
+            ;; Has moved to a non-empty line.  This movement does
+            ;; make sense.
+            (cl-decf arg moving-down))
+          (setq old-position (point))))
+    (dired--trivial-next-line arg)))
+
 (defun dired-previous-line (arg)
   "Move up lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
+Optional prefix ARG says how many lines to move; default is one line.
+
+Whether to skip empty lines and how to move when encountering a
+boundary are controlled by `dired-movement-style'."
   (interactive "^p")
   (dired-next-line (- (or arg 1))))
 
-- 
2.41.0.windows.3


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-02 12:05                             ` Shynur Xie
@ 2023-09-02 12:13                               ` Eli Zaretskii
  2023-09-02 12:18                                 ` Shynur Xie
  2023-09-03 21:47                               ` Drew Adams
  1 sibling, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-09-02 12:13 UTC (permalink / raw)
  To: Shynur Xie; +Cc: stefankangas, drew.adams, 65621

> Cc: "65621@debbugs.gnu.org" <65621@debbugs.gnu.org>
> From: Shynur Xie <one.last.kiss@outlook.com>
> Date: Sat, 2 Sep 2023 12:05:23 +0000
> 
> New patch is done.  Only 2 features: skipping empty lines; cycling.

Thanks, but I believe Stefan asked for a NEWS entry.  Would you like
to write one and add it to the patch?





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-02 12:13                               ` Eli Zaretskii
@ 2023-09-02 12:18                                 ` Shynur Xie
  2023-09-02 12:39                                   ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-09-02 12:18 UTC (permalink / raw)
  To: Eli Zaretskii, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

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

> Would you like to write one and add it to the patch?

I thought it was the maintainers' job.
Yes, I will write it.

[-- Attachment #2: Type: text/html, Size: 605 bytes --]

^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-02 12:18                                 ` Shynur Xie
@ 2023-09-02 12:39                                   ` Eli Zaretskii
  2023-09-02 14:40                                     ` Shynur Xie
  0 siblings, 1 reply; 34+ messages in thread
From: Eli Zaretskii @ 2023-09-02 12:39 UTC (permalink / raw)
  To: Shynur Xie; +Cc: stefankangas, 65621

> From: Shynur Xie <one.last.kiss@outlook.com>
> CC: "65621@debbugs.gnu.org" <65621@debbugs.gnu.org>
> Date: Sat, 2 Sep 2023 12:18:11 +0000
> 
> > Would you like to write one and add it to the patch?
> 
> I thought it was the maintainers' job.
> Yes, I will write it.

Thank you.

We prefer that the documentation accompanies the code changes.  If
nothing else, this enlarges the group of people who can write good
documentation for Emacs.  It also lowers the probability that we will
forget to document changes.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-02 12:39                                   ` Eli Zaretskii
@ 2023-09-02 14:40                                     ` Shynur Xie
  2023-09-10  7:45                                       ` Eli Zaretskii
  0 siblings, 1 reply; 34+ messages in thread
From: Shynur Xie @ 2023-09-02 14:40 UTC (permalink / raw)
  To: Eli Zaretskii, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

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

> We prefer that the documentation accompanies the code changes.  If
> nothing else, this enlarges the group of people who can write good
> documentation for Emacs.  It also lowers the probability that we
> will forget to document changes.

Understood.  This is my first time to write a NEWS entry, so it may
not be considered good documentation.

New patch is attached.  Thanks in advance for reviewing.

[-- Attachment #2: 0001-dired-next-line-movement-style.patch --]
[-- Type: application/octet-stream, Size: 5840 bytes --]

From d2927f014f5fb9eed8bb0fdf7c88c77a269a35d1 Mon Sep 17 00:00:00 2001
From: shynur <one.last.kiss@outlook.com>
Date: Sat, 2 Sep 2023 22:39:00 +0800
Subject: [PATCH] `dired-next-line' movement style

Point will skips empty lines and optionally goto the other
end when encountering a boundary.
* lisp/dired.el (dired-movement-style): Control whether to
skip empty lines and whether to cycle through non-empty
lines.
* lisp/dired.el (dired-next-line): Add a new movement style
controlled by `dired-movement-style'.
* etc/NEWS (dired-movement-style):
---
 etc/NEWS      |  9 ++++++
 lisp/dired.el | 76 ++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 78 insertions(+), 7 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 9a98db8..962e96c 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -282,6 +282,15 @@ This allows changing which type of whitespace changes are ignored when
 regenerating hunks with 'diff-ignore-whitespace-hunk'.  Defaults to
 the previously hard-coded "-b".
 
+** Dired
+
+---
+*** New user option 'dired-movement-style'.
+When non-nil, make 'dired-next-line' and 'dired-previous-line' skip
+empty lines.  It also controls how to move point when encountering a
+boundary (e.g., if every line is visible, invoking 'dired-next-line'
+at the last line will move to the first line).
+
 ** Ediff
 
 ---
diff --git a/lisp/dired.el b/lisp/dired.el
index e96b85a..88f8563 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -495,6 +495,21 @@ to nil: a pipe using `zcat' or `gunzip -c' will be used."
                  (string :tag "Switches"))
   :version "29.1")
 
+(defcustom dired-movement-style nil
+  "Non-nil means point skips empty lines when moving.
+This affects only `dired-next-line' and `dired-previous-line'.
+
+Possible non-nil values:
+ *   `cycle': the next/previous line of the last/first visible line is
+              the first/last visible line.
+ * `bounded': cannot move up/down if the current line is the
+              first/last visible line."
+  :type '(choice (const :tag "Move to any line" nil)
+                 (const :tag "Loop through non-empty lines" cycle)
+                 (const :tag "Only to non-empty line" bounded))
+  :group 'dired
+  :version "30.1")
+
 (defcustom dired-hide-details-preserved-columns nil
   "List of columns which are not hidden in `dired-hide-details-mode'."
   :type '(repeat integer)
@@ -2666,22 +2681,69 @@ Otherwise, toggle `read-only-mode'."
       (wdired-change-to-wdired-mode)
     (read-only-mode 'toggle)))
 
-(defun dired-next-line (arg)
-  "Move down lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
-  (interactive "^p")
+(defun dired--trivial-next-line (arg)
+  "Move down ARG lines then position at filename."
   (let ((line-move-visual)
-	(goal-column))
+    (goal-column))
     (line-move arg t))
   ;; We never want to move point into an invisible line.
   (while (and (invisible-p (point))
-	      (not (if (and arg (< arg 0)) (bobp) (eobp))))
+          (not (if (and arg (< arg 0)) (bobp) (eobp))))
     (forward-char (if (and arg (< arg 0)) -1 1)))
   (dired-move-to-filename))
 
+(defun dired-next-line (arg)
+  "Move down lines then position at filename.
+Optional prefix ARG says how many lines to move; default is one line.
+
+Whether to skip empty lines and how to move when encountering a
+boundary are controlled by `dired-movement-style'."
+  (interactive "^p")
+  (if dired-movement-style
+      (let ((old-position (progn
+                            ;; It's always true that we should move
+                            ;; to the filename when possible.
+                            (dired-move-to-filename)
+                            (point)))
+            ;; Up/Down indicates the direction.
+            (moving-down (if (cl-plusp arg)
+                             1    ; means Down.
+                           -1)))  ; means Up.
+        ;; Line by line in case we forget to skip empty lines.
+        (while (not (zerop arg))
+          (dired--trivial-next-line moving-down)
+          (when (= old-position (point))
+            ;; Now point is at beginning/end of movable area,
+            ;; but it still wants to move farther.
+            (if (eq dired-movement-style 'cycle)
+                ;; `cycle': go to the other end.
+                (goto-char (if (cl-plusp moving-down)
+                               (point-min)
+                             (point-max)))
+              ;; `bounded': go back to the last non-empty line.
+              (while (string-match-p "\\`[[:blank:]]*\\'"
+                                     (buffer-substring-no-properties
+                                      (line-beginning-position)
+                                      (line-end-position)))
+                (dired--trivial-next-line (- moving-down)))
+              ;; Encountered a boundary, so let's stop movement.
+              (setq arg moving-down)))
+          (when (not (string-match-p "\\`[[:blank:]]*\\'"
+                                     (buffer-substring-no-properties
+                                      (line-beginning-position)
+                                      (line-end-position))))
+            ;; Has moved to a non-empty line.  This movement does
+            ;; make sense.
+            (cl-decf arg moving-down))
+          (setq old-position (point))))
+    (dired--trivial-next-line arg)))
+
 (defun dired-previous-line (arg)
   "Move up lines then position at filename.
-Optional prefix ARG says how many lines to move; default is one line."
+Optional prefix ARG says how many lines to move; default is one line.
+
+Whether to skip empty lines and how to move when encountering a
+boundary are controlled by `dired-movement-style'."
   (interactive "^p")
   (dired-next-line (- (or arg 1))))
 
-- 
2.41.0.windows.3


^ permalink raw reply related	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-02 12:05                             ` Shynur Xie
  2023-09-02 12:13                               ` Eli Zaretskii
@ 2023-09-03 21:47                               ` Drew Adams
  2023-09-07 15:04                                 ` Shynur Xie
  1 sibling, 1 reply; 34+ messages in thread
From: Drew Adams @ 2023-09-03 21:47 UTC (permalink / raw)
  To: Shynur Xie, Stefan Kangas; +Cc: 65621@debbugs.gnu.org

> > > Drew wants to skip the current directory
> > > and the parent directory;
> >
> > No, he doesn't.  Not at all.  Definitely not.
> > I think maybe you misread what I wrote.
> 
> You really confused me.  If you don't want it,
> why mentioned it twice?

I mentioned it only because you were defining a
Boolean option `dired-headerline-is-meaningful'.

I wanted to let you know that `.' and `..' are
also lines that you might want to let the
option skip over, because vanilla Emacs doesn't
allow actions on them sometimes.

IOW, IF you're going to have an option for
choosing which lines are "meaningful", THEN you
might want to allow for header lines too, as
another kind of line to skip.

I clearly introduced that FYI with "FWIW", and
added:

  Dunno whether you want to include optionally
  skipping over them with `n'|`p', i.e., via
  `dired-cursor-goto-meaningful-line.

Clearly I wasn't _requesting_ being able to
skip `.' and `..' lines, and a conclusion that
Drew _asked_ for that was unwarranted.  As was
the further conclusion that his supposed ask
for that conflicted with his (repeated) FYIs
that this was _not_ something he requested.

The contradiction and confusion were in your
imagination, I'm afraid.  I think I was clear
from the outset.

If it were I, I'd have done what I did in
Dired+: no option to decide what lines are
"meaningful".

I do think it's fine to skip over blank lines
(which Dired+ hasn't done).  But I saw and I
see no real need for an option such as
`dired-headerline-is-meaningful'.  (I think
Stefan said that too, and I see you've now
removed it.)

What I argued for was having `n' and `p' go
to header lines, as they always have.  To
accommodate _your_ wish to _not_ do that I
suggested you make that optional (but have
skipping such lines be opt-in).

IOW, I suggested an option to accommodate
your new behavior (skip header lines) as
well as to respect Emacs's longstanding
(and more useful) behavior of not skipping
them.

> You proposed this idea:
> 
> > What I said was that for _vanilla_
> > Emacs some actions aren't allowed
> > on `.' and `..'.  And so based on
> > that, you or someone else (NOT I),
> > might want those lines, in addition
> > to blank lines, to be skipped over
> > as "meaningless".
> 
> and then rejected it:
> 
> > I'd say that none of the complications
> > you've offered with this latest
> > suggestion are helpful.

You elided the real point I made there:

  I'd suggest that skipping over blank lines, if
  you want to do that, is enough.  And offering
  cycling is a nice-to-have, but is orthogonal to
  the bug report.

And that matches what you ended up with.

> Inconsistent.
> 
> If you don't want it and you don't want anyone else to use it, then
> do not mention it in the first place.

I mentioned it because you were looking to
have users customize the kinds of lines
they want to consider "meaningless" (and
thus skip over).

I was just trying to help you in your attempt
to do that, by offering an FYI about another
kind of line that vanilla Emacs sometimes
allows no action on.

HTH.





^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-03 21:47                               ` Drew Adams
@ 2023-09-07 15:04                                 ` Shynur Xie
  0 siblings, 0 replies; 34+ messages in thread
From: Shynur Xie @ 2023-09-07 15:04 UTC (permalink / raw)
  To: Drew Adams; +Cc: 65621@debbugs.gnu.org

> Drew:
> I think I was clear from the outset.

Yes, it's me who wasn't clear whether that is what you want.

> I do think it's fine to skip over blank lines (which Dired+ hasn't
> done).  But I saw and I see no real need for an option such as
> `dired-headerline-is-meaningful'.

Hope this time I understand your point:

  You suggest I also skip `.' and `..' if the option
  `dired-headerline-is-meaningful' is provided, though
  you don't suggest to provide such an option.

> I see you've now removed it.

Yes, it's unnecessary.

Anyway, 

> I was just trying to help you in your attempt to do that, by
> offering an FYI about another kind of line that vanilla Emacs
> sometimes allows no action on.

Thank you.




^ permalink raw reply	[flat|nested] 34+ messages in thread

* bug#65621: [PATCH] `dired-next-line' go to meaningful line
  2023-09-02 14:40                                     ` Shynur Xie
@ 2023-09-10  7:45                                       ` Eli Zaretskii
  0 siblings, 0 replies; 34+ messages in thread
From: Eli Zaretskii @ 2023-09-10  7:45 UTC (permalink / raw)
  To: Shynur Xie; +Cc: 65621-done, stefankangas

> From: Shynur Xie <one.last.kiss@outlook.com>
> CC: "65621@debbugs.gnu.org" <65621@debbugs.gnu.org>
> Date: Sat, 2 Sep 2023 14:40:29 +0000
> msip_labels:
> 
> > We prefer that the documentation accompanies the code changes.  If
> > nothing else, this enlarges the group of people who can write good
> > documentation for Emacs.  It also lowers the probability that we
> > will forget to document changes.
> 
> Understood.  This is my first time to write a NEWS entry, so it may
> not be considered good documentation.
> 
> New patch is attached.  Thanks in advance for reviewing.

Thanks, installed on the master branch, and closing the bug.





^ permalink raw reply	[flat|nested] 34+ messages in thread

end of thread, other threads:[~2023-09-10  7:45 UTC | newest]

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-30 13:02 bug#65621: [PATCH] `dired-next-line' go to meaningful line Shynur Xie
2023-08-30 13:36 ` Eli Zaretskii
2023-08-30 13:50   ` Shynur Xie
2023-08-30 14:20     ` Eli Zaretskii
2023-08-30 19:14       ` Stefan Kangas
2023-08-30 20:58         ` Drew Adams
2023-08-30 21:34           ` Stefan Kangas
2023-08-30 22:36             ` Drew Adams
2023-08-31  5:45         ` Shynur Xie
2023-08-31 15:35           ` Drew Adams
2023-08-31 15:46             ` Shynur Xie
2023-08-31 15:55               ` Drew Adams
2023-08-31 18:15                 ` Shynur Xie
2023-08-31 19:10                   ` Drew Adams
2023-08-31 19:17                     ` Drew Adams
2023-08-31 19:44                     ` Shynur Xie
2023-08-31 21:51                       ` Drew Adams
2023-08-31 21:35                   ` Stefan Kangas
2023-08-31 21:37                     ` Stefan Kangas
2023-09-01 17:29                     ` Shynur Xie
2023-09-01 18:46                       ` Drew Adams
2023-09-01 20:51                         ` Shynur Xie
2023-09-01 21:06                           ` Drew Adams
2023-09-02 12:05                             ` Shynur Xie
2023-09-02 12:13                               ` Eli Zaretskii
2023-09-02 12:18                                 ` Shynur Xie
2023-09-02 12:39                                   ` Eli Zaretskii
2023-09-02 14:40                                     ` Shynur Xie
2023-09-10  7:45                                       ` Eli Zaretskii
2023-09-03 21:47                               ` Drew Adams
2023-09-07 15:04                                 ` Shynur Xie
2023-08-30 14:54 ` Drew Adams
2023-08-30 15:39   ` Shynur Xie
2023-08-30 15:55     ` Drew Adams

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).