Subject: bug#28797: 26.0.90; Improve printing of error on catching file-error in dired

From: Tino Calancha <[email-masked]>
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 <[email-masked]>
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---