From: Mike Kupfer <mkupfer@alum.berkeley.edu>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 58721@debbugs.gnu.org, gusbrs.2016@gmail.com
Subject: bug#58721: 28.2; dired with delete-by-moving-to-trash can't trash directory twice
Date: Sun, 30 Oct 2022 15:20:05 -0700 [thread overview]
Message-ID: <50290.1667168405@alto> (raw)
In-Reply-To: Your message of "Sun, 30 Oct 2022 21:51:18 +0200." <835yg1f5u1.fsf@gnu.org>
[-- Attachment #1: Type: text/plain, Size: 452 bytes --]
Eli Zaretskii wrote:
> I meant in the code, not the log message, and I meant to tell more
> detail.
>
> But never mind, I can always add it later myself.
Well, it turns out I hadn't tested my previous patch enough. It handled
the first deletion of directory foo, but not subsequent deletions (which
was the entire point of the fix, sigh). I've attached another patch.
I took a stab at improving the commenting in the code while I was
there.
mike
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: v2 fix --]
[-- Type: text/x-diff, Size: 2274 bytes --]
From 402c7c819dbe2a1cd96462be699ae2af1f5c2440 Mon Sep 17 00:00:00 2001
From: Mike Kupfer <mkupfer@alum.berkeley.edu>
Date: Sun, 30 Oct 2022 10:31:11 -0700
Subject: [PATCH] Fix cross-filesystem directory trashing (Bug#58721)
* lisp/files.el (move-file-to-trash): When trashing a directory with
the same name as something that's already in the trash, copy it into
the trash folder and then delete it, rather than using rename-file.
---
lisp/files.el | 21 +++++++++++++++++++--
1 file changed, 19 insertions(+), 2 deletions(-)
diff --git a/lisp/files.el b/lisp/files.el
index 1e1ec6127d..6bfb3aa738 100644
--- a/lisp/files.el
+++ b/lisp/files.el
@@ -8565,10 +8565,27 @@ move-file-to-trash
(setq files-base (substring (file-name-nondirectory info-fn)
0 (- (length ".trashinfo"))))
(write-region nil nil info-fn nil 'quiet info-fn)))
- ;; Finally, try to move the file to the trashcan.
+ ;; Finally, try to move the item to the trashcan. If
+ ;; it's a file, just move it. Things are more
+ ;; complicated for directories. If the target
+ ;; directory already exists (due to uniquification)
+ ;; and the trash directory is in a different
+ ;; filesystem, rename-file will error out, even when
+ ;; 'overwrite' is non-nil. Rather than worry about
+ ;; whether we're crossing filesystems, just check if
+ ;; we've moving a directory and the target directory
+ ;; already exists. That handles both the
+ ;; same-filesystem and cross-filesystem cases.
(let ((delete-by-moving-to-trash nil)
(new-fn (file-name-concat trash-files-dir files-base)))
- (rename-file fn new-fn overwrite)))))))))
+ (if (or (not is-directory)
+ (not (file-exists-p new-fn)))
+ (rename-file fn new-fn overwrite)
+ (copy-directory fn
+ (file-name-as-directory new-fn)
+ t nil t)
+ (delete-directory fn t))))))))))
+
\f
(defsubst file-attribute-type (attributes)
--
2.30.2
next prev parent reply other threads:[~2022-10-30 22:20 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-10-22 18:23 bug#58721: 28.2; dired with delete-by-moving-to-trash can't trash directory twice Gustavo Barros
2022-10-27 16:09 ` Eli Zaretskii
2022-10-27 17:07 ` Gustavo Barros
2022-10-27 17:22 ` Gustavo Barros
2022-10-27 17:30 ` Eli Zaretskii
2022-10-27 17:51 ` Gustavo Barros
2022-10-27 18:20 ` Eli Zaretskii
2022-10-27 18:41 ` Gustavo Barros
2022-10-27 19:04 ` Eli Zaretskii
2022-10-27 19:13 ` Gustavo Barros
2022-10-27 22:01 ` Gustavo Barros
2022-10-28 7:46 ` Eli Zaretskii
2022-10-28 10:43 ` Gustavo Barros
2022-10-28 11:44 ` Eli Zaretskii
2022-10-28 12:35 ` Gustavo Barros
2022-10-28 15:26 ` Stefan Kangas
2022-10-28 16:06 ` Gustavo Barros
2022-10-28 19:07 ` Gustavo Barros
2022-10-29 5:25 ` Mike Kupfer
2022-10-29 10:35 ` Gustavo Barros
2022-10-29 15:24 ` Mike Kupfer
2022-10-29 15:48 ` Eli Zaretskii
2022-10-29 16:32 ` Mike Kupfer
2022-10-29 16:56 ` Eli Zaretskii
2022-10-30 0:09 ` Mike Kupfer
2022-10-30 6:41 ` Eli Zaretskii
2022-10-30 17:40 ` Mike Kupfer
2022-10-30 18:02 ` Eli Zaretskii
2022-10-30 18:18 ` Mike Kupfer
2022-10-30 19:51 ` Eli Zaretskii
2022-10-30 22:20 ` Mike Kupfer [this message]
2022-10-30 23:10 ` Gustavo Barros
2022-10-31 0:01 ` Mike Kupfer
2022-10-31 0:23 ` Gustavo Barros
2022-10-31 12:49 ` Eli Zaretskii
2022-10-31 13:16 ` Gustavo Barros
2022-11-21 1:08 ` Mike Kupfer
2022-11-21 1:45 ` Gustavo Barros
2022-11-21 1:52 ` Mike Kupfer
2022-11-24 11:19 ` Eli Zaretskii
2022-11-24 11:28 ` Gustavo Barros
2022-10-27 19:07 ` Gustavo Barros
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=50290.1667168405@alto \
--to=mkupfer@alum.berkeley.edu \
--cc=58721@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=gusbrs.2016@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this external index
https://git.savannah.gnu.org/cgit/emacs.git
https://git.savannah.gnu.org/cgit/emacs/org-mode.git
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.