Re: master 1a3bad431d 3/3: Update project-kill-buffer-conditions to match buffer-match-p

[Dmitry Gutov]

Philip, Eli,

On 15.04.2022 11:07, Philip Kaludercic wrote:
> branch: master
> commit 1a3bad431d841e52a61e5f1f09b4ebe7fbbd70da
> Author: Philip Kaludercic<philipk@posteo.net>
> Commit: Philip Kaludercic<philipk@posteo.net>
>
>      Update project-kill-buffer-conditions to match buffer-match-p
>      
>      * project.el (project-kill-buffer-conditions): Document the
>      deprecation of the use of derived-mode
>      (project--buffer-check): Have `major-mode' behave like `derived-mode'
>      did previously, and issue a warning of `derived-mode' is used.
> ---
>   lisp/progmodes/project.el | 40 ++++++++++++++++++++++++----------------
>   1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index daaf86f327..ac6aa0ced2 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1201,18 +1201,22 @@ displayed."
>     (display-buffer-other-frame buffer-or-name))
>   
>   (defcustom project-kill-buffer-conditions
> -  '(buffer-file-name    ; All file-visiting buffers are included.
> +  `(buffer-file-name    ; All file-visiting buffers are included.
>       ;; Most of the temp buffers in the background:
> -    (major-mode . fundamental-mode)
> +    ,(lambda (buf)
> +       (not (eq (buffer-local-value 'major-mode buf)
> +                '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)
> -    (derived-mode . comint-mode)
> -    (derived-mode . eshell-mode)
> -    (derived-mode . change-log-mode))
> +    (and (major-mode . special-mode)

Apparently I missed the whole buffer-match-p discussion.

But whatever made you think it's okay to change the meaning of 
'major-mode' inside project-kill-buffer-conditions defcustom now?

Asking my opinion first would have been nice.

Putting aside the obvious incompleteness of the resulting DSL if the 
user is forced to use lambdas, the previous meaning of 'major-mode' in 
this user option has been nailed down in an Emacs release (28.1). So 
there can be (and probably are) users who have customized this var and 
who will see the resulting behavior change unexpectedly as soon as they 
update to a recent enough snapshot of Emacs 29. No warning is (or can 
be) issued in this case either.

That goes against our usual promise of backward compatibility.

And BTW, this 'not' changes the previous meaning too:

> +    ,(lambda (buf)
> +       (not (eq (buffer-local-value 'major-mode buf)
> +                'fundamental-mode)))

(the established definition had the 'eq' semantics and not 'not eq')

...which would have been more a problem if project--buffer-check 
supported lambdas at all -- it doesn't, so the condition is skipped. The 
relevant check is (symbolp c), which didn't account for lambdas (which 
aren't a great fit for a Custom-editable variable anyway).

So the result of this (apparently untested) change is that "Most of the 
temp buffers in the background" are never killed by project-kill-buffers 
anymore, rather than having *all* buffers inside project root killed. 
Could have been worse, I guess.