Subject: bug#28373: [PATCH] New variable controls whether dired confirms to kill buffers visiting deleted files

From: Alex Branham <[email-masked]>
Date: Wed, 06 Sep 2017 13:11:45 -0500

Thanks for the review. I think I've addressed the things you've brought up; specific replies below.

While I have your attention, how do I start the process of FSF copyright assignment? I'd rather start it now so I don't have to deal with it later. And do I have to do one form for Emacs and another for org-mode, or will one form take care of both?

Thanks!

On Wed 06 Sep 2017 at 16:36, Eli Zaretskii <[email-masked]> wrote:

>> * lisp/dired-x.el (dired-clean-confirm-killing-deleted-buffers):
>> * lisp/dired.el (dired-clean-up-after-deletion): Just kill buffers
>> visiting deleted files without confirming if
>> dired-clean-confirm-killing-deleted-buffers is nil
>
> This log entry makes it sound as if similar changes were made in both
> dired.el and dired-x.el, which is not what your changes do.  The entry
> for dired-x.el should only say this:
>
>   * lisp/dired-x.el (dired-clean-confirm-killing-deleted-buffers): New variable.

Thanks, modified

>
>> diff --git a/lisp/dired.el b/lisp/dired.el
>> index ff62183f09..bac3933502 100644
>> --- a/lisp/dired.el
>> +++ b/lisp/dired.el
>> @@ -3164,28 +3164,34 @@ dired-delete-entry
>>    (dired-clean-up-after-deletion file))
>>
>>  (defvar dired-clean-up-buffers-too)
>> +(defvar dired-clean-confirm-killing-deleted-buffers)
>
> Why did you need this defvar?

I assumed I did since the other dired-clean variable is there. I guess I don't, though.

>
>>  (defun dired-clean-up-after-deletion (fn)
>>    "Clean up after a deleted file or directory FN.
>> -Removes any expanded subdirectory of deleted directory.
>> -If `dired-x' is loaded and `dired-clean-up-buffers-too' is non-nil,
>> -also offers to kill buffers visiting deleted files and directories."
>> +Removes any expanded subdirectory of deleted directory. If
>> +`dired-x' is loaded and `dired-clean-up-buffers-too' is non-nil,
>> +also offers to kill buffers visiting deleted files and
>> +directories. Similarly, if `dired-x' is loaded and
>> +`dired-clean-confirm-killing-deleted-buffers is nil, kill the
>> +buffers without asking.'"
>
> We use the US English convention of leaving 2 spaces between
> sentences.  Also, I'd simplify the last sentence to avoid repeating
> what the previous one says, and perhaps even make one sentence out of
> the two.

Modified, thanks.

>
>>    (save-excursion (and (cdr dired-subdir-alist)
>> -		       (dired-goto-subdir fn)
>> -		       (dired-kill-subdir)))
>> +                       (dired-goto-subdir fn)
>> +                       (dired-kill-subdir)))
>
> Please don't change whitespace where you aren't changing code.

That slipped past me, sorry!

>
>> +           (or (not dired-clean-confirm-killing-deleted-buffers)
>> +               (funcall #'y-or-n-p
>> +                        (format "Kill buffer of %s, too? "
>> +                                (file-name-nondirectory fn))))
>
> Isn't it better to use this instead:
>
>       (and dired-clean-confirm-killing-deleted-buffers
>            (funcall ...
>
> ?  What you wrote is akin to double negation, IMO.

I changed it to this. I could've sworn this is what I wrote originally and it didn't work, but it seems to work fine now.

>From 452435ac4816b7561056ad0ab75f6387e09b0296 Mon Sep 17 00:00:00 2001
From: Alex Branham <[email-masked]>
Date: Wed, 6 Sep 2017 13:05:38 -0500
Subject: [PATCH] Add dired-confirm-killing-deleted-buffers

* lisp/dired-x.el (dired-clean-confirm-killing-deleted-buffers): New variable.
* lisp/dired.el (dired-clean-up-after-deletion): Just kill buffers
visiting deleted files without confirming if
dired-clean-confirm-killing-deleted-buffers is nil
* etc/NEWS: Document the change

Copyright-paperwork-exempt: yes
---
 etc/NEWS        |  4 ++++
 lisp/dired-x.el |  6 ++++++
 lisp/dired.el   | 22 +++++++++++++---------
 3 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/etc/NEWS b/etc/NEWS
index 2824349a53..4194f307b4 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -594,6 +594,10 @@ this is controlled by the 'wdired-create-parent-directories' variable.
 *** 'W' is now bound to 'browse-url-of-dired-file', and is useful for
 viewing HTML files and the like.
 
++++
+*** New variable 'dired-clean-confirm-killing-deleted-buffers'
+controls whether dired asks to kill buffers visiting deleted files
+
 ---
 ** html2text is now marked obsolete.
 
diff --git a/lisp/dired-x.el b/lisp/dired-x.el
index 1425278bdc..bfb5574da3 100644
--- a/lisp/dired-x.el
+++ b/lisp/dired-x.el
@@ -243,6 +243,12 @@ dired-clean-up-buffers-too
   :type 'boolean
   :group 'dired-x)
 
+(defcustom dired-clean-confirm-killing-deleted-buffers t
+  "If nil, don't ask whether to kill buffers visiting deleted files."
+  :version "26.1"
+  :type 'boolean
+  :group 'dired-x)
+
 ;;; KEY BINDINGS.
 
 (define-key dired-mode-map "\C-x\M-o" 'dired-omit-mode)
diff --git a/lisp/dired.el b/lisp/dired.el
index ff62183f09..33b9e66704 100644
--- a/lisp/dired.el
+++ b/lisp/dired.el
@@ -3167,9 +3167,11 @@ dired-clean-up-buffers-too
 
 (defun dired-clean-up-after-deletion (fn)
   "Clean up after a deleted file or directory FN.
-Removes any expanded subdirectory of deleted directory.
-If `dired-x' is loaded and `dired-clean-up-buffers-too' is non-nil,
-also offers to kill buffers visiting deleted files and directories."
+Removes any expanded subdirectory of deleted directory.  If
+`dired-x' is loaded and `dired-clean-up-buffers-too' is non-nil,
+kill any buffers visiting those files, prompting for
+confirmation.  To disable confirmation, see
+`dired-clean-confirm-killing-deleted-buffers'."
   (save-excursion (and (cdr dired-subdir-alist)
 		       (dired-goto-subdir fn)
 		       (dired-kill-subdir)))
@@ -3177,15 +3179,17 @@ dired-clean-up-after-deletion
   (when (and (featurep 'dired-x) dired-clean-up-buffers-too)
     (let ((buf (get-file-buffer fn)))
       (and buf
-           (funcall #'y-or-n-p
-                    (format "Kill buffer of %s, too? "
-                            (file-name-nondirectory fn)))
+           (and dired-clean-confirm-killing-deleted-buffers
+                (funcall #'y-or-n-p
+                         (format "Kill buffer of %s, too? "
+                                 (file-name-nondirectory fn))))
            (kill-buffer buf)))
     (let ((buf-list (dired-buffers-for-dir (expand-file-name fn))))
       (and buf-list
-           (y-or-n-p (format "Kill Dired buffer%s of %s, too? "
-                             (dired-plural-s (length buf-list))
-                             (file-name-nondirectory fn)))
+           (and dired-clean-confirm-killing-deleted-buffers
+                (y-or-n-p (format "Kill Dired buffer%s of %s, too? "
+                                  (dired-plural-s (length buf-list))
+                                  (file-name-nondirectory fn))))
            (dolist (buf buf-list)
              (kill-buffer buf))))))
 
-- 
2.14.1