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

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

[Alex Branham]

[-- Attachment #1: Type: text/plain, Size: 433 bytes --]

Hello,

This is a patch to include a new variable that, when set to nil, makes dired kill buffers visiting deleted files without confirmation.

There are apparently some people interested in this:

https://emacs.stackexchange.com/questions/30676/how-to-always-kill-dired-buffer-when-deleting-a-folder

I read through the CONTRIBUTE file, but please let me know if something is wrong with the commit message and I can redo it.

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-dired-confirm-killing-deleted-buffers.patch --]
[-- Type: text/x-diff, Size: 3951 bytes --]

From e8be25d16b6381cafa8b345b0509944e1dc66c68 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham@utexas.edu>
Date: Wed, 6 Sep 2017 10:29:50 -0500
Subject: [PATCH] Add dired-confirm-killing-deleted-buffers

* 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
* etc/NEWS: Document the change

Copyright-paperwork-exempt: yes
---
 etc/NEWS        |  4 ++++
 lisp/dired-x.el |  6 ++++++
 lisp/dired.el   | 28 +++++++++++++++++-----------
 3 files changed, 27 insertions(+), 11 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..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)
 
 (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.'"
   (save-excursion (and (cdr dired-subdir-alist)
-		       (dired-goto-subdir fn)
-		       (dired-kill-subdir)))
+                       (dired-goto-subdir fn)
+                       (dired-kill-subdir)))
   ;; Offer to kill buffer of deleted file FN.
   (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)))
+           (or (not 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)))
+           (or (not 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

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

[Eli Zaretskii]

> From: Alex Branham <alex.branham@gmail.com>
> Date: Wed, 06 Sep 2017 10:42:37 -0500
> 
> This is a patch to include a new variable that, when set to nil, makes dired kill buffers visiting deleted files without confirmation.
> 
> There are apparently some people interested in this:
> 
> https://emacs.stackexchange.com/questions/30676/how-to-always-kill-dired-buffer-when-deleting-a-folder
> 
> I read through the CONTRIBUTE file, but please let me know if something is wrong with the commit message and I can redo it.

Thanks.  A few comments:

> * 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.

> 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?

>  (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.

>    (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.

> +           (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.

Thanks again for working on this.

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

[Alex Branham]

[-- Attachment #1: Type: text/plain, Size: 3157 bytes --]

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 <eliz@gnu.org> 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.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0002-Add-dired-confirm-killing-deleted-buffers.patch --]
[-- Type: text/x-diff, Size: 3692 bytes --]

From 452435ac4816b7561056ad0ab75f6387e09b0296 Mon Sep 17 00:00:00 2001
From: Alex Branham <branham@utexas.edu>
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

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

[Eli Zaretskii]

> From: Alex Branham <alex.branham@gmail.com>
> Cc: 28373@debbugs.gnu.org
> 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.

Thanks, I will look into this soon.

> 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?

Form sent off-list.  Thanks.

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

[Eli Zaretskii]

> From: Alex Branham <alex.branham@gmail.com>
> Cc: 28373@debbugs.gnu.org
> 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.

Thanks, pushed to the master branch.

Two minor nits for the future:

 . please always mention the bug number in the commit log message
 . the NEWS entry should be marked with "+++" only if the new feature
   is described in the appropriate manual; otherwise it should be
   either marked with "---" (if it doesn't need to be in the manual),
   or not marked at all (which is a signal for the maintainers to
   consider documenting it at some future point).  This is explained
   at the beginning of NEWS.