GNU bug report logs - #28373
[PATCH] New variable controls whether dired confirms to kill buffers visiting deleted files

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 28373 in the body.
You can then email your comments to 28373 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

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

From: Alex Branham <alex.branham <at> gmail.com>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] New variable controls whether dired confirms to kill buffers
 visiting deleted files
Date: Wed, 06 Sep 2017 10:42:37 -0500

[Message part 1 (text/plain, inline)]

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

[0001-Add-dired-confirm-killing-deleted-buffers.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 28373 <at> debbugs.gnu.org
Subject: Re: bug#28373: [PATCH] New variable controls whether dired confirms to
 kill buffers visiting deleted files
Date: Wed, 06 Sep 2017 19:36:48 +0300

> From: Alex Branham <alex.branham <at> 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.

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

From: Alex Branham <alex.branham <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 28373 <at> debbugs.gnu.org
Subject: Re: bug#28373: [PATCH] New variable controls whether dired confirms
 to kill buffers visiting deleted files
Date: Wed, 06 Sep 2017 13:11:45 -0500

[Message part 1 (text/plain, inline)]

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

[0002-Add-dired-confirm-killing-deleted-buffers.patch (text/x-diff, attachment)]

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

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 28373 <at> debbugs.gnu.org
Subject: Re: bug#28373: [PATCH] New variable controls whether dired confirms
 to kill buffers visiting deleted files
Date: Wed, 06 Sep 2017 21:42:37 +0300

> From: Alex Branham <alex.branham <at> gmail.com>
> Cc: 28373 <at> 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.

Message #19 received at 28373-done <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Alex Branham <alex.branham <at> gmail.com>
Cc: 28373-done <at> debbugs.gnu.org
Subject: Re: bug#28373: [PATCH] New variable controls whether dired confirms
 to kill buffers visiting deleted files
Date: Fri, 08 Sep 2017 12:41:04 +0300

> From: Alex Branham <alex.branham <at> gmail.com>
> Cc: 28373 <at> 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.

Previous Next

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