bug#42386: Acknowledgement ([PATCH] Handle symbols in project-kill-buffers-ignores)

[Eli Zaretskii <eliz@gnu.org>]

> From: "Philip K." <philip@warpmail.net>
> Date: Tue, 21 Jul 2020 11:11:56 +0200
> Cc: 42386@debbugs.gnu.org
> 
> +(defcustom project-kill-buffer-conditions
> +  '(buffer-file-name    ; All file-visiting buffers are included.
> +    ;; Most of the temp buffers in the background:
> +    (major-mode . fundamental-mode)
> +    ;; non-text buffer such as xref, occur, vc, log, ...
> +    (and (derived-mode . special-mode)
> +         (not (major-mode . help-mode)))
> +    (derived-mode . compilation-mode)
> +    (derived-mode . dired-mode)
> +    (derived-mode . diff-mode))
> +  "Conditions for buffers `project-kill-buffers' should kill.

This should tell, somewhere, that these conditions are in addition to
the buffer being related to the project, otherwise someone could
interpret the above as meaning that _all_ the file-visiting buffers
will be killed...

> +Buffers that match any of the conditions will not be killed."

This and the previous sentence ("Conditions for...") contradict each
other.  Are these conditions for killing a buffer, or for NOT killing
it?  And if the former, then I guess the doc string of
project-kill-buffers should be amended accordingly?

> +  :type '(repeat (choice regexp function symbol
> +                         (cons :tag "Major mode"
> +                               (const major-mode) symbol)
> +                         (cons :tag "Derived mode"
> +                               (const derived-mode) symbol)
> +                         (cons :tag "Negation"
> +                               (const not) sexp)
> +                         (cons :tag "Conjunction"
> +                               (const and) sexp)
> +                         (cons :tag "Disjunction"
> +                               (const or) sexp)))
>    :version "28.1"
> -  :package-version '(project . "0.5.0"))
> +  :group 'project
> +  :package-version '(project . "0.6.0"))
>  
>  (defun project--buffer-list (pr)
>    "Return the list of all buffers in project PR."
> @@ -864,6 +900,38 @@ project--buffer-list
>          (push buf bufs)))
>      (nreverse bufs)))
>  
> +(defun project--kill-buffer-check (buf conds)
> +  "Return non-nil, if buffer BUF matches any CONDS.
> +CONDS is a list of conditions. See

It is better to explain what CONDS are in the same 1st line.
Alternatively, rename the argument to 'conditions", then the name will
explain itself naturally (although it might still be a good idea to
say it should be a list).

Also, please be sure to leave 2 spaces between sentences in comments
and doc strings.

>  (defun project-kill-buffers ()
>    "Kill all live buffers belonging to the current project.
> @@ -873,17 +941,13 @@ project-kill-buffers
>    (interactive)
>    (let ((pr (project-current t)) bufs)
>      (dolist (buf (project--buffer-list pr))
> -      (unless (seq-some
> -               (lambda (c)
> -                 (cond ((stringp c)
> -                        (string-match-p c (buffer-name buf)))
> -                       ((functionp c)
> -                        (funcall c buf))))
> -               project-kill-buffers-ignores)
> +      (when (project--kill-buffer-check buf project-kill-buffer-conditions)
>          (push buf bufs)))
> -    (when (yes-or-no-p (format "Kill %d buffers in %s? "
> -                               (length bufs) (project-root pr)))
> -      (mapc #'kill-buffer bufs))))
> +    (if (null bufs)
> +        (message "No buffers to kill")
> +      (when (yes-or-no-p (format "Kill %d buffers in %s? "
> +                                 (length bufs) (project-root pr)))
> +        (mapc #'kill-buffer bufs)))))

Is this function intended to never be invoked from Lisp?  If it can be
invoked from Lisp, then asking the yes-or-no-p question might not be
appropriate in the non-interactive case (and the "No buffers to kill"
message might be spared as well).

Thanks.