bug#63676: cancelling editable dired causes UI problems with dired

bug#63676: cancelling editable dired causes UI problems with dired

[Peter Mao]

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

I'm testing: GNU Emacs 29.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version
3.24.33, cairo
 version 1.16.0) of 2023-05-15

Dired behavior has multiple problems if subdirectories are present in the
buffer and writable mode is entered and then cancelled

To reproduce the problem
1. Open a directory in dired
2. Insert a subdirectory with "i"
3. Enter writable mode "C-x C-q"
4. Cancel writable mode "C-c ESC"

Now the subdirectory is in the buffer, but it can't be folded or removed
with "C-u k" and  the top dir files can no longer be accessed.  After
playing around a while, I found that "g" (revert-buffer) fixes things, but
it's not the most obvious thing to do.

On the plus side, at least I *can* enter writable mode with a folded
subdirectory in dired now!

Peter

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

bug#63676: cancelling editable dired causes UI problems with dired

[Eli Zaretskii]

> From: Peter Mao <peter.mao@gmail.com>
> Date: Tue, 23 May 2023 21:51:01 -0700
> 
> I'm testing: GNU Emacs 29.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo
>  version 1.16.0) of 2023-05-15
> 
> Dired behavior has multiple problems if subdirectories are present in the buffer and writable mode is
> entered and then cancelled
> 
> To reproduce the problem
> 1. Open a directory in dired
> 2. Insert a subdirectory with "i"
> 3. Enter writable mode "C-x C-q"
> 4. Cancel writable mode "C-c ESC"
> 
> Now the subdirectory is in the buffer, but it can't be folded or removed with "C-u k" and  the top dir files
> can no longer be accessed.

I seem to be unable to reproduce this.  But your recipe lack some
details, so I'm unsure.  Would you please describe exactly what to
type after "C-c ESC" to demonstrate that some commands don't work, and
what did you expect those command to do?

Thanks.

bug#63676: cancelling editable dired causes UI problems with dired

[Stephen Berman]

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

On Wed, 24 May 2023 14:05:43 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Peter Mao <peter.mao@gmail.com>
>> Date: Tue, 23 May 2023 21:51:01 -0700
>>
>> I'm testing: GNU Emacs 29.0.91 (build 1, x86_64-pc-linux-gnu, GTK+ Version
>> 3.24.33, cairo
>>  version 1.16.0) of 2023-05-15
>>
>> Dired behavior has multiple problems if subdirectories are present in the
>> buffer and writable mode is
>> entered and then cancelled
>>
>> To reproduce the problem
>> 1. Open a directory in dired
>> 2. Insert a subdirectory with "i"
>> 3. Enter writable mode "C-x C-q"
>> 4. Cancel writable mode "C-c ESC"
>>
>> Now the subdirectory is in the buffer, but it can't be folded or removed
>> with "C-u k" and the top dir files
>> can no longer be accessed.
>
> I seem to be unable to reproduce this.  But your recipe lack some
> details, so I'm unsure.  Would you please describe exactly what to
> type after "C-c ESC" to demonstrate that some commands don't work, and
> what did you expect those command to do?

I can reproduce the problem: After step 4 above, with point within the
subdirectory, typing `$' should hide just the subdirectory, but it hides
the entire directory structure, i.e. the topmost directory and all
subdirectories.  After typing `$' to show the directory, which also
shows the previously inserted subdirectory, with point on the first line
of the subdirectory (i.e. the line containing the name of the
subdirectory), typing `C-u k' should remove the display of the
subdirectory, but it does not and instead errors with the message "Can
only kill file lines".

The OP noted that typing `g' after step 4 makes subsequently typing `$'
and `C-u k' in the subdirectory work as expected.  In fact, `g' in dired
does not call revert-buffer but dired-revert, which properly restores
subdirectories.  And indeed, with the patch below I don't see the
problems any more.

Steve Berman


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: wdired-abort-changes patch --]
[-- Type: text/x-patch, Size: 381 bytes --]

diff --git a/lisp/wdired.el b/lisp/wdired.el
index 5572dcb32f3..ac3735d636f 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -470,6 +470,7 @@ wdired-abort-changes
     (insert wdired--old-content)
     (goto-char wdired--old-point))
   (wdired-change-to-dired-mode)
+  (dired-revert)
   (set-buffer-modified-p nil)
   (setq buffer-undo-list nil)
   (message "Changes aborted"))

bug#63676: cancelling editable dired causes UI problems with dired

[Peter Mao]

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

(btw -- I did try this with "emacs -Q" and it did reproduce for me)

5. Try to fold the subdir with "$". Instead of folding the subdir,
everything folds.
6. Try to kill the subdir with "C-u k" with point on the line with the
path, message: "Can only kill file lines" (expected inserted subdir to be
removed from buffer)
7. refresh buffer with "g" -- normal operations resume.

On Wed, May 24, 2023 at 4:05 AM Eli Zaretskii <eliz@gnu.org> wrote:

> > From: Peter Mao <peter.mao@gmail.com>
> > Date: Tue, 23 May 2023 21:51:01 -0700
> >
> > I'm testing: GNU Emacs 29.0.91 (build 1, x86_64-pc-linux-gnu, GTK+
> Version 3.24.33, cairo
> >  version 1.16.0) of 2023-05-15
> >
> > Dired behavior has multiple problems if subdirectories are present in
> the buffer and writable mode is
> > entered and then cancelled
> >
> > To reproduce the problem
> > 1. Open a directory in dired
> > 2. Insert a subdirectory with "i"
> > 3. Enter writable mode "C-x C-q"
> > 4. Cancel writable mode "C-c ESC"
> >
> > Now the subdirectory is in the buffer, but it can't be folded or removed
> with "C-u k" and  the top dir files
> > can no longer be accessed.
>
> I seem to be unable to reproduce this.  But your recipe lack some
> details, so I'm unsure.  Would you please describe exactly what to
> type after "C-c ESC" to demonstrate that some commands don't work, and
> what did you expect those command to do?
>
> Thanks.
>

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

bug#63676: cancelling editable dired causes UI problems with dired

[Eli Zaretskii]

> From: Peter Mao <peter.mao@gmail.com>
> Date: Wed, 24 May 2023 18:14:08 -0700
> Cc: 63676@debbugs.gnu.org
> 
> (btw -- I did try this with "emacs -Q" and it did reproduce for me)
> 
> 5. Try to fold the subdir with "$". Instead of folding the subdir, everything folds.
> 6. Try to kill the subdir with "C-u k" with point on the line with the path, message: "Can only kill file
> lines" (expected inserted subdir to be removed from buffer)
> 7. refresh buffer with "g" -- normal operations resume.

Thanks.  So I've now installed Stephen's suggested change on the
emacs-29 branch, and I'm closing this bug.

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> I seem to be unable to reproduce this.  But your recipe lack some
> details, so I'm unsure.  Would you please describe exactly what to
> type after "C-c ESC" to demonstrate that some commands don't work, and
> what did you expect those command to do?

I hope we find a better solution than the one you installed:
`dired-revert' can be slow, and, more importantly, it looses information
like killed files or positions of that buffer when it is displayed in
other windows.  This is the least thing that one wants to happen
automatically when using writable dired.

I try to have a look to find out why this problem happens.

Michael.

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

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

Eli Zaretskii <eliz@gnu.org> writes:

> I seem to be unable to reproduce this.  But your recipe lack some
> details, so I'm unsure.  Would you please describe exactly what to
> type after "C-c ESC" to demonstrate that some commands don't work, and
> what did you expect those command to do?

I think this should be appropriate:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-Fix-cancellation-of-Wdired.patch --]
[-- Type: text/x-diff, Size: 976 bytes --]

From 59a7929f5e7653be1baa72208e2b89ef452962ea Mon Sep 17 00:00:00 2001
From: Michael Heerdegen <michael_heerdegen@web.de>
Date: Sat, 27 May 2023 02:26:09 +0200
Subject: [PATCH] Fix "Fix cancellation of Wdired"

* lisp/wdired.el (wdired-abort-changes): Call
`dired-build-subdir-alist' instead of `dired-revert'.
---
 lisp/wdired.el | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/lisp/wdired.el b/lisp/wdired.el
index 70e908b3a38..e6a7ca1841d 100644
--- a/lisp/wdired.el
+++ b/lisp/wdired.el
@@ -473,9 +473,8 @@ wdired-abort-changes
     (insert wdired--old-content)
     (goto-char wdired--old-point))
   (wdired-change-to-dired-mode)
-  ;; Make sure the display is in synch, and all the variables are set
-  ;; correctly.
-  (dired-revert)
+  ;; Update markers in `dired-subdir-alist'
+  (dired-build-subdir-alist)
   (set-buffer-modified-p nil)
   (setq buffer-undo-list nil)
   (message "Changes aborted"))
--
2.30.2


[-- Attachment #3: Type: text/plain, Size: 662 bytes --]


Background: Aborting wdired (`wdired-abort-changes') erases the
buffer and insert the original buffer contents, then re-enters
dired-mode.  Positions in `dired-subdir-alist' (that are necessary for
$) are represented as markers.  These just have to be updated.


A less invasive way of aborting wdired could just undo any user changes.
I think this should be doable using change groups.  Then we would not
loose any kind of marker positions.  `wdired-finish-edit' does not
suffer from these kind of problems because it only touches buffer parts
that correspond to changed file lines.  Currently aborting is more
invasive than actually making changes.

Michael.

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Currently aborting is more invasive than actually making changes.

Sorry again, not true - obviously I have redefined more stuff in my
config than I had remembered.

So, personally I try to avoid to revert in all wdired commands,
because reverting is slow for large buffers and loses information.  Is
there interest to get rid of reverting?  dired and wdired have all the
tools to avoid it.

Sorry again,

Michael.

bug#63676: cancelling editable dired causes UI problems with dired

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Peter Mao <peter.mao@gmail.com>,  63676@debbugs.gnu.org
> Date: Sat, 27 May 2023 04:39:33 +0200
> 
> Michael Heerdegen <michael_heerdegen@web.de> writes:
> 
> > Currently aborting is more invasive than actually making changes.
> 
> Sorry again, not true - obviously I have redefined more stuff in my
> config than I had remembered.
> 
> So, personally I try to avoid to revert in all wdired commands,
> because reverting is slow for large buffers and loses information.  Is
> there interest to get rid of reverting?  dired and wdired have all the
> tools to avoid it.

As I said: the speed of aborting is not really important, IMO.  If
reverting loses information, we should restore it after reverting.

bug#63676: cancelling editable dired causes UI problems with dired

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: Peter Mao <peter.mao@gmail.com>,  63676@debbugs.gnu.org
> Date: Sat, 27 May 2023 02:55:56 +0200
> 
> I think this should be appropriate:

Thanks, but why removal of the comment? is the comment incorrect or
inaccurate?  I think having comments that explain why we do
non-trivial things is an advantage.

> Background: Aborting wdired (`wdired-abort-changes') erases the
> buffer and insert the original buffer contents, then re-enters
> dired-mode.  Positions in `dired-subdir-alist' (that are necessary for
> $) are represented as markers.  These just have to be updated.

Are we sure this is the _only_ thing that needs to be updated?
dired-revert does much more, so we should audit what it does carefully
to determine which parts may need re-doing here.  If you did that,
would you please present the analysis and the conclusions?  In
particular, wdired-abort-changes could be called after more commands
than the original recipe shows, and that could affect other aspects of
the Dired buffer, not just dired-subdir-alist.

> A less invasive way of aborting wdired could just undo any user changes.
> I think this should be doable using change groups.  Then we would not
> loose any kind of marker positions.  `wdired-finish-edit' does not
> suffer from these kind of problems because it only touches buffer parts
> that correspond to changed file lines.  Currently aborting is more
> invasive than actually making changes.

This change was installed in the emacs-29 branch.  Any alternative
change, if we want it in Emacs 29, should be both safe (in the sense
that its code doesn't risk breaking other things) and reliable (in the
sense that it solves the original problem in its entirety).  If we can
come up with such an alternative, fine; otherwise what you propose
might be good for experimenting on the master branch, but not for the
release branch.

And having said all that, I don't really understand why we should
worry so much about the downsides of the solution: is
wdired-abort-changes something that is used a lot?  At least its speed
sounds not important at all, and if the information it loses is indeed
important enough, the way to avoid that is to restore that information
after reverting, perhaps the way wdired-finish-edit does (which, btw,
does call revert).

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> > I think this should be appropriate [...]:

First, sorry for my initial confusion.  I had forgotten where I was
using a modified version of wdired code.  But please be assured that I
did not make this suggestion lightly.

> Thanks, but why removal of the comment? is the comment incorrect or
> inaccurate?  I think having comments that explain why we do
> non-trivial things is an advantage.

I think it was inaccurate: enabling and aborting wdired does not touch
local variables (unlike major mode changes that delete locals).  We only
need to revert the things that have been changed explicitly (plus
anything the user might have done in the meantime).  The code already
does do this carefully.  Going back to a fully functional dired buffer
is not something that was not being addressed.

This issue is a bit special because it was not trivially foreseeable:
because the old buffer contents are being restored, marker positions
are shredded.  I have verified that `dired-subdir-alist' is the only
local variable carrying markers.

> > Background: Aborting wdired (`wdired-abort-changes') erases the
> > buffer and insert the original buffer contents, then re-enters
> > dired-mode.  Positions in `dired-subdir-alist' (that are necessary for
> > $) are represented as markers.  These just have to be updated.
>
> Are we sure this is the _only_ thing that needs to be updated?

> dired-revert does much more, so we should audit what it does carefully
> to determine which parts may need re-doing here.

My point is that dired-revert does _too_ much - see below.  And I'm
relatively sure that what we used to do is not problematic, so this is
not necessary (please note that the installed patch introduces a mistake
into the code: we now remember the buffer contents when entering wdired
in a variable.  When aborting, we now erase the buffer, restore those
remembered old contents, throw all of that away again, and revert from
anew).

> If you did that, would you please present the analysis and the
> conclusions?

> In particular, wdired-abort-changes could be called
> after more commands than the original recipe shows, and that could
> affect other aspects of the Dired buffer, not just dired-subdir-alist.

You only edit a text buffer then whose contents are later completely
discarded.  Dired commands are completely unavailable and/or not
functional.  Marks are part of the saved original buffer contents. Local
variables are not touched.  ATM I can't imagine what else could go
wrong.

Note that the posted recipe doesn't involve doing anything in between:
already entering wdired and immediate aborting causes the problem.  What
you do in between is irrelevant in this case.

> And having said all that, I don't really understand why we should
> worry so much about the downsides of the solution: is
> wdired-abort-changes something that is used a lot?  At least its speed
> sounds not important at all, and if the information it loses is indeed
> important enough, the way to avoid that is to restore that information
> after reverting, perhaps the way wdired-finish-edit does (which, btw,
> does call revert).

This is not a trivial matter.  Reverting, for example, also gets rid of
information, like that of `dired-do-kill-lines' calls.  Note that
`dired-revert' is existing to _undo_ such things, to get to the initial,
starting state.  That's not what the user normally wants when aborting
wdired.  This is like aborting `query-replace' would call
`revert-buffer'!  An example:

When I'm in the middle of a complex renaming operation using wdired, I
might have done some preparations before entering wdired, like killing
some unrelated lines so that it's simpler to use rectangle commands on
the rest (to rename a "block" of files).  When aborting reverts the
buffer (Say, I made a mistake ans lost orientation), I now have to do
the preparations again.  This is not helpful.

Also note that there are things that we can't easily restore at all:
markers that are not under the control of dired are lost (also with the
original code).  For example, when you remember a (file) position in
dired using a register (say, you want to rename it later, or copy it
into another buffer after doing something else) the position is saved
using a marker - after reverting (or wdired-aborting) it is gone.  Other
packages or features might use markers and overlays for other purposes
(highlighting stuff, showing thumbnails, such things).

Since wdired has developed in a direction where it only very carefully
touches the buffer at all (good! toggling wdired should only toggle the
mode of operation and leave the rest as is), the right direction, in my
opinion, is to try to avoid not explicitly user requested modifications
as much as possible.  The current patch goes into the opposite direction
:-(


I'll have another look whether something else could be broken and
everything that is part of the dired buffer's state is restored
appropriately, ok?

And of course did I not want suggest to experiment with any further
things on the release branch.


Michael.

bug#63676: cancelling editable dired causes UI problems with dired

[Thierry Volpiatto]

Hello Michael,

Michael Heerdegen <michael_heerdegen@web.de> writes:

> When I'm in the middle of a complex renaming operation using wdired, I
> might have done some preparations before entering wdired, like killing
> some unrelated lines so that it's simpler to use rectangle commands on
> the rest (to rename a "block" of files).  When aborting reverts the
> buffer (Say, I made a mistake ans lost orientation), I now have to do
> the preparations again.  This is not helpful.

You could avoid such problem by creating a dired buffer with _only_ the
files you want to modify (helm allows this), then switch to wdired-mode
and do your modifications.
Probably implementing this in dired (i.e. open a new dired buffer with
only marked files) would be a good addition.

Note: Only Emacs-30 (see commit 66040fbeed2) allow this,
Emacs-28 dired doesn't support a list of files.

-- 
Thierry

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

Thierry Volpiatto <thievol@posteo.net> writes:

> You could avoid such problem by creating a dired buffer with _only_ the
> files you want to modify (helm allows this), then switch to wdired-mode
> and do your modifications.
> Probably implementing this in dired (i.e. open a new dired buffer with
> only marked files) would be a good addition.

Thanks for mentioning this.  Yes, we definitely want something like
this.  I was experimenting with such "helper" dired buffers myself
(inspired by Icicles and Helm among others).

It's surprisingly hard to integrate such buffers into the dired concept,
though: you can't advertise them (else they would be displayed when you
"open the respective directory").  If you don't advertise them, file
changes in other buffers (like renamings) are not propagated into these
additional buffers.  Of course it must also work into the other
direction.  Implementing this feature in a proper manner is not as easy
as it seems I think (you don't accidently want to take the
opportunity?).

OTOH, performing such more complex renaming operations should be
supported as conveniently as possible in "standard" dired buffers.

Regards,

Michael.

bug#63676: cancelling editable dired causes UI problems with dired

[Thierry Volpiatto]

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


Michael Heerdegen <michael_heerdegen@web.de> writes:

> Implementing this feature in a proper manner is not as easy as it
> seems I think (you don't accidently want to take the opportunity?).

No I am very rarely using dired and now run my own package (wfnames) to
edit filenames.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#63676: cancelling editable dired causes UI problems with dired

[Drew Adams]

> > You could avoid such problem by creating a dired buffer with _only_ the
> > files you want to modify (helm allows this), then switch to wdired-mode
> > and do your modifications.
> > Probably implementing this in dired (i.e. open a new dired buffer with
> > only marked files) would be a good addition.
> 
> Thanks for mentioning this.  Yes, we definitely want something like
> this.  I was experimenting with such "helper" dired buffers myself
> (inspired by Icicles and Helm among others).
> 
> It's surprisingly hard to integrate such buffers into the dired concept,
> though: you can't advertise them (else they would be displayed when you
> "open the respective directory").  If you don't advertise them, file
> changes in other buffers (like renamings) are not propagated into these
> additional buffers.  Of course it must also work into the other
> direction.  Implementing this feature in a proper manner is not as easy
> as it seems I think (you don't accidently want to take the
> opportunity?).
> 
> OTOH, performing such more complex renaming operations should be
> supported as conveniently as possible in "standard" dired buffers.

(Caveat: I haven't followed this thread.)

With Dired+ you can create a Dired buffer for any
arbitrary list of files & dirs, even interactively,
in several ways.  Such a buffer isn't connected
with any particular directory listings (whether by
`ls' or ls-lisp).

E.g., commands `diredp-marked(-other-window)' do
it for the marked files & dirs in a Dired buffer.
(`C-M-*' is bound to `diredp-marked-other-window'.)

You can then use WDired on such buffer, to make any
changes you like.

Dired+ hasn't tried to fix any WDired problems, so
some WDired problems that you pointed to might
still be problematic with Dired+; dunno.  I'm not
an expert on WDired, and I haven't tried to look
into use cases etc.  But for the case you describe
it seems to work OK (superficial testing).


https://www.emacswiki.org/emacs/download/dired%2b.el

bug#63676: cancelling editable dired causes UI problems with dired

[Thierry Volpiatto]

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


Hello Drew,

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

> With Dired+ you can create a Dired buffer for any
> arbitrary list of files & dirs, even interactively,
> in several ways.  Such a buffer isn't connected
> with any particular directory listings (whether by
> `ls' or ls-lisp).
>
> E.g., commands `diredp-marked(-other-window)' do
> it for the marked files & dirs in a Dired buffer.
> (`C-M-*' is bound to `diredp-marked-other-window'.)
>
> You can then use WDired on such buffer, to make any
> changes you like.
>
> Dired+ hasn't tried to fix any WDired problems, so
> some WDired problems that you pointed to might
> still be problematic with Dired+; dunno.

Should work fine with emacs-29+ and broken before (needed a patched Wdired).

> I'm not an expert on WDired, and I haven't tried to look into use
> cases etc.  But for the case you describe it seems to work OK
> (superficial testing).
>
>
> https://www.emacswiki.org/emacs/download/dired%2b.el


-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#63676: cancelling editable dired causes UI problems with dired

[Drew Adams]

Hi Thierry,

> > With Dired+ you can create a Dired buffer for any
> > arbitrary list of files & dirs, even interactively,
> > in several ways.  Such a buffer isn't connected
> > with any particular directory listings (whether by
> > `ls' or ls-lisp).
> >
> > E.g., commands `diredp-marked(-other-window)' do
> > it for the marked files & dirs in a Dired buffer.
> > (`C-M-*' is bound to `diredp-marked-other-window'.)
> >
> > You can then use WDired on such buffer, to make any
> > changes you like.
> >
> > Dired+ hasn't tried to fix any WDired problems, so
> > some WDired problems that you pointed to might
> > still be problematic with Dired+; dunno.
> 
> Should work fine with emacs-29+ and broken before (needed a patched
> Wdired).

What is it that should work fine with 29+ but was broken before?

bug#63676: cancelling editable dired causes UI problems with dired

[Thierry Volpiatto]

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


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

> Hi Thierry,
>
>> > With Dired+ you can create a Dired buffer for any
>> > arbitrary list of files & dirs, even interactively,
>> > in several ways.  Such a buffer isn't connected
>> > with any particular directory listings (whether by
>> > `ls' or ls-lisp).
>> >
>> > E.g., commands `diredp-marked(-other-window)' do
>> > it for the marked files & dirs in a Dired buffer.
>> > (`C-M-*' is bound to `diredp-marked-other-window'.)
>> >
>> > You can then use WDired on such buffer, to make any
>> > changes you like.
>> >
>> > Dired+ hasn't tried to fix any WDired problems, so
>> > some WDired problems that you pointed to might
>> > still be problematic with Dired+; dunno.
>> 
>> Should work fine with emacs-29+ and broken before (needed a patched
>> Wdired).
>
> What is it that should work fine with 29+ but was broken before?

A dired buffer in wdired-mode composed of a list of absolute filenames.

-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#63676: cancelling editable dired causes UI problems with dired

[Drew Adams]

> >> > With Dired+ you can create a Dired buffer for any
> >> > arbitrary list of files & dirs, even interactively,
> >> > in several ways.  Such a buffer isn't connected
> >> > with any particular directory listings (whether by
> >> > `ls' or ls-lisp).
> >> >
> >> > E.g., commands `diredp-marked(-other-window)' do
> >> > it for the marked files & dirs in a Dired buffer.
> >> > (`C-M-*' is bound to `diredp-marked-other-window'.)
> >> >
> >> > You can then use WDired on such buffer, to make any
> >> > changes you like.
> >> >
> >> > Dired+ hasn't tried to fix any WDired problems, so
> >> > some WDired problems that you pointed to might
> >> > still be problematic with Dired+; dunno.
> >>
> >> Should work fine with emacs-29+ and broken before (needed a patched
> >> Wdired).
> >
> > What is it that should work fine with 29+ but was broken before?
> 	
> A dired buffer in wdired-mode composed of a list of absolute filenames.

I see.  Did you maybe mean "arbitrary" instead of
"absolute"?  I guess maybe you're saying that WDired
didn't work with arbitrary file-name listings before
Emacs 29?	

bug#63676: cancelling editable dired causes UI problems with dired

[Thierry Volpiatto]

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


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

>> >> > With Dired+ you can create a Dired buffer for any
>> >> > arbitrary list of files & dirs, even interactively,
>> >> > in several ways.  Such a buffer isn't connected
>> >> > with any particular directory listings (whether by
>> >> > `ls' or ls-lisp).
>> >> >
>> >> > E.g., commands `diredp-marked(-other-window)' do
>> >> > it for the marked files & dirs in a Dired buffer.
>> >> > (`C-M-*' is bound to `diredp-marked-other-window'.)
>> >> >
>> >> > You can then use WDired on such buffer, to make any
>> >> > changes you like.
>> >> >
>> >> > Dired+ hasn't tried to fix any WDired problems, so
>> >> > some WDired problems that you pointed to might
>> >> > still be problematic with Dired+; dunno.
>> >>
>> >> Should work fine with emacs-29+ and broken before (needed a patched
>> >> Wdired).
>> >
>> > What is it that should work fine with 29+ but was broken before?
>> 	
>> A dired buffer in wdired-mode composed of a list of absolute filenames.
>
> I see.  Did you maybe mean "arbitrary" instead of
> "absolute"?

Absolute, but it is not really important, try both:

    (dired '("~/tmp" "/home/you/tmp/foo.txt" "/home/you/tmp/bar.txt" "/home/you/tmp/baz.txt"))
    (dired '("~/tmp" "foo.txt" "bar.txt" "baz.txt"))

Then edit a filename with wdired and see the error in 28.2 and not in 29+.

> I guess maybe you're saying that WDired didn't work with arbitrary
> file-name listings before Emacs 29?


-- 
Thierry

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 686 bytes --]

bug#63676: cancelling editable dired causes UI problems with dired

[Drew Adams]

> >> >> Should work fine with emacs-29+ and broken before (needed a patched
> >> >> Wdired).
> >> >
> >> > What is it that should work fine with 29+ but was broken before?
> >>
> >> A dired buffer in wdired-mode composed of a list of absolute filenames.
> >
> > I see.  Did you maybe mean "arbitrary" instead of
> > "absolute"?
> 
> Absolute, but it is not really important, try both:
> 
>     (dired '("~/tmp" "/home/you/tmp/foo.txt" "/home/you/tmp/bar.txt"
> "/home/you/tmp/baz.txt"))
>     (dired '("~/tmp" "foo.txt" "bar.txt" "baz.txt"))
> 
> Then edit a filename with wdired and see the error in 28.2 and not in 29+.
> 
> > I guess maybe you're saying that WDired didn't work with arbitrary
> > file-name listings before Emacs 29?

I see; thx for explaining.

bug#63676: cancelling editable dired causes UI problems with dired

[Drew Adams]

> You could avoid such problem by creating a dired buffer with _only_ the
> files you want to modify (helm allows this), then switch to wdired-mode
> and do your modifications.
> Probably implementing this in dired (i.e. open a new dired buffer with
> only marked files) would be a good addition.
> 
> Note: Only Emacs-30 (see commit 66040fbeed2) allow this,
> Emacs-28 dired doesn't support a list of files.

I don't know what you mean, and I don't know
what that commit does.  But Dired has always
supported an arbitrary list of files & dirs.

It just doesn't support it in some ways.  In
particular, it doesn't let you create such a
listing _interactively_.  And what should it
mean to revert such a buffer, if some of the
files & dirs listed are renamed or no longer
exist?

FWIW:

Dired+ makes it easy to create such arbitrary
listings, in various ways, interactively. 

But if what you mean is that vanilla Dired
doesn't let you create a Dired buffer with
just the files & dirs that you mark in another
Dired buffer, that's true.

Maybe that's what will be added by that commit
for Emacs 30.  Dired+ has provided this since
2008.

Dired+ also supports _reverting_ arbitrary
listings.  For some kinds, such as listings of
your recently accessed files or dirs, reverting
refreshes as you'd expect: renamings, removals,
etc. are reflected.

If you revert a completely arbitrary listing,
however, only the files & dirs that you listed
originally are included in the reverted listing
(it's impossible to know what else to expect).


https://www.emacswiki.org/emacs/download/dired%2b.el

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Are we sure this is the _only_ thing that needs to be updated?
> dired-revert does much more, so we should audit what it does carefully
> to determine which parts may need re-doing here.

Ok, I now had another look.  The only thing that aborting does not
100%-cleanly revert is: if option `wdired-search-replace-filenames' is
non-nil, wdired does some setup for isearch to enable the user to
query-replace only in filenames (and ignore other buffer parts).  If the
user disables that option after entering wdired, but before aborting,
the current code doesn't revert those modifications to isearch
functions.  [ Note that calling dired-revert does not help here because
it doesn't touch buffer local variables ].  I think that needs fixing,
although it is unlikely that somebody does that.

The other modifications to the dired buffer are all restored carefully.

What's left are effects that could be caused by restoring the buffer
contents with the old contents when reverting (like this issue here).

`dired-build-subdir-alist' is the only thing in dired that uses markers
not only temporarily.  All other markers are used to ease operations and
are thrown away after the operation is finished.  Overlays are not used
at all.

Text properties and file marks (those like "*") are part of the saved
buffer contents and are restored along with the contents.

Things like re-registering the dired buffer are done (`dired-advertize')
accordingly.

Some things also have already been fixed AFAIR (like the cooperation of
dired-hide-details-mode and wdired).  People have been using and testing
aborting for a long time.  I think nearly everything would have been
uncovered until now: recent related bug reports mostly discovered
problems related to recent changes to dired that not yet had been
handled by the wdired code.

It's a complex matter but I don't have more useful ideas what else could
go wrong that a `dired-revert' would fix (or not; in general).  From my
perspective my suggested change and the quality of dired and wdired wrt
this change is quite safe.


Michael.

bug#63676: cancelling editable dired causes UI problems with dired

[Eli Zaretskii]

> From: Michael Heerdegen <michael_heerdegen@web.de>
> Cc: peter.mao@gmail.com,  63676@debbugs.gnu.org
> Date: Sun, 28 May 2023 05:39:34 +0200
> 
> It's a complex matter but I don't have more useful ideas what else could
> go wrong that a `dired-revert' would fix (or not; in general).  From my
> perspective my suggested change and the quality of dired and wdired wrt
> this change is quite safe.

Thanks, so I've now installed your change on the emacs-29 branch (and
will hold you responsible for any breakage that causes ;-).

bug#63676: cancelling editable dired causes UI problems with dired

[Michael Heerdegen]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, so I've now installed your change on the emacs-29 branch

Thanks.

> (and will hold you responsible for any breakage that causes ;-).

Of course!  A breakage would hurt more than the responsibility.

Jokes aside: Calling `dired-build-subdir-alist' here should be save:
`dired-undo' also calls it when a user undoes mark changes (so it's not
only called after reading in a fresh buffer), it should be "normal
business" (and the right fix).

Michael.