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

[Alex Branham]

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.

0002-Add-dired-confirm-killing-deleted-buffers.patch
Description: Text Data