GNU bug report logs - #28797
26.0.90; Improve printing of error on catching file-error in dired

Previous Next

Full log

Message #30 received at 28797 <at> debbugs.gnu.org (full text, mbox):

From: Tino Calancha <tino.calancha <at> gmail.com>
To: Noam Postavsky <npostavs <at> users.sourceforge.net>
Cc: Eli Zaretskii <eliz <at> gnu.org>, Tino Calancha <tino.calancha <at> gmail.com>,
 28797 <at> debbugs.gnu.org, kaushal.modi <at> gmail.com
Subject: Re: bug#28797: 26.0.90; Improve printing of error on catching
 file-error in dired
Date: Wed, 25 Oct 2017 00:41:23 +0900 (JST)

On Sun, 22 Oct 2017, Noam Postavsky wrote:

>> Let's wait to hear what others think about this part.
>
> Not seeing any opinions, so here is a fact: due to bug#11218, changing
> this breaks dired-test-bug27940 [1].  Fixing #11218 would involve some
> major changes to ert internals.
>
> [1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27940#65

>The "no" case of dired-test-bug27940 is failing now.  I guess if
>RECURSIVE is set to nil, we should not try to delete non-empty
>directories, or maybe just catch the error if it happens?
I think that might be OK.  Following add that change on top
of your original patch:

--8<-----------------------------cut here---------------start------------->8---
commit 36c924fca0b4cde3a320b10d40e9453e55170a0f
Author: Tino Calancha <tino.calancha <at> gmail.com>
Date:   Wed Oct 25 00:38:56 2017 +0900

    Improve dired deletion error handling (Bug#28797)

    * lisp/dired (dired-delete-file): If the dir is non-empty and
    RECURSIVE is nil then return 'skip and don't try to delete
    the dir (Bug#28797).
    * lisp/dired.el (dired-internal-do-deletions): Use
    condition-case-unless-debug.  Use `error-message-string' to produce a
    human readable error message.
    Don't call dired-fun-in-all-buffers if `dired-delete-file' returns 'skip.

diff --git a/lisp/dired.el b/lisp/dired.el
index 1ec3ac4f99..74a37da992 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3062,7 +3062,10 @@ dired-delete-file
                      ('"no" (setq recursive nil))
                      ('"quit" (keyboard-quit)))))
              (setq recursive nil)) ; Empty dir or recursive is nil.
-           (delete-directory file recursive trash))))
+           ;; Don't delete non-empty dirs when recursive is nil.
+           (if (and (not empty-dir-p) (not recursive))
+             'skip
+             (delete-directory file recursive trash)))))

 (defun dired-do-flagged-delete (&optional nomessage)
   "In Dired, delete the files flagged for deletion.
@@ -3134,18 +3137,19 @@ dired-internal-do-deletions
 	    (while l
 	      (goto-char (cdr (car l)))
 	      (let ((inhibit-read-only t))
-		(condition-case err
+		(condition-case-unless-debug err
 		    (let ((fn (car (car l))))
-		      (dired-delete-file fn dired-recursive-deletes trash)
-		      ;; if we get here, removing worked
-		      (setq succ (1+ succ))
-		      (progress-reporter-update progress-reporter succ)
-		      (dired-fun-in-all-buffers
-		       (file-name-directory fn) (file-name-nondirectory fn)
-		       #'dired-delete-entry fn))
+		      (if (eq 'skip (dired-delete-file fn dired-recursive-deletes trash))
+                          nil
+		        ;; if we get here, removing worked
+		        (setq succ (1+ succ))
+		        (progress-reporter-update progress-reporter succ)
+		        (dired-fun-in-all-buffers
+		         (file-name-directory fn) (file-name-nondirectory fn)
+		         #'dired-delete-entry fn)))
                   (quit (throw '--delete-cancel (message "OK, canceled")))
 		  (error ;; catch errors from failed deletions
-		   (dired-log "%s\n" err)
+		   (dired-log "%s: %s\n" (car err) (error-message-string err))
 		   (setq failures (cons (car (car l)) failures)))))
 	      (setq l (cdr l)))
 	    (if (not failures)
--8<-----------------------------cut here---------------end--------------->8---

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.