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

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

[Philip K.]

Hi,

using project-kill-buffers for the last few weeks I noticed that my ERC
buffers get killed when I started my session in a project. This kicks me
out of all the channels I have joined, which is not intended.

Fixing this is easy, I just have to ignore ERC buffers:

        (add-to-list 'project-kill-buffers-ignores
                     (lambda (buf)
                       (with-current-buffer buf
                         (derived-mode-p 'erc-mode))))

but because I think that this is a common use-case, I implemented a
patch to automatically exclude certain major modes from being treated as
project dependent. The above example could then be simplified to

        (add-to-list 'project-kill-buffers-ignores
                     'erc-mode)

Currently it only checks major modes, but I guess it could be easily
extended to also handle minor modes/any buffer local variables.

-- 
	Philip K.

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

[Philip K.]

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


Didn't properly test the last patch, sorry for that. The previous
version would try to call the major mode initialisation as a predicate.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-symbols-in-project-kill-buffers-ignores.patch --]
[-- Type: text/x-diff, Size: 1867 bytes --]

From 01df630f6bfa8939b2df53f31882e3ef9fd7dfa1 Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Thu, 16 Jul 2020 10:03:35 +0200
Subject: [PATCH] Handle symbols in project-kill-buffers-ignores

---
 lisp/progmodes/project.el | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 67ce3dc7d9..811c18cb56 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -847,10 +847,11 @@ project-kill-buffers-ignores
   '("\\*Help\\*")
   "Conditions for buffers `project-kill-buffers' should not kill.
 Each condition is either a regular expression matching a buffer
-name, or a predicate function that takes a buffer object as
-argument and returns non-nil if it matches.  Buffers that match
-any of the conditions will not be killed."
-  :type '(repeat (choice regexp function))
+name, a predicate function that takes a buffer object as argument
+and returns non-nil if it matches, or a symbol protecting buffers
+of certain major modes.  Buffers that match any of the conditions
+will not be killed."
+  :type '(repeat (choice regexp function symbol))
   :version "28.1"
   :package-version '(project . "0.5.0"))
 
@@ -877,7 +878,12 @@ project-kill-buffers
                (lambda (c)
                  (cond ((stringp c)
                         (string-match-p c (buffer-name buf)))
-                       ((functionp c)
+                       ((and (symbolp c)
+                             (provided-mode-derived-p
+                              (buffer-local-value 'major-mode buf)
+                              c)))
+                       ((and (not (symbolp c))
+                             (functionp c))
                         (funcall c buf))))
                project-kill-buffers-ignores)
         (push buf bufs)))
-- 
2.20.1

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

[Dmitry Gutov]

On 16.07.2020 11:15, Philip K. wrote:
> using project-kill-buffers for the last few weeks I noticed that my ERC
> buffers get killed when I started my session in a project. This kicks me
> out of all the channels I have joined, which is not intended.
> 
> Fixing this is easy, I just have to ignore ERC buffers:
> 
>          (add-to-list 'project-kill-buffers-ignores
>                       (lambda (buf)
>                         (with-current-buffer buf
>                           (derived-mode-p 'erc-mode))))

So there is no way to match the buffers in question using a regexp by name?

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

[Eli Zaretskii]

> From: "Philip K." <philip@warpmail.net>
> Date: Thu, 16 Jul 2020 10:47:43 +0200
> 
>  Each condition is either a regular expression matching a buffer
> -name, or a predicate function that takes a buffer object as
> -argument and returns non-nil if it matches.  Buffers that match
> -any of the conditions will not be killed."
> -  :type '(repeat (choice regexp function))
> +name, a predicate function that takes a buffer object as argument
> +and returns non-nil if it matches, or a symbol protecting buffers
> +of certain major modes.

The part you added to the doc string is unclear: how can a symbol
"protect buffers of certain major modes"?  (I think I can guess what
you mean, but readers of doc strings should be guessing.)

> -                       ((functionp c)
> +                       ((and (symbolp c)
> +                             (provided-mode-derived-p
> +                              (buffer-local-value 'major-mode buf)
> +                              c)))
> +                       ((and (not (symbolp c))
> +                             (functionp c))
>                          (funcall c buf))))

Is this logic correct and reliable?  A function is a symbol, and there
could be symbols that have both a function cell and a variable cell --
how does this cope with that?

And why do you require a function not to be a symbol?  I'm probably
missing something.

Thanks.

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

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 16.07.2020 11:15, Philip K. wrote:
>> using project-kill-buffers for the last few weeks I noticed that my ERC
>> buffers get killed when I started my session in a project. This kicks me
>> out of all the channels I have joined, which is not intended.
>> 
>> Fixing this is easy, I just have to ignore ERC buffers:
>> 
>>          (add-to-list 'project-kill-buffers-ignores
>>                       (lambda (buf)
>>                         (with-current-buffer buf
>>                           (derived-mode-p 'erc-mode))))
>
> So there is no way to match the buffers in question using a regexp by name?

Not in this case, because an ERC buffer can have any name (direct
conversations don't start with a "#").

-- 
	Philip K.

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

[Philip K.]

-- 
	Philip K.

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

[Philip K.]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Philip K." <philip@warpmail.net>
>> Date: Thu, 16 Jul 2020 10:47:43 +0200
>> 
>>  Each condition is either a regular expression matching a buffer
>> -name, or a predicate function that takes a buffer object as
>> -argument and returns non-nil if it matches.  Buffers that match
>> -any of the conditions will not be killed."
>> -  :type '(repeat (choice regexp function))
>> +name, a predicate function that takes a buffer object as argument
>> +and returns non-nil if it matches, or a symbol protecting buffers
>> +of certain major modes.
>
> The part you added to the doc string is unclear: how can a symbol
> "protect buffers of certain major modes"?  (I think I can guess what
> you mean, but readers of doc strings should be guessing.)

You're right, I'll think about rephrasing that.

>> -                       ((functionp c)
>> +                       ((and (symbolp c)
>> +                             (provided-mode-derived-p
>> +                              (buffer-local-value 'major-mode buf)
>> +                              c)))
>> +                       ((and (not (symbolp c))
>> +                             (functionp c))
>>                          (funcall c buf))))
>
> Is this logic correct and reliable?  A function is a symbol, and there
> could be symbols that have both a function cell and a variable cell --
> how does this cope with that?
>
> And why do you require a function not to be a symbol?  I'm probably
> missing something.

My idea was to only allow lambda expressions, but that would make it
hard to implement more complex predicates that should be defined in
their own defuns.

How about this: Instead of symbols, adding a cons-cell:

    (major-mode . erc-mode)

prevents erc-buffers from being killed?

> Thanks.
>

-- 
	Philip K.

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

[Dmitry Gutov]

On 16.07.2020 18:14, Eli Zaretskii wrote:
> Is this logic correct and reliable?  A function is a symbol, and there
> could be symbols that have both a function cell and a variable cell --
> how does this cope with that?
> 
> And why do you require a function not to be a symbol?  I'm probably
> missing something.

There is little possibility of conflict here, al least if we redo the 
implementation a little (major modes rarely match predicate functions by 
name, so these kind of conditions could coexist), but I'm worried about 
the non-obvious semantics.

Do we have existing variables that offer similar functionality?

I have used font-lock-global-modes as an example in the past, but it 
doesn't support free-form predicates or matching by buffer name.

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

[Eli Zaretskii]

> From: "Philip K." <philip@warpmail.net>
> Cc: 42386@debbugs.gnu.org
> Date: Thu, 16 Jul 2020 20:16:40 +0200
> 
> > And why do you require a function not to be a symbol?  I'm probably
> > missing something.
> 
> My idea was to only allow lambda expressions, but that would make it
> hard to implement more complex predicates that should be defined in
> their own defuns.

Right, I don't think we should disallow named functions.

> How about this: Instead of symbols, adding a cons-cell:
> 
>     (major-mode . erc-mode)
> 
> prevents erc-buffers from being killed?

You are trying to separate modes from other functions?  Why?  Just see
if the major-mode's symbol is in the list, and if so, spare the
buffer.  Otherwise, if it's a function, call that function assuming
it's a predicate.  Does this present some problems?

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

[Philip K.]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> How about this: Instead of symbols, adding a cons-cell:
>> 
>>     (major-mode . erc-mode)
>> 
>> prevents erc-buffers from being killed?
>
> You are trying to separate modes from other functions?  Why?  

I figured it would keep the structure simpler, that way it wouldn't be
necessary to distinguish between major modes and predicates. I'd also
see it reducing the ambiguity from the user-side.

> Just see if the major-mode's symbol is in the list, and if so, spare
> the buffer.  Otherwise, if it's a function, call that function
> assuming it's a predicate.  Does this present some problems?

Hmm, it seems to work, but my initial attempt doesn't look that
nice. I'll think about improving it tomorrow, but I'm still more
attracted to the (major-mode . mode-name) approach.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-symbols-in-project-kill-buffers-ignores.patch --]
[-- Type: text/x-diff, Size: 2771 bytes --]

From d7b8357942fcfb33d9b8f3dbef1aefb6849e05d4 Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Thu, 16 Jul 2020 10:03:35 +0200
Subject: [PATCH] Handle symbols in project-kill-buffers-ignores

---
 lisp/progmodes/project.el | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 67ce3dc7d9..7594fe18ac 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -844,13 +844,14 @@ project-switch-to-buffer
       predicate))))
 
 (defcustom project-kill-buffers-ignores
-  '("\\*Help\\*")
+  '(emacs-lisp-mode "\\*Help\\*")
   "Conditions for buffers `project-kill-buffers' should not kill.
 Each condition is either a regular expression matching a buffer
-name, or a predicate function that takes a buffer object as
-argument and returns non-nil if it matches.  Buffers that match
-any of the conditions will not be killed."
-  :type '(repeat (choice regexp function))
+name, a predicate function that takes a buffer object as argument
+and returns non-nil if it matches, or a symbol that prevents
+buffers from being killed, if it equals buffer's major mode.
+Buffers that match any of the conditions will not be killed."
+  :type '(repeat (choice regexp function symbol))
   :version "28.1"
   :package-version '(project . "0.5.0"))
 
@@ -873,17 +874,22 @@ 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)
+      (unless (or (memq (buffer-local-value 'major-mode buf)
+                        project-kill-buffers-ignores)
+                  (seq-some
+                   (lambda (c)
+                     (cond ((stringp c)
+                            (string-match-p c (buffer-name buf)))
+                           ((and (not (memq c project-kill-buffers-ignores))
+                                 (functionp c))
+                            (funcall c buf))))
+                   project-kill-buffers-ignores))
         (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)))))
 
 \f
 ;;; Project list
-- 
2.20.1

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

[Juri Linkov]

> Do we have existing variables that offer similar functionality?
>
> I have used font-lock-global-modes as an example in the past, but it
> doesn't support free-form predicates or matching by buffer name.

Another example is desktop.el with distinct options
desktop-buffers-not-to-save and desktop-modes-not-to-save.
So maybe this needs a new option project-kill-buffers-ignore-modes.

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

[Dmitry Gutov]

On 17.07.2020 01:46, Juri Linkov wrote:
>> Do we have existing variables that offer similar functionality?
>>
>> I have used font-lock-global-modes as an example in the past, but it
>> doesn't support free-form predicates or matching by buffer name.
> Another example is desktop.el with distinct options
> desktop-buffers-not-to-save and desktop-modes-not-to-save.
> So maybe this needs a new option project-kill-buffers-ignore-modes.

Thanks.

It's a curious example, but with all three variables (-buffers-, 
-files-, -modes-), it still doesn't support arbitrary predicates.

Using a cons as suggested by Philip seems like a better option.

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

[Eli Zaretskii]

> From: "Philip K." <philip@warpmail.net>
> Cc: 42386@debbugs.gnu.org
> Date: Fri, 17 Jul 2020 00:22:56 +0200
> 
>  Each condition is either a regular expression matching a buffer
> -name, or a predicate function that takes a buffer object as
> -argument and returns non-nil if it matches.  Buffers that match
> -any of the conditions will not be killed."
> -  :type '(repeat (choice regexp function))
> +name, a predicate function that takes a buffer object as argument
> +and returns non-nil if it matches, or a symbol that prevents
> +buffers from being killed, if it equals buffer's major mode.

This is better, but I think you will be able to make it better yet if
you divide that long sentence into several ones.  Like this:

  Buffers that match any of the conditions will not be killed.
  Each condition can be one of the following:

   - a regular expression, to match against the buffer's name;
   - a predicate function...
   ...

> +                   (lambda (c)
> +                     (cond ((stringp c)
> +                            (string-match-p c (buffer-name buf)))
> +                           ((and (not (memq c project-kill-buffers-ignores))
> +                                 (functionp c))

Once again, I don't think I understand why you need the memq part.

Thanks.

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

[Philip K.]

Eli Zaretskii <eliz@gnu.org> writes:

>> +                   (lambda (c)
>> +                     (cond ((stringp c)
>> +                            (string-match-p c (buffer-name buf)))
>> +                           ((and (not (memq c project-kill-buffers-ignores))
>> +                                 (functionp c))
>
> Once again, I don't think I understand why you need the memq part.

The issue is that a major mode symbol is interpreted as a function, and
that triggers an error. So if I want to keep shell-mode buffers, and I'm
checking an emacs-lisp-mode buffer, the major mode isn't part of
project-kill-buffers-ignores (-> first memq), so the seq-some is
evaluated, and because shell-mode is a function, the code would try to
evaluate it as a predicate, but that fails.

I realise just now how stupid the memq is, because instead of stopping
major-mode symbols from being evaluated, I just stop everything, as
every member of the list is tautologically part of the list.

I don't see any way around this, except for trying to analyse the
symbols string representation or checking the arity if the
function. Neither seem like a particularly clean approach.

-- 
	Philip K.

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

[Eli Zaretskii]

> From: "Philip K." <philip@warpmail.net>
> Cc: 42386@debbugs.gnu.org
> Date: Fri, 17 Jul 2020 10:16:59 +0200
> 
> I don't see any way around this, except for trying to analyse the
> symbols string representation or checking the arity if the
> function. Neither seem like a particularly clean approach.

Dmitry suggested a cons cell, which will allow you to differentiate
between predicates and major-modes.  Isn't that a way around the
problem?

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

[Philip K.]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Philip K." <philip@warpmail.net>
>> Cc: 42386@debbugs.gnu.org
>> Date: Fri, 17 Jul 2020 10:16:59 +0200
>> 
>> I don't see any way around this, except for trying to analyse the
>> symbols string representation or checking the arity if the
>> function. Neither seem like a particularly clean approach.
>
> Dmitry suggested a cons cell, which will allow you to differentiate
> between predicates and major-modes.  Isn't that a way around the
> problem?

That's what I had in mind when I wrote:

> How about this: Instead of symbols, adding a cons-cell:
> 
>     (major-mode . erc-mode)
> 
> prevents erc-buffers from being killed?

So if you're ok with that, I'll try submitting a patch with that
approach.

-- 
	Philip K.

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

[Eli Zaretskii]

> From: "Philip K." <philip@warpmail.net>
> Cc: 42386@debbugs.gnu.org
> Date: Fri, 17 Jul 2020 13:17:17 +0200
> 
> > Dmitry suggested a cons cell, which will allow you to differentiate
> > between predicates and major-modes.  Isn't that a way around the
> > problem?
> 
> That's what I had in mind when I wrote:
> 
> > How about this: Instead of symbols, adding a cons-cell:
> > 
> >     (major-mode . erc-mode)
> > 
> > prevents erc-buffers from being killed?
> 
> So if you're ok with that, I'll try submitting a patch with that
> approach.

Sure, I'm okay.  Especially since Dmitry seems also okay with it.

Thanks.

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

[Philip K.]

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


The patch below should implement that behaviour + an updating docstring.

Eli Zaretskii <eliz@gnu.org> writes:

>> From: "Philip K." <philip@warpmail.net>
>> Cc: 42386@debbugs.gnu.org
>> Date: Fri, 17 Jul 2020 13:17:17 +0200
>> 
>> > Dmitry suggested a cons cell, which will allow you to differentiate
>> > between predicates and major-modes.  Isn't that a way around the
>> > problem?
>> 
>> That's what I had in mind when I wrote:
>> 
>> > How about this: Instead of symbols, adding a cons-cell:
>> > 
>> >     (major-mode . erc-mode)
>> > 
>> > prevents erc-buffers from being killed?
>> 
>> So if you're ok with that, I'll try submitting a patch with that
>> approach.
>
> Sure, I'm okay.  Especially since Dmitry seems also okay with it.
>
> Thanks.
>

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Handle-symbols-in-project-kill-buffers-ignores.patch --]
[-- Type: text/x-diff, Size: 2580 bytes --]

From 4497ef69ed27fff7979966ece8803be1f8918874 Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Thu, 16 Jul 2020 10:03:35 +0200
Subject: [PATCH] Handle symbols in project-kill-buffers-ignores

---
 lisp/progmodes/project.el | 31 ++++++++++++++++++++++---------
 1 file changed, 22 insertions(+), 9 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 67ce3dc7d9..a73ab8ce8a 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -846,11 +846,18 @@ project-switch-to-buffer
 (defcustom project-kill-buffers-ignores
   '("\\*Help\\*")
   "Conditions for buffers `project-kill-buffers' should not kill.
-Each condition is either a regular expression matching a buffer
-name, or a predicate function that takes a buffer object as
-argument and returns non-nil if it matches.  Buffers that match
-any of the conditions will not be killed."
-  :type '(repeat (choice regexp function))
+Each condition is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object as argument
+  and returns non-nil if the buffer should not be killed,
+- a cons-cell, where the car describes how to interpret the cdr.
+  If the car contains `major-mode', the cdr has to be the symbol
+  of a major mode, that should never be killed.
+
+Buffers that match any of the conditions will not be killed."
+  :type '(repeat (choice regexp function
+                         (cons :tag "Major mode"
+                               (const major-mode) symbol)))
   :version "28.1"
   :package-version '(project . "0.5.0"))
 
@@ -878,12 +885,18 @@ project-kill-buffers
                  (cond ((stringp c)
                         (string-match-p c (buffer-name buf)))
                        ((functionp c)
-                        (funcall c buf))))
+                        (funcall c buf))
+                       ((eq (car-safe c) 'major-mode)
+                        (provided-mode-derived-p
+                         (buffer-local-value 'major-mode buf)
+                         (cdr c)))))
                project-kill-buffers-ignores)
         (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)))))
 
 \f
 ;;; Project list
-- 
2.20.1

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

[Dmitry Gutov]

On 17.07.2020 18:30, Philip K. wrote:
>   (defcustom project-kill-buffers-ignores
>     '("\\*Help\\*")

You might as well update the default value.

> +- a cons-cell, where the car describes how to interpret the cdr.
> +  If the car contains `major-mode', the cdr has to be the symbol
> +  of a major mode, that should never be killed.

...and its derivatives?

Or I wonder if you should just use memq in the implementation.

Both font-lock-global-modes and desktop-modes-not-to-save are not 
applied to derivatives.

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

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 17.07.2020 18:30, Philip K. wrote:
>>   (defcustom project-kill-buffers-ignores
>>     '("\\*Help\\*")
>
> You might as well update the default value.

So what should be added, besides ERC?

>> +- a cons-cell, where the car describes how to interpret the cdr.
>> +  If the car contains `major-mode', the cdr has to be the symbol
>> +  of a major mode, that should never be killed.
>
> ...and its derivatives?

I'll add that.

> Or I wonder if you should just use memq in the implementation.

I tried to find out what major-mode hierarchies exist on my system, so I
wrote

        (let (tree)
          (dolist (f features)
            (require f))
          (mapatoms
           (lambda (a)
             (let ((p (get a 'derived-mode-parent)))
               (when p
                 (push (cons p a)
                       tree)))))
          (with-temp-buffer
            (insert "digraph {\n")
            (dolist (node tree)
              (insert (format "\"%s\" -> \"%s\";\n" (car node) (cdr node))))
            (insert "}\n")
            (write-file "dep.dot")
            (shell-command "dot -Tpng dep.dot > dep.png")
            (delete-file "dep.dot")))

that generated an image of a tree. It seems it's mostly flat, meaning
that a majority of the major modes are based on prog-mode, text-mode or
special-mode. All further levels seem to be connected, such as with
magit or gnus. So while I get that someone might not find it intuitive
that major modes other than the one listed are not killed, I think a
generous interpretation is better than killing more than one might want.

> Both font-lock-global-modes and desktop-modes-not-to-save are not 
> applied to derivatives.

Hmm, I didn't know about that. As explained, just from looking at my
system, taking derivations into consideration appears to have more
advantages than disadvantages, but I'm not dogmatic on that point.

-- 
	Philip K.

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

[Dmitry Gutov]

On 17.07.2020 20:16, Philip K. wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> On 17.07.2020 18:30, Philip K. wrote:
>>>    (defcustom project-kill-buffers-ignores
>>>      '("\\*Help\\*")
>>
>> You might as well update the default value.
> 
> So what should be added, besides ERC?

Probably nothing, unless you have something else to propose.

At this point, you are the main driver of this feature. I like it in 
theory (a lot), but so far has done little to incorporate in my 
workflow, so you're the foremost person to hit the edge cases. I also 
don't do email/IRC/notes/etc in Emacs.

Speaking of some ideas, though, if you are worried about more unknown 
modes needing the same treatment, we could flip the meaning of this var 
and go with the whitelist approach, like font-lock-global-modes does. It 
can still serve as a blacklist if the first element is `not`.

To give an example:

(defcustom project-kill-buffers-conditions
   '(buffer-file-name ; All file-visiting buffers are included.
     (derived-mode . compilation-mode)
     ;; Most of the temp buffers in the background:
     (major-mode . fundamental-mode)
     ;; A bit questionable (alternatively, include
     ;; xref--xref-buffer-mode, occur-mode,
     ;; vc-dir-mode, log-view-mode, log-edit-mode separately):
     (derived-mode . special-mode))
   "Conditions for buffers `project-kill-buffers' should kill.
Each condition is either a regular expression matching a buffer
name, or a predicate function that takes a buffer object as
argument and returns non-nil if it matches, or a cons cell which <...>.

Buffers that belong to the current project, and match any of the
conditions, will be killed.  If the list starts with `not',
the meaning is negated."
   :type '(repeat (choice regexp function))
   :version "28.1"
   :package-version '(project . "0.6.0"))

If this kind of list can be exhaustive enough, this can be a decent default.

What do you think? If you're not sure, let's go with your patch now.

>> Or I wonder if you should just use memq in the implementation.
> 
> I tried to find out what major-mode hierarchies exist on my system, so I
> wrote
> 
>          (let (tree)
>            (dolist (f features)
>              (require f))
>            (mapatoms
>             (lambda (a)
>               (let ((p (get a 'derived-mode-parent)))
>                 (when p
>                   (push (cons p a)
>                         tree)))))
>            (with-temp-buffer
>              (insert "digraph {\n")
>              (dolist (node tree)
>                (insert (format "\"%s\" -> \"%s\";\n" (car node) (cdr node))))
>              (insert "}\n")
>              (write-file "dep.dot")
>              (shell-command "dot -Tpng dep.dot > dep.png")
>              (delete-file "dep.dot")))
> 
> that generated an image of a tree. It seems it's mostly flat, meaning
> that a majority of the major modes are based on prog-mode, text-mode or
> special-mode. All further levels seem to be connected, such as with
> magit or gnus. So while I get that someone might not find it intuitive
> that major modes other than the one listed are not killed, I think a
> generous interpretation is better than killing more than one might want.

Fair enough. Or we could provide both kinds of checks, like in the 
example above (major-mode vs derived-mode).

>> Both font-lock-global-modes and desktop-modes-not-to-save are not
>> applied to derivatives.
> 
> Hmm, I didn't know about that. As explained, just from looking at my
> system, taking derivations into consideration appears to have more
> advantages than disadvantages, but I'm not dogmatic on that point.

I don't know which is the best approach either.

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

[Philip K.]

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

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 17.07.2020 20:16, Philip K. wrote:
>> Dmitry Gutov <dgutov@yandex.ru> writes:
>> 
>>> On 17.07.2020 18:30, Philip K. wrote:
>>>>    (defcustom project-kill-buffers-ignores
>>>>      '("\\*Help\\*")
>>>
>>> You might as well update the default value.
>> 
>> So what should be added, besides ERC?
>
> Probably nothing, unless you have something else to propose.
>
> At this point, you are the main driver of this feature. I like it in 
> theory (a lot), but so far has done little to incorporate in my 
> workflow, so you're the foremost person to hit the edge cases. I also 
> don't do email/IRC/notes/etc in Emacs.
>
> Speaking of some ideas, though, if you are worried about more unknown 
> modes needing the same treatment, we could flip the meaning of this var 
> and go with the whitelist approach, like font-lock-global-modes does. It 
> can still serve as a blacklist if the first element is `not`.
>
> To give an example:
>
> (defcustom project-kill-buffers-conditions
>    '(buffer-file-name ; All file-visiting buffers are included.
>      (derived-mode . compilation-mode)
>      ;; Most of the temp buffers in the background:
>      (major-mode . fundamental-mode)
>      ;; A bit questionable (alternatively, include
>      ;; xref--xref-buffer-mode, occur-mode,
>      ;; vc-dir-mode, log-view-mode, log-edit-mode separately):
>      (derived-mode . special-mode))
>    "Conditions for buffers `project-kill-buffers' should kill.
> Each condition is either a regular expression matching a buffer
> name, or a predicate function that takes a buffer object as
> argument and returns non-nil if it matches, or a cons cell which <...>.
>
> Buffers that belong to the current project, and match any of the
> conditions, will be killed.  If the list starts with `not',
> the meaning is negated."
>    :type '(repeat (choice regexp function))
>    :version "28.1"
>    :package-version '(project . "0.6.0"))
>
> If this kind of list can be exhaustive enough, this can be a decent default.
>
> What do you think? If you're not sure, let's go with your patch now.

I like this idea a lot, the patch below should implement this +
backwards compatibility code. Thought this might be getting too
complicated, I also went ahead and added "and" and "or".

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Replace-project-kill-buffers-ignores-with-.-kill-buf.patch --]
[-- Type: text/x-diff, Size: 6296 bytes --]

From 6a9c268a340025bca428b5ec7c35229a29b4a95f Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Thu, 16 Jul 2020 10:03:35 +0200
Subject: [PATCH] Replace project-kill-buffers-ignores with
 ...-kill-buffer-conditions

---
 lisp/progmodes/project.el | 119 ++++++++++++++++++++++++++++++++------
 1 file changed, 100 insertions(+), 19 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 67ce3dc7d9..869401606a 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -843,16 +843,66 @@ project-switch-to-buffer
       nil
       predicate))))
 
-(defcustom project-kill-buffers-ignores
-  '("\\*Help\\*")
-  "Conditions for buffers `project-kill-buffers' should not kill.
-Each condition is either a regular expression matching a buffer
-name, or a predicate function that takes a buffer object as
-argument and returns non-nil if it matches.  Buffers that match
-any of the conditions will not be killed."
-  :type '(repeat (choice regexp function))
+(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, ...
+    (derived-mode . special-mode)
+    (derived-mode . compilation-mode)
+    (derived-mode . dired-mode)
+    (derived-mode . diff-mode))
+  "Conditions for buffers `project-kill-buffers' should kill.
+Each condition is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object as argument
+  and returns non-nil if the buffer should be killed,
+- a symbol, denoting a buffer local variable, where the buffer
+  is killed if it's value is non-nil. If the symbol also has a
+  function slot, it will be interpreted as a function first.
+- a cons-cell, where the car describes how to interpret the cdr.
+  The car can be one of the following:
+  * `major-mode': the buffer is killed if the buffers major
+    mode is eq to the cons-cell's cdr
+  * `defived-mode': the buffer is killed if the buffers major
+    mode is derived from the major mode denoted by the cons-cell's
+    cdr
+  * `not': the cdr is interpreted as a negation of a condition.
+  * `and': the cdr is a list of recursive conditions, that all have
+    to be met.
+  * `or': the cdr is a list of recursive conditions, of which at
+    least one has to be met.
+
+Buffers that match any of the conditions will not be killed."
+  :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"))
+
+(defcustom project-kill-buffers-ignores nil
+  "Conditions for buffers `project-kill-buffers' should not kill."
+  :type '(repeat choice regexp function)
+  :set (lambda (var val)
+         (add-to-list 'project-kill-buffer-conditions
+                      (cons 'not val))
+         (custom-set-default var val))
+  :version "28.1"
+  :group 'project
+  :package-version '(project . "0.6.0")))
+
+(make-obsolete-variable 'project-kill-buffers-ignores
+                        'project-kill-buffer-conditions
+                        "0.6.0")
 
 (defun project--buffer-list (pr)
   "Return the list of all buffers in project PR."
@@ -864,6 +914,41 @@ project--buffer-list
         (push buf bufs)))
     (nreverse bufs)))
 
+(defun project--kill-buffer-check (buf &optional conds)
+  "Throw"
+  (unless conds
+    (setq conds project-kill-buffer-conditions))
+  (catch (if (eq project-kill-buffer-conditions conds)
+             'kill 'other)
+    (dolist (c conds)
+      (when (cond
+             ((stringp c)
+              (string-match-p c (buffer-name buf)))
+             ((and (functionp c)
+                   (ignore-errors (funcall c buf))))
+             ((and (symbolp c) (boundp c))
+              (buffer-local-value c buf))
+             ((eq (car-safe c) 'major-mode)
+              (eq (buffer-local-value 'major-mode buf)
+                  (cdr c)))
+             ((eq (car-safe c) 'derived-mode)
+              (provided-mode-derived-p
+               (buffer-local-value 'major-mode buf)
+               (cdr c)))
+             ((eq (car-safe c) 'not)
+              (not (project--kill-buffer-check buf (cdr c))))
+             ((eq (car-safe c) 'and)
+              (seq-every-p
+               (apply-partially #'project--kill-buffer-check
+                                buf)
+               (cdr c)))
+             ((eq (car-safe c) 'or)
+              (seq-some
+               (apply-partially #'project--kill-buffer-check
+                                buf)
+               (cdr c))))
+        (throw 'kill t)))))
+
 ;;;###autoload
 (defun project-kill-buffers ()
   "Kill all live buffers belonging to the current project.
@@ -873,17 +958,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)
         (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)))))
 
 \f
 ;;; Project list
-- 
2.20.1

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

[Dmitry Gutov]

On 18.07.2020 15:48, Philip K. wrote:
> I like this idea a lot, the patch below should implement this +
> backwards compatibility code. Thought this might be getting too
> complicated, I also went ahead and added "and" and "or".

All right. That's a bit further than I expected, but the result is still 
fast in the default scenario, so why not. ;-)

I take it this approach behaved well enough in your testing?

Should we replace

   (derived-mode . special-mode)

with

   (and (derived-mode . special-mode)
        (not (major-mode . help-mode)))

?

Some other minor comments below.

> 0001-Replace-project-kill-buffers-ignores-with-.-kill-buf.patch
> 
>  From 6a9c268a340025bca428b5ec7c35229a29b4a95f Mon Sep 17 00:00:00 2001
> From: Philip K<philip@warpmail.net>
> Date: Thu, 16 Jul 2020 10:03:35 +0200
> Subject: [PATCH] Replace project-kill-buffers-ignores with
>   ...-kill-buffer-conditions

Full commit message, if you can.

> +- a symbol, denoting a buffer local variable, where the buffer
> +  is killed if it's value is non-nil. If the symbol also has a
> +  function slot, it will be interpreted as a function first.

This also introduces an ambiguity which I'd like to avoid. Let's just 
make it if a symbol is there, it must be a function (and we should 
silence its errors).

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

Will be. I think.

> +(defcustom project-kill-buffers-ignores nil
> +  "Conditions for buffers `project-kill-buffers' should not kill."
> +  :type '(repeat choice regexp function)
> +  :set (lambda (var val)
> +         (add-to-list 'project-kill-buffer-conditions
> +                      (cons 'not val))
> +         (custom-set-default var val))
> +  :version "28.1"
> +  :group 'project
> +  :package-version '(project . "0.6.0")))

Nice thought, but I think we're allowed to simply do away with this 
variable.

At least I have been informed that as long as the package version 
haven't been in a "proper" Emacs release, its contents don't have the 
same backward compatibility promise.

> +(defun project--kill-buffer-check (buf &optional conds)
> +  "Throw"

Just so you don't forget to update or delete this docstring.

> +  (unless conds
> +    (setq conds project-kill-buffer-conditions))

I think we can make the CONDS argument required and pass in this value 
from project-kill-buffers.

> +  (catch (if (eq project-kill-buffer-conditions conds)
> +             'kill 'other)

Do we really need this condition?

> +    (dolist (c conds)
> +      (when (cond
> +             ((stringp c)
> +              (string-match-p c (buffer-name buf)))
> +             ((and (functionp c)

Let's just make this (symbolp c).

> +                   (ignore-errors (funcall c buf))))

and remove the 'ignore-errors' form. It's better to inform the user 
right away that their predicate is broken.

> +             ((and (symbolp c) (boundp c))
> +              (buffer-local-value c buf))

And remove this case.

> +             ((eq (car-safe c) 'major-mode)
> +              (eq (buffer-local-value 'major-mode buf)
> +                  (cdr c)))
> +             ((eq (car-safe c) 'derived-mode)
> +              (provided-mode-derived-p
> +               (buffer-local-value 'major-mode buf)
> +               (cdr c)))
> +             ((eq (car-safe c) 'not)
> +              (not (project--kill-buffer-check buf (cdr c))))
> +             ((eq (car-safe c) 'and)
> +              (seq-every-p
> +               (apply-partially #'project--kill-buffer-check
> +                                buf)
> +               (cdr c)))
> +             ((eq (car-safe c) 'or)
> +              (seq-some
> +               (apply-partially #'project--kill-buffer-check
> +                                buf)

I think we can simply recurse in this case.

Thanks!

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

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 18.07.2020 15:48, Philip K. wrote:
>> I like this idea a lot, the patch below should implement this +
>> backwards compatibility code. Thought this might be getting too
>> complicated, I also went ahead and added "and" and "or".
>
> All right. That's a bit further than I expected, but the result is still 
> fast in the default scenario, so why not. ;-)
>
> I take it this approach behaved well enough in your testing?
>
> Should we replace
>
>    (derived-mode . special-mode)
>
> with
>
>    (and (derived-mode . special-mode)
>         (not (major-mode . help-mode)))
>
> ?

Sounds good.

>> +- a symbol, denoting a buffer local variable, where the buffer
>> +  is killed if it's value is non-nil. If the symbol also has a
>> +  function slot, it will be interpreted as a function first.
>
> This also introduces an ambiguity which I'd like to avoid. Let's just 
> make it if a symbol is there, it must be a function (and we should 
> silence its errors).

Whoops, I forgot that "buffer-file-name" was a function too. Will remove
it then.

>> +  (catch (if (eq project-kill-buffer-conditions conds)
>> +             'kill 'other)
>
> Do we really need this condition?

No, my intention was to avoid throw-catch'ing up the call-stack, step by
step, but considering the complexity of an average condition, this will
probably not even make any difference -- I'll remove it.

>> +             ((eq (car-safe c) 'major-mode)
>> +              (eq (buffer-local-value 'major-mode buf)
>> +                  (cdr c)))
>> +             ((eq (car-safe c) 'derived-mode)
>> +              (provided-mode-derived-p
>> +               (buffer-local-value 'major-mode buf)
>> +               (cdr c)))
>> +             ((eq (car-safe c) 'not)
>> +              (not (project--kill-buffer-check buf (cdr c))))
>> +             ((eq (car-safe c) 'and)
>> +              (seq-every-p
>> +               (apply-partially #'project--kill-buffer-check
>> +                                buf)
>> +               (cdr c)))
>> +             ((eq (car-safe c) 'or)
>> +              (seq-some
>> +               (apply-partially #'project--kill-buffer-check
>> +                                buf)
>
> I think we can simply recurse in this case.

I feel stupid for asking, but what do you mean? Am I not recursively
calling project--kill-buffer-check?

-- 
	Philip K.

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

[Dmitry Gutov]

On 20.07.2020 15:07, Philip K. wrote:
>>> +             ((eq (car-safe c) 'or)
>>> +              (seq-some
>>> +               (apply-partially #'project--kill-buffer-check
>>> +                                buf)
>> I think we can simply recurse in this case.
> I feel stupid for asking, but what do you mean? Am I not recursively
> calling project--kill-buffer-check?

Just

   (project--kill-buffer-check buf (cdr c))

instead of using apply-partially since we use the 'or' semantics by 
default already in there.

It's not a big thing, and can be changed later anyway.

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

[Philip K.]

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

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 20.07.2020 15:07, Philip K. wrote:
>>>> +             ((eq (car-safe c) 'or)
>>>> +              (seq-some
>>>> +               (apply-partially #'project--kill-buffer-check
>>>> +                                buf)
>>> I think we can simply recurse in this case.
>> I feel stupid for asking, but what do you mean? Am I not recursively
>> calling project--kill-buffer-check?
>
> Just
>
>    (project--kill-buffer-check buf (cdr c))
>
> instead of using apply-partially since we use the 'or' semantics by 
> default already in there.
>
> It's not a big thing, and can be changed later anyway.

Oh yes, I missed that.

Anyways, the updated patch is attached below.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-project-kill-buffer-conditions.patch --]
[-- Type: text/x-diff, Size: 5746 bytes --]

From f054c4983f9a90d8bf411f6d06c327a3fa382f4b Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Mon, 20 Jul 2020 21:20:34 +0200
Subject: [PATCH] Add project-kill-buffer-conditions

This replaces its negation, project-kill-buffers-ignores, from the
previous version.
---
 lisp/progmodes/project.el | 102 +++++++++++++++++++++++++++++++-------
 1 file changed, 83 insertions(+), 19 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 67ce3dc7d9..50c164ebc7 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -843,16 +843,52 @@ project-switch-to-buffer
       nil
       predicate))))
 
-(defcustom project-kill-buffers-ignores
-  '("\\*Help\\*")
-  "Conditions for buffers `project-kill-buffers' should not kill.
-Each condition is either a regular expression matching a buffer
-name, or a predicate function that takes a buffer object as
-argument and returns non-nil if it matches.  Buffers that match
-any of the conditions will not be killed."
-  :type '(repeat (choice regexp function))
+(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.
+Each condition is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object as argument
+  and returns non-nil if the buffer should be killed,
+- a symbol, denoting a buffer local variable, where the buffer
+  is killed if it's value is non-nil. If the symbol also has a
+  function slot, it will be interpreted as a function first.
+- a cons-cell, where the car describes how to interpret the cdr.
+  The car can be one of the following:
+  * `major-mode': the buffer is killed if the buffers major
+    mode is eq to the cons-cell's cdr
+  * `defived-mode': the buffer is killed if the buffers major
+    mode is derived from the major mode denoted by the cons-cell's
+    cdr
+  * `not': the cdr is interpreted as a negation of a condition.
+  * `and': the cdr is a list of recursive conditions, that all have
+    to be met.
+  * `or': the cdr is a list of recursive conditions, of which at
+    least one has to be met.
+
+Buffers that match any of the conditions will not be killed."
+  :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
+`project-kill-buffer-conditions' for more details."
+  (catch 'kill
+    (dolist (c conds)
+      (when (cond
+             ((stringp c)
+              (string-match-p c (buffer-name buf)))
+             ((symbolp c)
+              (funcall c buf))
+             ((eq (car-safe c) 'major-mode)
+              (eq (buffer-local-value 'major-mode buf)
+                  (cdr c)))
+             ((eq (car-safe c) 'derived-mode)
+              (provided-mode-derived-p
+               (buffer-local-value 'major-mode buf)
+               (cdr c)))
+             ((eq (car-safe c) 'not)
+              (not (project--kill-buffer-check buf (cdr c))))
+             ((eq (car-safe c) 'and)
+              (seq-every-p
+               (apply-partially #'project--kill-buffer-check
+                                buf)
+               (cdr c)))
+             ((eq (car-safe c) 'or)
+              (seq-some
+               (apply-partially #'project--kill-buffer-check
+                                buf)
+               (cdr c))))
+        (throw 'kill t)))))
+
 ;;;###autoload
 (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)))))
 
 \f
 ;;; Project list
-- 
2.20.1

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

[Eli Zaretskii]

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

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

[Philip K.]

Thanks for the input, I'll try to fix everything you mentioned.

Eli Zaretskii <eliz@gnu.org> writes:

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

So would you suggest factoring out that programmatic part or checking if
the command was invoked interactively?

> (and the "No buffers to kill" message might be spared as well).

I added "No buffers to kill", because if you're in a buffer that isn't
killed, calling project-kill-buffers seems to have no effect.

-- 
	Philip K.

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

[Dmitry Gutov]

On 21.07.2020 12:11, Philip K. wrote:
>> Just
>>
>>     (project--kill-buffer-check buf (cdr c))
>>
>> instead of using apply-partially since we use the 'or' semantics by
>> default already in there.
>>
>> It's not a big thing, and can be changed later anyway.
> Oh yes, I missed that.
> 
> Anyways, the updated patch is attached below.

Is it me, or did you keep that case as it was?

The 'or' case, I mean.

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

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 21.07.2020 12:11, Philip K. wrote:
>>> Just
>>>
>>>     (project--kill-buffer-check buf (cdr c))
>>>
>>> instead of using apply-partially since we use the 'or' semantics by
>>> default already in there.
>>>
>>> It's not a big thing, and can be changed later anyway.
>> Oh yes, I missed that.
>> 
>> Anyways, the updated patch is attached below.
>
> Is it me, or did you keep that case as it was?
>
> The 'or' case, I mean.

You're right, I forgot to stage that. 

-- 
	Philip K.

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

[Eli Zaretskii]

> From: "Philip K." <philip@warpmail.net>
> Cc: dgutov@yandex.ru, 42386@debbugs.gnu.org
> Date: Tue, 21 Jul 2020 20:35:55 +0200
> 
> > 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
> 
> So would you suggest factoring out that programmatic part or checking if
> the command was invoked interactively?

Something like that.  Assuming that Dmitry agrees.

> > (and the "No buffers to kill" message might be spared as well).
> 
> I added "No buffers to kill", because if you're in a buffer that isn't
> killed, calling project-kill-buffers seems to have no effect.

The message is okay for interactive invocation, IMO, but maybe we
shouldn't show it for non-interactive case?

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

[Dmitry Gutov]

On 21.07.2020 21:57, Eli Zaretskii wrote:
>> So would you suggest factoring out that programmatic part or checking if
>> the command was invoked interactively?
> Something like that.  Assuming that Dmitry agrees.

The latter, I think.

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

[Dmitry Gutov]

Hey Philip,

How's it going?

On 21.07.2020 21:51, Philip K. wrote:
> You're right, I forgot to stage that.

I can fix up the minor nits and push the patch now, if you like.

Just wanted to make sure you didn't come upon any more significant 
problems in daily usage.

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

[Philip K.]

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


I actually sent a patch yesterday, but thanks to your message I realised
that my messages weren't being sent (new mail provider, didn't configure
it properly).

I attached the patch below, and hope everything works now.

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hey Philip,
>
> How's it going?

> On 21.07.2020 21:51, Philip K. wrote:
>> You're right, I forgot to stage that.
>
> I can fix up the minor nits and push the patch now, if you like.
>
> Just wanted to make sure you didn't come upon any more significant 
> problems in daily usage.


-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-project-kill-buffer-conditions.patch --]
[-- Type: text/x-diff, Size: 6961 bytes --]

From ae708413a583fa48ed175ac51a465958030d914a Mon Sep 17 00:00:00 2001
From: Philip K <philip@warpmail.net>
Date: Mon, 20 Jul 2020 21:20:34 +0200
Subject: [PATCH] Add project-kill-buffer-conditions

This replaces its negation, project-kill-buffers-ignores, from the
previous version.
---
 lisp/progmodes/project.el | 134 ++++++++++++++++++++++++++++++--------
 1 file changed, 107 insertions(+), 27 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 67ce3dc7d9..237bc1635e 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -843,16 +843,56 @@ project-switch-to-buffer
       nil
       predicate))))
 
-(defcustom project-kill-buffers-ignores
-  '("\\*Help\\*")
-  "Conditions for buffers `project-kill-buffers' should not kill.
-Each condition is either a regular expression matching a buffer
-name, or a predicate function that takes a buffer object as
-argument and returns non-nil if it matches.  Buffers that match
-any of the conditions will not be killed."
-  :type '(repeat (choice regexp function))
+(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))
+  "List of conditions for how to kill buffers related to a project.
+This list is used by `project-kill-buffers'.
+Each condition is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object as argument
+  and returns non-nil if the buffer should be killed,
+- a symbol, denoting a buffer local variable, where the buffer
+  is killed if it's value is non-nil.  If the symbol also has a
+  function slot, it will be interpreted as a function first.
+- a cons-cell, where the car describes how to interpret the cdr.
+  The car can be one of the following:
+  * `major-mode': the buffer is killed if the buffers major
+    mode is eq to the cons-cell's cdr
+  * `defived-mode': the buffer is killed if the buffers major
+    mode is derived from the major mode denoted by the cons-cell's
+    cdr
+  * `not': the cdr is interpreted as a negation of a condition.
+  * `and': the cdr is a list of recursive conditions, that all have
+    to be met.
+  * `or': the cdr is a list of recursive conditions, of which at
+    least one has to be met.
+
+If any of these conditions are satified, a buffer will be
+killed.  By default buffers are left alone, so that
+`project-kill-buffers' doesn't accidentally delete more than it
+should."
+  :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,26 +904,66 @@ project--buffer-list
         (push buf bufs)))
     (nreverse bufs)))
 
-;;;###autoload
-(defun project-kill-buffers ()
-  "Kill all live buffers belonging to the current project.
-Two buffers belong to the same project if their project instances,
-as reported by `project-current' in each buffer, are identical.
-Certain buffers may be \"spared\", see `project-kill-buffers-ignores'."
-  (interactive)
-  (let ((pr (project-current t)) bufs)
+(defun project--kill-buffer-check (buf conditions)
+  "Check if buffer BUF matches any element of the list CONDITIONS.
+See `project-kill-buffer-conditions' for more details on the form
+of CONDITIONS."
+  (catch 'kill
+    (dolist (c conditions)
+      (when (cond
+             ((stringp c)
+              (string-match-p c (buffer-name buf)))
+             ((symbolp c)
+              (funcall c buf))
+             ((eq (car-safe c) 'major-mode)
+              (eq (buffer-local-value 'major-mode buf)
+                  (cdr c)))
+             ((eq (car-safe c) 'derived-mode)
+              (provided-mode-derived-p
+               (buffer-local-value 'major-mode buf)
+               (cdr c)))
+             ((eq (car-safe c) 'not)
+              (not (project--kill-buffer-check buf (cdr c))))
+             ((eq (car-safe c) 'or)
+              (project--kill-buffer-check buf (cdr c)))
+             ((eq (car-safe c) 'and)
+              (seq-every-p
+               (apply-partially #'project--kill-buffer-check
+                                buf)
+               (mapcar #'list (cdr c)))))
+        (throw 'kill t)))))
+
+(defun project-list-buffers-to-kill (pr)
+  "Return list of buffers in project PR to kill.
+What buffers should or should not be killed is described
+in `project-kill-buffer-conditions'."
+  (let (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))))
+    bufs))
+
+;;;###autoload
+(defun project-kill-buffers (&optional no-confirm)
+  "Kill all live buffers belonging to the current project.
+Two buffers belong to the same project if their project
+instances, as reported by `project-current' in each buffer, are
+identical.  Only the buffers that match a condition in
+`project-kill-buffer-conditions' will be killed.  If NO-CONFIRM
+is non-nil, the command will not ask the user for confirmation.
+NO-CONFIRM is always nil when the command is invoked
+interactivly."
+  (interactive)
+  (let* ((pr (project-current t))
+         (bufs (project-list-buffers-to-kill pr)))
+    (cond (no-confirm
+           (mapc #'kill-buffer bufs))
+          ((null bufs)
+           (message "No buffers to kill"))
+          ((yes-or-no-p (format "Kill %d buffers in %s? "
+                                (length bufs)
+                                (project-root pr)))
+           (mapc #'kill-buffer bufs)))))
 
 \f
 ;;; Project list
-- 
2.20.1

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

[Dmitry Gutov]

On 27.07.2020 21:33, Philip K. wrote:
> I actually sent a patch yesterday, but thanks to your message I realised
> that my messages weren't being sent (new mail provider, didn't configure
> it properly).
> 
> I attached the patch below, and hope everything works now.

Thanks!

Applied a few minor changes and pushed.