bug#27940: Recursively delete dir34? (yes, no, all, quit)

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[積丹尼 Dan Jacobson]

dired-do-flagged-delete and me interaction:
Recursively delete dcepc? (yes or no) yes
Recursively delete emmpc? (yes or no) yes
Recursively delete zpspc? (yes or no) yes
Recursively delete dgcpc? (yes or no) yes

Wouldn't it be nice if there was instead:
Recursively delete dgcpc? (yes, no, all, quit)

Yes, if before we started we set the variables we needn't be asked all
those questions.

But now *midway* through the list, we decide we would like no more
question, there should be a way, without needing to quit and start over,
even if doing that isn't so bad.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

積丹尼 Dan Jacobson <jidanni@jidanni.org> writes:

> dired-do-flagged-delete and me interaction:
> Recursively delete dcepc? (yes or no) yes
> Recursively delete emmpc? (yes or no) yes
> Recursively delete zpspc? (yes or no) yes
> Recursively delete dgcpc? (yes or no) yes
>
> Wouldn't it be nice if there was instead:
> Recursively delete dgcpc? (yes, no, all, quit)
>
> Yes, if before we started we set the variables we needn't be asked all
> those questions.
>
> But now *midway* through the list, we decide we would like no more
> question, there should be a way, without needing to quit and start over,
> even if doing that isn't so bad.
Thanks for the suggestion.
You can already quit with '\C-g'.
Concerning accept 'all' in the prompt, i am not sure:
it's a bit dangerous operation.

In the other hand:
1) Customize `dired-recursive-deletes' to value 'always.
2) Do the deletion.
3) Set back `dired-recursive-deletes' to its original value.

It looks a little cumbersome.

How about if `dired-do-delete' called interactively with 2 prefices
performs recursive deletions?
Eli?

--8<-----------------------------cut here---------------start------------->8---
commit 9360866c364f75cac40dc6f91dce93f6a1071f43
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Fri Aug 4 17:21:29 2017 +0900

    dired-do-delete: Delete dirs recursively if called w/ 2 prefices
    
    * lisp/dired.el (dired-do-delete): Bind dired-recursive-deletes
    to 'always when called interactively with 2 prefices.
    (dired-internal-do-deletions): Show in the prompt if we are
    deleteing directories recursively.

diff --git a/lisp/dired.el b/lisp/dired.el
index 24759c6c9b..198968361f 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3031,15 +3031,23 @@ dired-do-flagged-delete
 (defun dired-do-delete (&optional arg)
   "Delete all marked (or next ARG) files.
 `dired-recursive-deletes' controls whether deletion of
-non-empty directories is allowed."
+non-empty directories is allowed.
+
+Interactively with 2 prefices, delete recursively non-empty
+directories without prompt user."
   ;; This is more consistent with the file marking feature than
   ;; dired-do-flagged-delete.
   (interactive "P")
-  (dired-internal-do-deletions
-   ;; this may move point if ARG is an integer
-   (dired-map-over-marks (cons (dired-get-filename) (point))
-			 arg)
-   arg t))
+  (let ((dired-recursive-deletes
+         (pcase arg
+           ('(16) (setq arg nil) 'always)
+           (_ dired-recursive-deletes))))
+    (message "dired-recursive-deletes %S" dired-recursive-deletes)
+    (dired-internal-do-deletions
+     ;; this may move point if ARG is an integer
+     (dired-map-over-marks (cons (dired-get-filename) (point))
+			   arg)
+     arg t)))
 
 (defvar dired-deletion-confirmer 'yes-or-no-p) ; or y-or-n-p?
 
@@ -3055,13 +3063,15 @@ dired-internal-do-deletions
   (let* ((files (mapcar #'car l))
 	 (count (length l))
 	 (succ 0)
+         (recursive-del-p (eq dired-recursive-deletes 'always))
 	 (trashing (and trash delete-by-moving-to-trash)))
     ;; canonicalize file list for pop up
     (setq files (nreverse (mapcar #'dired-make-relative files)))
     (if (dired-mark-pop-up
 	 " *Deletions*" 'delete files dired-deletion-confirmer
-	 (format "%s %s "
+	 (format "%s%s %s"
 		 (if trashing "Trash" "Delete")
+                 (if recursive-del-p " recursively" "")
 		 (dired-mark-prompt arg files)))
 	(save-excursion
 	  (let ((progress-reporter
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-04
Repository revision: db5d38ddb0de83d8f920b7a128fe3fd5156fdf85

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: 27940@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
> Date: Fri, 04 Aug 2017 17:25:49 +0900
> 
> > dired-do-flagged-delete and me interaction:
> > Recursively delete dcepc? (yes or no) yes
> > Recursively delete emmpc? (yes or no) yes
> > Recursively delete zpspc? (yes or no) yes
> > Recursively delete dgcpc? (yes or no) yes
> >
> > Wouldn't it be nice if there was instead:
> > Recursively delete dgcpc? (yes, no, all, quit)
> >
> > Yes, if before we started we set the variables we needn't be asked all
> > those questions.
> >
> > But now *midway* through the list, we decide we would like no more
> > question, there should be a way, without needing to quit and start over,
> > even if doing that isn't so bad.
> Thanks for the suggestion.
> You can already quit with '\C-g'.
> Concerning accept 'all' in the prompt, i am not sure:
> it's a bit dangerous operation.
> 
> In the other hand:
> 1) Customize `dired-recursive-deletes' to value 'always.
> 2) Do the deletion.
> 3) Set back `dired-recursive-deletes' to its original value.

Actually, I think the value he wants is 'top'.

I don't object to accepting something like "!" to mean "all", I
believe we already have a few features that do this, and the
implementation should be simple, I think.  (Creeping featurism, I
know, but what else did you expect from users who have no real bugs to
report? ;-)

> How about if `dired-do-delete' called interactively with 2 prefices
> performs recursive deletions?
> Eli?

Sounds too cumbersome to me.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Tino Calancha <tino.calancha@gmail.com>
>> Cc: 27940@debbugs.gnu.org, Eli Zaretskii <eliz@gnu.org>
>> Date: Fri, 04 Aug 2017 17:25:49 +0900
>> 
>> > dired-do-flagged-delete and me interaction:
>> > Recursively delete dcepc? (yes or no) yes
>> > Recursively delete emmpc? (yes or no) yes
>> > Recursively delete zpspc? (yes or no) yes
>> > Recursively delete dgcpc? (yes or no) yes
>> >
>> > Wouldn't it be nice if there was instead:
>> > Recursively delete dgcpc? (yes, no, all, quit)
>> >
>> > Yes, if before we started we set the variables we needn't be asked all
>> > those questions.
>> >
>> > But now *midway* through the list, we decide we would like no more
>> > question, there should be a way, without needing to quit and start over,
>> > even if doing that isn't so bad.
>> Thanks for the suggestion.
>> You can already quit with '\C-g'.
>> Concerning accept 'all' in the prompt, i am not sure:
>> it's a bit dangerous operation.
>> 
>> In the other hand:
>> 1) Customize `dired-recursive-deletes' to value 'always.
>> 2) Do the deletion.
>> 3) Set back `dired-recursive-deletes' to its original value.
>
> Actually, I think the value he wants is 'top'.
The he would be prompted the 34 times all over.  I think the OP
wants 'always (like Bon Jovi).

> I don't object to accepting something like "!" to mean "all", I
> believe we already have a few features that do this, and the
> implementation should be simple, I think.  (Creeping featurism, I
> know, but what else did you expect from users who have no real bugs to
> report? ;-)
>
>> How about if `dired-do-delete' called interactively with 2 prefices
>> performs recursive deletions?
>> Eli?
>
> Sounds too cumbersome to me.
Updated patch.  Now it accepts answers: y, n, !, q
(as the OP suggested)
--8<-----------------------------cut here---------------start------------->8---
commit 90e4eb9fa1b708bab87844160371ec9ce439ab91
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Fri Aug 4 18:17:51 2017 +0900

    dired-do-delete: Allow to delete dirs recursively
    
    * lisp/dired.el (dired-delete-file): Accept 2 additional answers:
    '!', to delete all directories recursively and no prompt anymore.
    'q', to cancel the directry deletions (Bug#27940).
    (dired-do-flagged-delete): Bind locally dired-recursive-deletes
    so that we can overwrite its global value.
    Wrapp the loop within a catch '--delete-cancel to catch when
    the user abort the directtry deletion.

diff --git a/lisp/dired.el b/lisp/dired.el
index 24759c6c9b..278acc2cf5 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2990,23 +2990,33 @@ dired-delete-file
 
 TRASH non-nil means to trash the file instead of deleting, provided
 `delete-by-moving-to-trash' (which see) is non-nil."
-  ;; This test is equivalent to
-  ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
-  ;; but more efficient
-  (if (not (eq t (car (file-attributes file))))
-      (delete-file file trash)
-    (if (and recursive
-	     (directory-files file t dired-re-no-dot) ; Not empty.
-	     (or (eq recursive 'always)
-		 (yes-or-no-p (format "Recursively %s %s? "
-				      (if (and trash
-					       delete-by-moving-to-trash)
-					  "trash"
-					"delete")
-				      (dired-make-relative file)))))
-	(if (eq recursive 'top) (setq recursive 'always)) ; Don't ask again.
-      (setq recursive nil))
-    (delete-directory file recursive trash)))
+       ;; This test is equivalent to
+       ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
+       ;; but more efficient
+       (if (not (eq t (car (file-attributes file))))
+           (delete-file file trash)
+         (let* ((valid-answers (list "y" "n" "!" "q"))
+                (answer "")
+                (input-fn (lambda ()
+                            (setq answer
+                                  (completing-read (format "Recursively %s %s? [y, n, !, q] "
+				                           (if (and trash
+					                            delete-by-moving-to-trash)
+					                       "trash"
+				                             "delete")
+				                           (dired-make-relative file))
+                                                   valid-answers nil t)))))
+           (if (and recursive
+	            (directory-files file t dired-re-no-dot) ; Not empty.
+	            (eq recursive 'always))
+	       (if (eq recursive 'top) (setq recursive 'always)) ; Don't ask again.
+             ;; Otherwise prompt user:
+             (while (string= "" answer) (funcall input-fn))
+             (pcase answer
+               ('"!" (setq recursive 'always dired-recursive-deletes recursive))
+               ('"y" (if (eq recursive 'top) (setq recursive 'always)))
+               ('"q" (keyboard-quit))))
+           (delete-directory file recursive trash))))
 
 (defun dired-do-flagged-delete (&optional nomessage)
   "In Dired, delete the files flagged for deletion.
@@ -3055,6 +3065,9 @@ dired-internal-do-deletions
   (let* ((files (mapcar #'car l))
 	 (count (length l))
 	 (succ 0)
+	 ;; Bind `dired-recursive-deletes' so that we can change it
+	 ;; locally according with the user answer within `dired-delete-file'.
+	 (dired-recursive-deletes dired-recursive-deletes)
 	 (trashing (and trash delete-by-moving-to-trash)))
     ;; canonicalize file list for pop up
     (setq files (nreverse (mapcar #'dired-make-relative files)))
@@ -3064,6 +3077,7 @@ dired-internal-do-deletions
 		 (if trashing "Trash" "Delete")
 		 (dired-mark-prompt arg files)))
 	(save-excursion
+          (catch '--delete-cancel
 	  (let ((progress-reporter
 		 (make-progress-reporter
 		  (if trashing "Trashing..." "Deleting...")
@@ -3081,6 +3095,7 @@ dired-internal-do-deletions
 		      (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)
 		   (setq failures (cons (car (car l)) failures)))))
@@ -3091,7 +3106,7 @@ dired-internal-do-deletions
 	       (format "%d of %d deletion%s failed"
 		       (length failures) count
 		       (dired-plural-s count))
-	       failures))))
+	       failures)))))
       (message "(No deletions performed)")))
   (dired-move-to-filename))
 
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-04
Repository revision: db5d38ddb0de83d8f920b7a128fe3fd5156fdf85

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

Tino Calancha <tino.calancha@gmail.com> writes:

> +             ;; Otherwise prompt user:
> +             (while (string= "" answer) (funcall input-fn))
> +             (pcase answer
> +               ('"!" (setq recursive 'always dired-recursive-deletes recursive))
> +               ('"y" (if (eq recursive 'top) (setq recursive 'always)))
> +               ('"q" (keyboard-quit))))
> +           (delete-directory file recursive trash))))

Upps, i forgot the 'n' case:
@@ -3015,6 +3015,7 @@ dired-delete-file
              (pcase answer
                ('"!" (setq recursive 'always dired-recursive-deletes recursive))
                ('"y" (if (eq recursive 'top) (setq recursive 'always)))
+               ('"n" (setq recursive nil))
                ('"q" (keyboard-quit))))
            (delete-directory file recursive trash))))
 

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: 27940@debbugs.gnu.org,  jidanni@jidanni.org
> Date: Fri, 04 Aug 2017 18:29:41 +0900
> 
> index 24759c6c9b..278acc2cf5 100644
> --- a/lisp/dired.el
> +++ b/lisp/dired.el
> @@ -2990,23 +2990,33 @@ dired-delete-file
>  
>  TRASH non-nil means to trash the file instead of deleting, provided
>  `delete-by-moving-to-trash' (which see) is non-nil."
> -  ;; This test is equivalent to
> -  ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
> -  ;; but more efficient
> -  (if (not (eq t (car (file-attributes file))))
> -      (delete-file file trash)
> -    (if (and recursive
> -	     (directory-files file t dired-re-no-dot) ; Not empty.
> -	     (or (eq recursive 'always)
> -		 (yes-or-no-p (format "Recursively %s %s? "
> -				      (if (and trash
> -					       delete-by-moving-to-trash)
> -					  "trash"
> -					"delete")
> -				      (dired-make-relative file)))))
> -	(if (eq recursive 'top) (setq recursive 'always)) ; Don't ask again.
> -      (setq recursive nil))
> -    (delete-directory file recursive trash)))
> +       ;; This test is equivalent to
> +       ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
> +       ;; but more efficient
> +       (if (not (eq t (car (file-attributes file))))
> +           (delete-file file trash)
> +         (let* ((valid-answers (list "y" "n" "!" "q"))

Shouldn't the valid-answers be "yes" and "no", not "y" and "n", for
backward compatibility?

> +                                  (completing-read (format "Recursively %s %s? [y, n, !, q] "

Maybe the "!" and "q" parts should be explained?  Or maybe just use
"yes", "no", "all", and "quite", which are self-explanatory?

This warrants a NEWS entry, I think.  I also wonder whether we should
describe this in the user manual, under "Dired Deletion".

Than ks.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

Eli Zaretskii <eliz@gnu.org> writes:


> Shouldn't the valid-answers be "yes" and "no", not "y" and "n", for
> backward compatibility?
Yes, they should.
>
>> +                                  (completing-read (format "Recursively %s %s? [y, n, !, q] "
>
> Maybe the "!" and "q" parts should be explained?  Or maybe just use
> "yes", "no", "all", and "quite", which are self-explanatory?
Look the updated patch; it's a mix:
1. uses 'yes', 'no'
2. '!', 'q', 'help'
  This is similar like `query-replace' does (there is used '!', 'q', and
  '?').  With 'help', a Help buffer is shown with a help message.
  
>
> This warrants a NEWS entry, I think.  I also wonder whether we should
> describe this in the user manual, under "Dired Deletion".
I did.


--8<-----------------------------cut here---------------start------------->8---
commit a883db5e05364bd7a76138642d39b296266ff0a1
Author: Tino Calancha <tino.calancha@gmail.com>
Date:   Fri Aug 4 23:26:30 2017 +0900

    dired-do-delete: Allow to delete dirs recursively without prompts
    
    * lisp/dired.el (dired-delete-file): Accept 2 additional answers:
    '!', to delete all directories recursively and no prompt anymore.
    'q', to cancel the directory deletions (Bug#27940).
    Show help message when user inputs 'help'.
    (dired-do-flagged-delete): Bind locally dired-recursive-deletes
    so that we can overwrite its global value.
    Wrapp the loop within a catch '--delete-cancel to catch when
    the user abort the directtry deletion.
    * doc/emacs/dired.texi (Dired Deletion): Update manual.
    * etc/NEWS (Changes in Specialized Modes and Packages in Emacs 26.1):
    Announce this change.

diff --git a/doc/emacs/dired.texi b/doc/emacs/dired.texi
index 150ac8427a..5eb066d927 100644
--- a/doc/emacs/dired.texi
+++ b/doc/emacs/dired.texi
@@ -236,6 +236,14 @@ Dired Deletion
 @code{dired-recursive-deletes} is non-@code{nil}, then Dired can
 delete nonempty directories including all their contents.  That can
 be somewhat risky.
+Even if you have set @code{dired-recursive-deletes} to @code{nil},
+you might want sometimes to delete recursively directories
+without being asked for confirmation for all of them.  This is handy
+when you have marked many directories for deletion and you are very
+sure that all of them can safely being deleted.  For every nonempty
+directory you are asked for confirmation; if you answer @code{!},
+then all the remaining directories will be deleted without more
+questions.
 
 @vindex delete-by-moving-to-trash
   If you change the variable @code{delete-by-moving-to-trash} to
diff --git a/etc/NEWS b/etc/NEWS
index b72793dec0..dcac7d5e41 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -611,6 +611,10 @@ paragraphs, for the purposes of bidirectional display.
 ** Dired
 
 +++
+*** You can answer '!' in 'dired-do-delete' to delete recursively all
+remaining directories without more prompts.
+
++++
 *** Dired supports wildcards in the directory part of the file names.
 
 +++
diff --git a/lisp/dired.el b/lisp/dired.el
index 24759c6c9b..133fd2e719 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -2975,6 +2975,14 @@ dired-recursive-deletes
 ;; Match anything but `.' and `..'.
 (defvar dired-re-no-dot "^\\([^.]\\|\\.\\([^.]\\|\\..\\)\\).*")
 
+(defconst dired-delete-help
+  "Type:
+`yes' to delete recursively the current directory,
+`no' to skip to next,
+`!' to delete all remaining directories with no more questions,
+`q' to exit,
+`help' to show this help message.")
+
 ;; Delete file, possibly delete a directory and all its files.
 ;; This function is useful outside of dired.  One could change its name
 ;; to e.g. recursive-delete-file and put it somewhere else.
@@ -2990,23 +2998,40 @@ dired-delete-file
 
 TRASH non-nil means to trash the file instead of deleting, provided
 `delete-by-moving-to-trash' (which see) is non-nil."
-  ;; This test is equivalent to
-  ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
-  ;; but more efficient
-  (if (not (eq t (car (file-attributes file))))
-      (delete-file file trash)
-    (if (and recursive
-	     (directory-files file t dired-re-no-dot) ; Not empty.
-	     (or (eq recursive 'always)
-		 (yes-or-no-p (format "Recursively %s %s? "
-				      (if (and trash
-					       delete-by-moving-to-trash)
-					  "trash"
-					"delete")
-				      (dired-make-relative file)))))
-	(if (eq recursive 'top) (setq recursive 'always)) ; Don't ask again.
-      (setq recursive nil))
-    (delete-directory file recursive trash)))
+       ;; This test is equivalent to
+       ;; (and (file-directory-p fn) (not (file-symlink-p fn)))
+       ;; but more efficient
+       (if (not (eq t (car (file-attributes file))))
+           (delete-file file trash)
+         (let* ((valid-answers (list "yes" "no" "!" "q" "help"))
+                (answer "")
+                (input-fn (lambda ()
+                            (setq answer
+                                  (completing-read
+                                   (format "Recursively %s %s? [yes, no, !, q, help] "
+				           (if (and trash
+					            delete-by-moving-to-trash)
+					       "trash"
+				             "delete")
+				           (dired-make-relative file))
+                                   valid-answers nil t))
+                            (when (string= answer "help")
+                              (setq answer "")
+                              (with-help-window "*Help*"
+                                (with-current-buffer "*Help*" (insert dired-delete-help))))
+                            answer)))
+           (if (and recursive
+	            (directory-files file t dired-re-no-dot) ; Not empty.
+	            (eq recursive 'always))
+	       (if (eq recursive 'top) (setq recursive 'always)) ; Don't ask again.
+             ;; Otherwise prompt user:
+             (while (string= "" answer) (funcall input-fn))
+             (pcase answer
+               ('"!" (setq recursive 'always dired-recursive-deletes recursive))
+               ('"yes" (if (eq recursive 'top) (setq recursive 'always)))
+               ('"no" (setq recursive nil))
+               ('"q" (keyboard-quit))))
+           (delete-directory file recursive trash))))
 
 (defun dired-do-flagged-delete (&optional nomessage)
   "In Dired, delete the files flagged for deletion.
@@ -3055,6 +3080,9 @@ dired-internal-do-deletions
   (let* ((files (mapcar #'car l))
 	 (count (length l))
 	 (succ 0)
+	 ;; Bind `dired-recursive-deletes' so that we can change it
+	 ;; locally according with the user answer within `dired-delete-file'.
+	 (dired-recursive-deletes dired-recursive-deletes)
 	 (trashing (and trash delete-by-moving-to-trash)))
     ;; canonicalize file list for pop up
     (setq files (nreverse (mapcar #'dired-make-relative files)))
@@ -3064,6 +3092,7 @@ dired-internal-do-deletions
 		 (if trashing "Trash" "Delete")
 		 (dired-mark-prompt arg files)))
 	(save-excursion
+          (catch '--delete-cancel
 	  (let ((progress-reporter
 		 (make-progress-reporter
 		  (if trashing "Trashing..." "Deleting...")
@@ -3081,6 +3110,7 @@ dired-internal-do-deletions
 		      (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)
 		   (setq failures (cons (car (car l)) failures)))))
@@ -3091,8 +3121,7 @@ dired-internal-do-deletions
 	       (format "%d of %d deletion%s failed"
 		       (length failures) count
 		       (dired-plural-s count))
-	       failures))))
+	       failures)))))
       (message "(No deletions performed)")))
   (dired-move-to-filename))
 
--8<-----------------------------cut here---------------end--------------->8---
In GNU Emacs 26.0.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.22.11)
 of 2017-08-04
Repository revision: d32d8d0ceaa05939bbf56a246707aed05a53385c

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Eli Zaretskii]

> From: Tino Calancha <tino.calancha@gmail.com>
> Cc: 27940@debbugs.gnu.org,  jidanni@jidanni.org
> Date: Fri, 04 Aug 2017 23:33:51 +0900
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> 
> > Shouldn't the valid-answers be "yes" and "no", not "y" and "n", for
> > backward compatibility?
> Yes, they should.
> >
> >> +                                  (completing-read (format "Recursively %s %s? [y, n, !, q] "
> >
> > Maybe the "!" and "q" parts should be explained?  Or maybe just use
> > "yes", "no", "all", and "quite", which are self-explanatory?
> Look the updated patch; it's a mix:
> 1. uses 'yes', 'no'
> 2. '!', 'q', 'help'
>   This is similar like `query-replace' does (there is used '!', 'q', and
>   '?').  With 'help', a Help buffer is shown with a help message.
>   
> >
> > This warrants a NEWS entry, I think.  I also wonder whether we should
> > describe this in the user manual, under "Dired Deletion".
> I did.

Thanks, LGTM.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Drew Adams]

> > > Shouldn't the valid-answers be "yes" and "no", not "y" and "n", for
> > > backward compatibility?
> > Yes, they should.

> > > Maybe the "!" and "q" parts should be explained?  Or maybe just use
> > > "yes", "no", "all", and "quite", which are self-explanatory?
> > Look the updated patch; it's a mix:
> > 1. uses 'yes', 'no'
> > 2. '!', 'q', 'help'
> >   This is similar like `query-replace' does (there is used '!', 'q', and
> >   '?').  With 'help', a Help buffer is shown with a help message.

I haven't been following this thread, so apologies if I misunderstand.

Is it a good idea to mix `yes-or-no-p'-style (minibuffer) input,
such as `yes' and `no', with `read-char'-style input, such as `!'
and `q'?  I don't think so, a priori.  Especially for `!', which
has worse repercussions in case of a mistake than does `y'.  No?

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

On Fri, 4 Aug 2017, Drew Adams wrote:

>>>> Shouldn't the valid-answers be "yes" and "no", not "y" and "n", for
>>>> backward compatibility?
>>> Yes, they should.
>
>>>> Maybe the "!" and "q" parts should be explained?  Or maybe just use
>>>> "yes", "no", "all", and "quite", which are self-explanatory?
>>> Look the updated patch; it's a mix:
>>> 1. uses 'yes', 'no'
>>> 2. '!', 'q', 'help'
>>>   This is similar like `query-replace' does (there is used '!', 'q', and
>>>   '?').  With 'help', a Help buffer is shown with a help message.
>
> I haven't been following this thread, so apologies if I misunderstand.
>
> Is it a good idea to mix `yes-or-no-p'-style (minibuffer) input,
> such as `yes' and `no', with `read-char'-style input, such as `!'
> and `q'?  I don't think so, a priori.  Especially for `!', which
> has worse repercussions in case of a mistake than does `y'.  No?
Agreed.  If you see my first answer i was reluctant to allow the dangerous
operation.
I will use "all" and "quit" instead of "!", "q".

* Question:
Do we need to show the help after this change?

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Drew Adams]

> > Is it a good idea to mix `yes-or-no-p'-style (minibuffer) input,
> > such as `yes' and `no', with `read-char'-style input, such as `!'
> > and `q'?  I don't think so, a priori.  Especially for `!', which
> > has worse repercussions in case of a mistake than does `y'.  No?
> Agreed.  If you see my first answer i was reluctant to allow the dangerous
> operation.
> I will use "all" and "quit" instead of "!", "q".
> 
> * Question:
> Do we need to show the help after this change?

I would say yes, for the help.  Think of `yes-or-no-p'.
If you type `foo' to `yes-or-no-p' it helps you.  Here
we should offer more help, explaining the possibilities.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

On Fri, 4 Aug 2017, Drew Adams wrote:

>>> Is it a good idea to mix `yes-or-no-p'-style (minibuffer) input,
>>> such as `yes' and `no', with `read-char'-style input, such as `!'
>>> and `q'?  I don't think so, a priori.  Especially for `!', which
>>> has worse repercussions in case of a mistake than does `y'.  No?
>> Agreed.  If you see my first answer i was reluctant to allow the dangerous
>> operation.
>> I will use "all" and "quit" instead of "!", "q".
>>
>> * Question:
>> Do we need to show the help after this change?
>
> I would say yes, for the help.  Think of `yes-or-no-p'.
> If you type `foo' to `yes-or-no-p' it helps you.  Here
> we should offer more help, explaining the possibilities.
I see.  Thank you for the explanation!

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

Eli Zaretskii <eliz@gnu.org> writes:

>> > This warrants a NEWS entry, I think.  I also wonder whether we should
>> > describe this in the user manual, under "Dired Deletion".
>> Done.
>
> Thanks, LGTM.
Added feature into master branch as commit
cbea38e5c4af5386192fb9a48ef4fca5080d6561
(dired-do-delete: Allow to delete dirs recursively without prompts)

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

On Tue, 8 Aug 2017, Robert Marshall wrote:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>>>> This warrants a NEWS entry, I think.  I also wonder whether we should
>>>>> describe this in the user manual, under "Dired Deletion".
>>>> Done.
>>>
>>> Thanks, LGTM.
>> Added feature into master branch as commit
>> cbea38e5c4af5386192fb9a48ef4fca5080d6561
>> (dired-do-delete: Allow to delete dirs recursively without prompts)
>
> Hi
>
> With this change deleting an empty directory now asks for confirmation
> whereas it didn't before and answering no to the recursively delete
> question still deletes the directory, which seems a bit counter
> intuitive?
>
> Robert
Thank you very much.
I believe commit da4438e14f1c55808937872b6d651a807404daa2
(dired-delete-file:  Dont't ask for empty dirs)
has fixed it.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Noam Postavsky]

Tino Calancha <tino.calancha@gmail.com> writes:

> I believe commit da4438e14f1c55808937872b6d651a807404daa2
> (dired-delete-file:  Dont't ask for empty dirs)
> has fixed it.

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?

Test dired-test-bug27940 backtrace:
  delete-directory-internal("/tmp/bug27940QSb9cm/non-empty-0")
  apply(delete-directory-internal "/tmp/bug27940QSb9cm/non-empty-0")
  files--force(nil delete-directory-internal "/tmp/bug27940QSb9cm/non-
  delete-directory("/tmp/bug27940QSb9cm/non-empty-0" nil t)
  dired-delete-file("/tmp/bug27940QSb9cm/non-empty-0" top t)
  dired-internal-do-deletions((("/tmp/bug27940QSb9cm/empty-dir-0" . 18
  dired-do-delete(nil)
  (progn (advice-add 'dired--yes-no-all-quit-help :override (function 
  (unwind-protect (progn (advice-add 'dired--yes-no-all-quit-help :ove
  (let* ((dir (make-temp-file "bug27940" t)) (dired-deletion-confirmer
  (closure (t) nil (let* ((dir (make-temp-file "bug27940" t)) (dired-d
  ert--run-test-internal(#s(ert--test-execution-info :test #s(ert-test
  ert-run-test(#s(ert-test :name dired-test-bug27940 :documentation "T
  ert-run-or-rerun-test(#s(ert--stats :selector t :tests [#s(ert-test 
  ert-run-tests(t #f(compiled-function (event-type &rest event-args) #
  ert-run-tests-batch(nil)
  ert-run-tests-batch-and-exit(nil)
  eval((ert-run-tests-batch-and-exit nil))
  command-line-1(("-L" ":../../test" "-l" "ert" "-l" "lisp/dired-tests
  command-line()
  normal-top-level()
Test dired-test-bug27940 condition:
    (file-error "Removing directory" "Directory not empty" "/tmp/bug27940QSb9cm/non-empty-0")
   FAILED   9/11  dired-test-bug27940

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> I believe commit da4438e14f1c55808937872b6d651a807404daa2
>> (dired-delete-file:  Dont't ask for empty dirs)
>> has fixed it.
>
> 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?
Those tests implicitely assume you keep the
'dired-recursive-deletes' default, i.e. 'top; IMO it's not
sorprising that they fail if you run them interactively
with other 'dired-recursive-deletes' value.

We could explicitely set 'dired-recursive-deletes' 'top if you
prefer that.

The following snippet behaves the same in Emacs-25 and the master branch:

emacs -Q /tmp -eval "(customize-set-variable 'dired-recursive-deletes nil)"
+ foo RET RET
+ a/b RET
+ c/d RET
+ e RET
t D yes RET no RET no RET ; Only 'e' is deleted

;; Delete just 'c'
U D yes RET ; Signal an error
;; Probably this error should be catched as well?
;; We could open a bug report against 25.3 for this.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Noam Postavsky]

Tino Calancha <tino.calancha@gmail.com> writes:

> Those tests implicitely assume you keep the
> 'dired-recursive-deletes' default, i.e. 'top; IMO it's not
> sorprising that they fail if you run them interactively
> with other 'dired-recursive-deletes' value.

It is failing for me when I run non-interactively:

    make -C test dired-tests

Does it not fail for you?

> We could explicitely set 'dired-recursive-deletes' 'top if you
> prefer that.

I tried also binding it during the test, but it still failed:

modified   test/lisp/dired-tests.el
@@ -364,7 +364,8 @@ dired-test-with-temp-dirs
   (declare (indent 1) (debug body))
   (let ((dir (make-symbol "dir"))
         (ignore-funcs (make-symbol "ignore-funcs")))
-    `(let* ((,dir (make-temp-file "bug27940" t))
+    `(let* ((dired-recursive-deletes 'top)
+            (,dir (make-temp-file "bug27940" t))
             (dired-deletion-confirmer (lambda (_) "yes")) ; Suppress prompts.
             (inhibit-message t)
             (default-directory ,dir))

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Tino Calancha]

Noam Postavsky <npostavs@users.sourceforge.net> writes:

> Tino Calancha <tino.calancha@gmail.com> writes:
>
>> Those tests implicitely assume you keep the
>> 'dired-recursive-deletes' default, i.e. 'top; IMO it's not
>> sorprising that they fail if you run them interactively
>> with other 'dired-recursive-deletes' value.
>
> It is failing for me when I run non-interactively:
>
>     make -C test dired-tests
>
> Does it not fail for you?
No, in my case they pass
(commit: eed3a3d9e95d2c5346a23c9d92ca4e5848330183):

   passed   4/11  dired-test-bug27243-01
   passed   5/11  dired-test-bug27243-02
   passed   6/11  dired-test-bug27243-03
   passed   7/11  dired-test-bug27631
   passed   8/11  dired-test-bug27899
   passed   9/11  dired-test-bug27940
   passed  10/11  dired-test-bug27968
   passed  11/11  dired-test-bug7131

Ran 11 tests, 11 results as expected (2017-10-16 19:37:34+0900)
It pass in the gitlab CI as well:
https://gitlab.com/emacs-ci/emacs/-/jobs/36237548

I don't know why it fails for you.

bug#27940: Recursively delete dir34? (yes, no, all, quit)

[Noam Postavsky]

Tino Calancha <tino.calancha@gmail.com> writes:

> Ran 11 tests, 11 results as expected (2017-10-16 19:37:34+0900)
> It pass in the gitlab CI as well:
> https://gitlab.com/emacs-ci/emacs/-/jobs/36237548
>
> I don't know why it fails for you.

Oh, it's because of another patch I have[1] which changes the
condition-case in dired-do-internal-deletions to
condition-case-unless-debug.  This is really another case of Bug#11218.

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=28797#11