bug#54296: Add buffer-matching functionality

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

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

Tags: patch


Project.el currently has a small language for matching buffers, used by
project-kill-buffers and project--read-project-buffer.  As mentioned in
[0], this could be generalised, as done in the patch below.  As to what
file this should be added to, should be discussed.

Either way I would consider these functions useful and would have wanted
to use them in my own code many times before.  While difficult, it might
also be useful for things like display-buffer-alist (the issue is that
a function as a condition in display-buffer-alist has to accept two
arguments, while the proposed patch only takes one).

To match functions such as string-match, the argument of buffer-match
could be reversed so that the function can be used as a testfn to
assoc/alist-get.

<tangent>
The reason this was not immediately done when project-kill-buffers was
implemented, was that this would raise the "emacs" dependency of the
ELPA package "project" to the latest release or even the current
development version.

To solve issues like these, I have been working on "compat", a
yet-unreleased library added to GNU ELPA a while back that could be
added as a dependency to project.  That way newer functions, such as the
ones propose below could be used, without breaking ELPA compatibility.

To make this work properly in the near future, compat would have to
follow the upstream development, before a release is made.  If there is
any interest in this kind of an arrangement, I could start a thread on
emacs-devel to discuss the details.
</tangent>

[0] https://mail.gnu.org/archive/html/emacs-devel/2020-09/msg00082.html
[1] https://elpa.gnu.org/devel/compat.html


In GNU Emacs 29.0.50 (build 13, x86_64-pc-linux-gnu, GTK+ Version 3.24.30, cairo version 1.16.0)
 of 2022-02-24 built on viero
Repository revision: bd17fa2c7565f180cedbfa396c0b159e144178cb
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12011000

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalise-buffer-matching-from-project.el.patch --]
[-- Type: text/patch, Size: 3359 bytes --]

From f268509ba1b617b65851231c5ae262f3b169cd6c Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Mon, 7 Mar 2022 20:49:42 +0100
Subject: [PATCH] Generalise buffer matching from project.el

* subr.el (buffer-match): Add function to check if a buffer satisfies
a condition.
(match-buffers): Returns all buffers that satisfy a condition.
---
 lisp/subr.el | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index eb9af0b36d..1d0c9cf967 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -6613,4 +6613,63 @@ delete-line
                    (forward-line 1)
                    (point))))
 
+(defun buffer-match (buffer condition)
+  "Return non-nil if BUFFER matches CONDITION.
+CONDITION is 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 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 buffer's major
+    mode is eq to the cons-cell's cdr
+  * `derived-mode': the buffer is killed if the buffer's 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 condition, that all have
+    to be met.
+  * `or': the cdr is a list of recursive condition, of which at
+    least one has to be met."
+  (letrec
+      ((match
+        (lambda (conditions)
+          (catch 'match
+            (dolist (condition conditions)
+              (when (cond
+                     ((stringp condition)
+                      (string-match-p condition (buffer-name buffer)))
+                     ((symbolp condition)
+                      (funcall condition buffer))
+                     ((eq (car-safe condition) 'major-mode)
+                      (eq (buffer-local-value 'major-mode buffer)
+                          (cdr condition)))
+                     ((eq (car-safe condition) 'derived-mode)
+                      (provided-mode-derived-p
+                       (buffer-local-value 'major-mode buffer)
+                       (cdr condition)))
+                     ((eq (car-safe condition) 'not)
+                      (not (funcall match (cdr condition))))
+                     ((eq (car-safe condition) 'or)
+                      (funcall match (cdr condition)))
+                     ((eq (car-safe condition) 'and)
+                      (catch 'fail
+                        (dolist (c conditions)
+                          (unless (funcall match c)
+                            (throw 'fail nil)))
+                        t)))
+                (throw 'match t)))))))
+    (funcall match (list condition))))
+
+(defun match-buffers (condition &optional buffers)
+  "Return a list of buffers that match CONDITION.
+See `buffer-match' for details on CONDITION.  By default all
+buffers are checked, this can be restricted by passing an
+optional argument BUFFERS, set to a list of buffers to check."
+  (let (bufs)
+    (dolist (buf (or buffers (buffer-list)))
+      (when (buffer-match buf condition)
+        (push buf bufs)))
+    bufs))
+
 ;;; subr.el ends here
-- 
2.34.0


[-- Attachment #3: Type: text/plain, Size: 24 bytes --]


-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

> Either way I would consider these functions useful and would have wanted
> to use them in my own code many times before.  While difficult, it might
> also be useful for things like display-buffer-alist (the issue is that
> a function as a condition in display-buffer-alist has to accept two
> arguments, while the proposed patch only takes one).

Hm...  how would this be used with display-buffer-alist, then?  (And
perhaps Martin has some comments; added to the CCs.)

> To match functions such as string-match, the argument of buffer-match
> could be reversed so that the function can be used as a testfn to
> assoc/alist-get.

I think I'd prefer to have the parameters reversed -- the condition is a
kind of predicate, and these days we seem to prefer to have the
predicate first.

> +(defun buffer-match (buffer condition)
> +  "Return non-nil if BUFFER matches CONDITION.
> +CONDITION is 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,

Killed?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#54296: Add buffer-matching functionality

[martin rudalics]

 >> Either way I would consider these functions useful and would have wanted
 >> to use them in my own code many times before.  While difficult, it might
 >> also be useful for things like display-buffer-alist (the issue is that
 >> a function as a condition in display-buffer-alist has to accept two
 >> arguments, while the proposed patch only takes one).
 >
 > Hm...  how would this be used with display-buffer-alist, then?  (And
 > perhaps Martin has some comments; added to the CCs.)

Either add an optional second argument to 'buffer-match' or add to
'display-buffer-alist' an entry that uses a function as condition that
calls 'buffer-match' with the first argument and returns non-nil if
'buffer-match' reports a match.  I see no issue here.

martin

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

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

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Either way I would consider these functions useful and would have wanted
>> to use them in my own code many times before.  While difficult, it might
>> also be useful for things like display-buffer-alist (the issue is that
>> a function as a condition in display-buffer-alist has to accept two
>> arguments, while the proposed patch only takes one).
>
> Hm...  how would this be used with display-buffer-alist, then?  (And
> perhaps Martin has some comments; added to the CCs.)

See below.

>> To match functions such as string-match, the argument of buffer-match
>> could be reversed so that the function can be used as a testfn to
>> assoc/alist-get.
>
> I think I'd prefer to have the parameters reversed -- the condition is a
> kind of predicate, and these days we seem to prefer to have the
> predicate first.

I agree, it makes more sense.

>> +(defun buffer-match (buffer condition)
>> +  "Return non-nil if BUFFER matches CONDITION.
>> +CONDITION is 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,
>
> Killed?

That was a typo from copying the docstring.  Here is the updated patch:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalise-buffer-matching-from-project.el.patch --]
[-- Type: text/x-patch, Size: 3655 bytes --]

From 6714a4dc7168e6806dba0707c9c0b80d3365b2c5 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Mon, 7 Mar 2022 20:49:42 +0100
Subject: [PATCH 1/2] Generalise buffer matching from project.el

* subr.el (buffer-match): Add function to check if a buffer satisfies
a condition.
(match-buffers): Returns all buffers that satisfy a condition.
---
 lisp/subr.el | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index 2321765f95..adcd25ec14 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -6613,4 +6613,68 @@ delete-line
                    (forward-line 1)
                    (point))))
 
+(defun buffer-match (condition buffer-or-name &optional arg)
+  "Return non-nil if BUFFER-OR-NAME matches CONDITION.
+CONDITION is is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object and ARG as
+  arguments and returns non-nil if the buffer matches,
+- a cons-cell, where the car describes how to interpret the cdr.
+  The car can be one of the following:
+  * `major-mode': the buffer matches if the buffer's major
+    mode is eq to the cons-cell's cdr
+  * `derived-mode': the buffer matches if the buffer's 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 condition, that all have
+    to be met.
+  * `or': the cdr is a list of recursive condition, of which at
+    least one has to be met."
+  (letrec
+      ((buffer (get-buffer buffer-or-name))
+       (match
+        (lambda (conditions)
+          (catch 'match
+            (dolist (condition conditions)
+              (when (cond
+                     ((stringp condition)
+                      (string-match-p condition (buffer-name buffer)))
+                     ((functionp condition)
+                      (if (eq 1 (cdr (func-arity condition)))
+                          (funcall condition buffer)
+                        (funcall condition buffer arg)))
+                     ((eq (car-safe condition) 'major-mode)
+                      (eq (buffer-local-value 'major-mode buffer)
+                          (cdr condition)))
+                     ((eq (car-safe condition) 'derived-mode)
+                      (provided-mode-derived-p
+                       (buffer-local-value 'major-mode buffer)
+                       (cdr condition)))
+                     ((eq (car-safe condition) 'not)
+                      (not (funcall match (cdr condition))))
+                     ((eq (car-safe condition) 'or)
+                      (funcall match (cdr condition)))
+                     ((eq (car-safe condition) 'and)
+                      (catch 'fail
+                        (dolist (c conditions)
+                          (unless (funcall match c)
+                            (throw 'fail nil)))
+                        t)))
+                (throw 'match t)))))))
+    (funcall match (list condition))))
+
+(defun match-buffers (condition &optional buffers arg)
+  "Return a list of buffers that match CONDITION.
+See `buffer-match' for details on CONDITION.  By default all
+buffers are checked, this can be restricted by passing an
+optional argument BUFFERS, set to a list of buffers to check.
+ARG is passed to `buffer-match', for predicate conditions in
+CONDITION."
+  (let (bufs)
+    (dolist (buf (or buffers (buffer-list)))
+      (when (buffer-match condition (get-buffer buf) arg)
+        (push buf bufs)))
+    bufs))
+
 ;;; subr.el ends here
-- 
2.34.0


[-- Attachment #3: Type: text/plain, Size: 914 bytes --]


martin rudalics <rudalics@gmx.at> writes:

>>> Either way I would consider these functions useful and would have
>>> wanted
>>> to use them in my own code many times before.  While difficult, it
>>> might
>>> also be useful for things like display-buffer-alist (the issue is that
>>> a function as a condition in display-buffer-alist has to accept two
>>> arguments, while the proposed patch only takes one).
>>
>> Hm...  how would this be used with display-buffer-alist, then?  (And
>> perhaps Martin has some comments; added to the CCs.)
>
> Either add an optional second argument to 'buffer-match' or add to
> 'display-buffer-alist' an entry that uses a function as condition that
> calls 'buffer-match' with the first argument and returns non-nil if
> 'buffer-match' reports a match.  I see no issue here.

This seems to work, and updating window.el was also pretty trivial
(unless I have missed something):


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0002-window.el-display-buffer-assq-regexp-Use-buffer-matc.patch --]
[-- Type: text/x-patch, Size: 1541 bytes --]

From fadea32d0dc74952ca70f8d98ced59616c0e3e1a Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 10 Mar 2022 10:59:52 +0100
Subject: [PATCH 2/2] * window.el (display-buffer-assq-regexp): Use
 buffer-match

---
 lisp/window.el | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index 54c9eee5f3..4225019920 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7497,19 +7497,12 @@ display-buffer-fallback-action
 (defun display-buffer-assq-regexp (buffer-name alist action)
   "Retrieve ALIST entry corresponding to BUFFER-NAME.
 This returns the cdr of the alist entry ALIST if either its key
-is a string that matches BUFFER-NAME, as reported by
-`string-match-p'; or if the key is a function that returns
-non-nil when called with three arguments: the ALIST key,
-BUFFER-NAME and ACTION.  ACTION should have the form of the
-action argument passed to `display-buffer'."
+satisfied a BUFFER-NAME per `buffer-match'.  ACTION should have
+the form of the action argument passed to `display-buffer'."
   (catch 'match
     (dolist (entry alist)
-      (let ((key (car entry)))
-	(when (or (and (stringp key)
-		       (string-match-p key buffer-name))
-		  (and (functionp key)
-		       (funcall key buffer-name action)))
-	  (throw 'match (cdr entry)))))))
+      (when (buffer-match (car entry) buffer-name action)
+        (throw 'match (cdr entry))))))
 
 (defvar display-buffer--same-window-action
   '(display-buffer-same-window
-- 
2.34.0


[-- Attachment #5: Type: text/plain, Size: 37 bytes --]



> martin
>

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Date: Thu, 10 Mar 2022 10:05:04 +0000
> Cc: 54296@debbugs.gnu.org
> 
> +(defun buffer-match (condition buffer-or-name &optional arg)
> +  "Return non-nil if BUFFER-OR-NAME matches CONDITION.

This is a predicate function, so its name should be buffer-matches-p
or maybe buffer-matches-condition-p.

> +CONDITION is is either:
             ^^^^^
Typo.

> +- a regular expression, to match a buffer name,
> +- a predicate function that takes a buffer object and ARG as
> +  arguments and returns non-nil if the buffer matches,
              ^
Comma missing there.

> +- a cons-cell, where the car describes how to interpret the cdr.
> +  The car can be one of the following:
> +  * `major-mode': the buffer matches if the buffer's major
> +    mode is eq to the cons-cell's cdr
> +  * `derived-mode': the buffer matches if the buffer's major
> +    mode is derived from the major mode denoted by the cons-cell's
> +    cdr

Do we really need both major-mode and derived-mode?

> +  * `not': the cdr is interpreted as a negation of a condition.
> +  * `and': the cdr is a list of recursive condition, that all have
> +    to be met.                            ^^^^^^^^^

Typo: should be "conditions".

> +  * `or': the cdr is a list of recursive condition, of which at
> +    least one has to be met."            ^^^^^^^^^

Likewise.

Thanks.

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Date: Thu, 10 Mar 2022 10:05:04 +0000
>> Cc: 54296@debbugs.gnu.org
>> 
>> +(defun buffer-match (condition buffer-or-name &optional arg)
>> +  "Return non-nil if BUFFER-OR-NAME matches CONDITION.
>
> This is a predicate function, so its name should be buffer-matches-p
> or maybe buffer-matches-condition-p.

I like buffer-matches-p.

>> +CONDITION is is either:
>              ^^^^^
> Typo.

Sorry about those, it and the other ones here:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalise-buffer-matching-from-project.el.patch --]
[-- Type: text/x-patch, Size: 3658 bytes --]

From 8dfcabb8c655f06c8c7285c6da7efad7057b636e Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Mon, 7 Mar 2022 20:49:42 +0100
Subject: [PATCH 1/2] Generalise buffer matching from project.el

* subr.el (buffer-match): Add function to check if a buffer satisfies
a condition.
(match-buffers): Returns all buffers that satisfy a condition.
---
 lisp/subr.el | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index 2321765f95..e55a4c5802 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -6613,4 +6613,68 @@ delete-line
                    (forward-line 1)
                    (point))))
 
+(defun buffer-match-p (condition buffer-or-name &optional arg)
+  "Return non-nil if BUFFER-OR-NAME matches CONDITION.
+CONDITION is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object and ARG as
+  arguments, and returns non-nil if the buffer matches,
+- a cons-cell, where the car describes how to interpret the cdr.
+  The car can be one of the following:
+  * `major-mode': the buffer matches if the buffer's major
+    mode is eq to the cons-cell's cdr
+  * `derived-mode': the buffer matches if the buffer's 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 condition, of which at
+    least one has to be met."
+  (letrec
+      ((buffer (get-buffer buffer-or-name))
+       (match
+        (lambda (conditions)
+          (catch 'match
+            (dolist (condition conditions)
+              (when (cond
+                     ((stringp condition)
+                      (string-match-p condition (buffer-name buffer)))
+                     ((functionp condition)
+                      (if (eq 1 (cdr (func-arity condition)))
+                          (funcall condition buffer)
+                        (funcall condition buffer arg)))
+                     ((eq (car-safe condition) 'major-mode)
+                      (eq (buffer-local-value 'major-mode buffer)
+                          (cdr condition)))
+                     ((eq (car-safe condition) 'derived-mode)
+                      (provided-mode-derived-p
+                       (buffer-local-value 'major-mode buffer)
+                       (cdr condition)))
+                     ((eq (car-safe condition) 'not)
+                      (not (funcall match (cdr condition))))
+                     ((eq (car-safe condition) 'or)
+                      (funcall match (cdr condition)))
+                     ((eq (car-safe condition) 'and)
+                      (catch 'fail
+                        (dolist (c conditions)
+                          (unless (funcall match c)
+                            (throw 'fail nil)))
+                        t)))
+                (throw 'match t)))))))
+    (funcall match (list condition))))
+
+(defun match-buffers (condition &optional buffers arg)
+  "Return a list of buffers that match CONDITION.
+See `buffer-match' for details on CONDITION.  By default all
+buffers are checked, this can be restricted by passing an
+optional argument BUFFERS, set to a list of buffers to check.
+ARG is passed to `buffer-match', for predicate conditions in
+CONDITION."
+  (let (bufs)
+    (dolist (buf (or buffers (buffer-list)))
+      (when (buffer-match-p condition (get-buffer buf) arg)
+        (push buf bufs)))
+    bufs))
+
 ;;; subr.el ends here
-- 
2.34.0


[-- Attachment #3: Type: text/plain, Size: 717 bytes --]


>> +- a cons-cell, where the car describes how to interpret the cdr.
>> +  The car can be one of the following:
>> +  * `major-mode': the buffer matches if the buffer's major
>> +    mode is eq to the cons-cell's cdr
>> +  * `derived-mode': the buffer matches if the buffer's major
>> +    mode is derived from the major mode denoted by the cons-cell's
>> +    cdr
>
> Do we really need both major-mode and derived-mode?

It seems to have been useful in project.el, see
`project-kill-buffer-conditions'.  In that case you want to both be able
to say something like "kill buffers only if they are in
fundamental-mode", but also something like "kill all buffers that are
based on comint-mode".

-- 
	Philip Kaludercic

bug#54296: [External] : bug#54296: Add buffer-matching functionality

[Drew Adams]

> > This is a predicate function, so its name should be buffer-matches-p
> > or maybe buffer-matches-condition-p.
> 
> I like buffer-matches-p.

(I'm not following this thread, BTW.)

"Matching" typically has a connotation of pattern
matching.  In the case of Emacs it is especially
about textual patterns matching text.

If you're testing a buffer object, and not just
its text or its name, then it's probably better
to speak of the buffer "satisfying" a condition
or a predicate than it is to speak of it
"matching" a condition or a predicate.

In general, patterns are matched; conditions and
predicates are satisfied.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> Date: Thu, 10 Mar 2022 12:13:59 +0000
> 
> > Do we really need both major-mode and derived-mode?
> 
> It seems to have been useful in project.el, see
> `project-kill-buffer-conditions'.  In that case you want to both be able
> to say something like "kill buffers only if they are in
> fundamental-mode", but also something like "kill all buffers that are
> based on comint-mode".

So this is only because of fundamental-mode?  If so, shouldn't it be
enough to have a possibility to have a predicate function, which can
do anything one likes?

I think we want in general avoid comparison with major-mode, and
prefer derived-mode instead, and if so, IMO we had better did as we
say and not exposed comparison to major mode unless we absolutely
must.

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> Date: Thu, 10 Mar 2022 12:13:59 +0000
>> 
>> > Do we really need both major-mode and derived-mode?
>> 
>> It seems to have been useful in project.el, see
>> `project-kill-buffer-conditions'.  In that case you want to both be able
>> to say something like "kill buffers only if they are in
>> fundamental-mode", but also something like "kill all buffers that are
>> based on comint-mode".
>
> So this is only because of fundamental-mode?

I wouldn't say so, that is a known use-case, that is one I know of right
now, but there is no reason something like that couldn't come up again?

> If so, shouldn't it be enough to have a possibility to have a
> predicate function, which can do anything one likes?

Everything this patch introduces could be replace by a predicate, but
the other issue is that if the major-mode/derived-mode distinction is
removed the function couldn't be used in project.el anymore, or would
have to break compatibility.

> I think we want in general avoid comparison with major-mode, and
> prefer derived-mode instead, and if so, IMO we had better did as we
> say and not exposed comparison to major mode unless we absolutely
> must.

Would it be enough to clarify this point in the documentation string?

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> Date: Fri, 11 Mar 2022 16:21:06 +0000
> 
> > I think we want in general avoid comparison with major-mode, and
> > prefer derived-mode instead, and if so, IMO we had better did as we
> > say and not exposed comparison to major mode unless we absolutely
> > must.
> 
> Would it be enough to clarify this point in the documentation string?

What would you like to clarify?

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> Date: Fri, 11 Mar 2022 16:21:06 +0000
>> 
>> > I think we want in general avoid comparison with major-mode, and
>> > prefer derived-mode instead, and if so, IMO we had better did as we
>> > say and not exposed comparison to major mode unless we absolutely
>> > must.
>> 
>> Would it be enough to clarify this point in the documentation string?
>
> What would you like to clarify?

To clarify that the usage of (major-mode . foo-mode) might not be what
the user intends, and that in most cases derived-mode is preferable.

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> Date: Sun, 13 Mar 2022 20:40:49 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk@posteo.net>
> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> >> Date: Fri, 11 Mar 2022 16:21:06 +0000
> >> 
> >> > I think we want in general avoid comparison with major-mode, and
> >> > prefer derived-mode instead, and if so, IMO we had better did as we
> >> > say and not exposed comparison to major mode unless we absolutely
> >> > must.
> >> 
> >> Would it be enough to clarify this point in the documentation string?
> >
> > What would you like to clarify?
> 
> To clarify that the usage of (major-mode . foo-mode) might not be what
> the user intends, and that in most cases derived-mode is preferable.

I suggested to "clarify" that by not providing the 'major-mode'
predicate at all.  I still don't think I understand why it is so
important that we should provide a special case for it.

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> Date: Sun, 13 Mar 2022 20:40:49 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Philip Kaludercic <philipk@posteo.net>
>> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> >> Date: Fri, 11 Mar 2022 16:21:06 +0000
>> >> 
>> >> > I think we want in general avoid comparison with major-mode, and
>> >> > prefer derived-mode instead, and if so, IMO we had better did as we
>> >> > say and not exposed comparison to major mode unless we absolutely
>> >> > must.
>> >> 
>> >> Would it be enough to clarify this point in the documentation string?
>> >
>> > What would you like to clarify?
>> 
>> To clarify that the usage of (major-mode . foo-mode) might not be what
>> the user intends, and that in most cases derived-mode is preferable.
>
> I suggested to "clarify" that by not providing the 'major-mode'
> predicate at all.  I still don't think I understand why it is so
> important that we should provide a special case for it.

It is not inherently important, it is just that if the predicate would
also be used in project.el, then compatibility would have to be broken,
as the distinction between `major-mode' and `derived-mode' exists there.

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> Date: Mon, 14 Mar 2022 08:21:56 +0000
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> From: Philip Kaludercic <philipk@posteo.net>
> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> >> Date: Sun, 13 Mar 2022 20:40:49 +0000
> >> 
> >> Eli Zaretskii <eliz@gnu.org> writes:
> >> 
> >> >> From: Philip Kaludercic <philipk@posteo.net>
> >> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> >> >> Date: Fri, 11 Mar 2022 16:21:06 +0000
> >> >> 
> >> >> > I think we want in general avoid comparison with major-mode, and
> >> >> > prefer derived-mode instead, and if so, IMO we had better did as we
> >> >> > say and not exposed comparison to major mode unless we absolutely
> >> >> > must.
> >> >> 
> >> >> Would it be enough to clarify this point in the documentation string?
> >> >
> >> > What would you like to clarify?
> >> 
> >> To clarify that the usage of (major-mode . foo-mode) might not be what
> >> the user intends, and that in most cases derived-mode is preferable.
> >
> > I suggested to "clarify" that by not providing the 'major-mode'
> > predicate at all.  I still don't think I understand why it is so
> > important that we should provide a special case for it.
> 
> It is not inherently important, it is just that if the predicate would
> also be used in project.el, then compatibility would have to be broken,
> as the distinction between `major-mode' and `derived-mode' exists there.

Then project.el could use the predicate route, right?  It's quite a
special case, AFAIU, so having a special solution is OK.

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> Date: Mon, 14 Mar 2022 08:21:56 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Philip Kaludercic <philipk@posteo.net>
>> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> >> Date: Sun, 13 Mar 2022 20:40:49 +0000
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> >> From: Philip Kaludercic <philipk@posteo.net>
>> >> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> >> >> Date: Fri, 11 Mar 2022 16:21:06 +0000
>> >> >> 
>> >> >> > I think we want in general avoid comparison with major-mode, and
>> >> >> > prefer derived-mode instead, and if so, IMO we had better did as we
>> >> >> > say and not exposed comparison to major mode unless we absolutely
>> >> >> > must.
>> >> >> 
>> >> >> Would it be enough to clarify this point in the documentation string?
>> >> >
>> >> > What would you like to clarify?
>> >> 
>> >> To clarify that the usage of (major-mode . foo-mode) might not be what
>> >> the user intends, and that in most cases derived-mode is preferable.
>> >
>> > I suggested to "clarify" that by not providing the 'major-mode'
>> > predicate at all.  I still don't think I understand why it is so
>> > important that we should provide a special case for it.
>> 
>> It is not inherently important, it is just that if the predicate would
>> also be used in project.el, then compatibility would have to be broken,
>> as the distinction between `major-mode' and `derived-mode' exists there.
>
> Then project.el could use the predicate route, right?  It's quite a
> special case, AFAIU, so having a special solution is OK.

The issue isn't that the default value couldn't be updated, but that if
anyone has e.g. been using `project-kill-buffer-conditions' over the
last 1½ years and has relied on the specific distinction between
`major-mode' and `derived-mode', they would run in to unexpected
results, that might result in more buffers being killed than intended,
potentially data being lost that the user might not expect because of an
update.

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 14.03.2022 15:38, Philip Kaludercic wrote:
> The issue isn't that the default value couldn't be updated, but that if
> anyone has e.g. been using `project-kill-buffer-conditions' over the
> last 1½ years and has relied on the specific distinction between
> `major-mode' and `derived-mode', they would run in to unexpected
> results, that might result in more buffers being killed than intended,
> potentially data being lost that the user might not expect because of an
> update.

I see that this point hasn't been addressed at all.

Eli?

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Mon, 13 Jun 2022 03:21:13 +0300
> Cc: 54296@debbugs.gnu.org, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 14.03.2022 15:38, Philip Kaludercic wrote:
> > The issue isn't that the default value couldn't be updated, but that if
> > anyone has e.g. been using `project-kill-buffer-conditions' over the
> > last 1½ years and has relied on the specific distinction between
> > `major-mode' and `derived-mode', they would run in to unexpected
> > results, that might result in more buffers being killed than intended,
> > potentially data being lost that the user might not expect because of an
> > update.
> 
> I see that this point hasn't been addressed at all.
> 
> Eli?

Do you agree with Philip that deprecating or removing major-mode would
be a problem for users of project-kill-buffer-conditions etc.?

I kinda gave up because Philip felt extremely uneasy with dropping
major-mode, leaving us with just derived-mode, but I still think we
shouldn't encourage Lisp programs to use major-mode.  So if you don't
think it would be a problem, I'd be happier if we removed major-mode
and used derived-mode instead.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 13.06.2022 15:04, Eli Zaretskii wrote:
>> Date: Mon, 13 Jun 2022 03:21:13 +0300
>> Cc: 54296@debbugs.gnu.org, larsi@gnus.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 14.03.2022 15:38, Philip Kaludercic wrote:
>>> The issue isn't that the default value couldn't be updated, but that if
>>> anyone has e.g. been using `project-kill-buffer-conditions' over the
>>> last 1½ years and has relied on the specific distinction between
>>> `major-mode' and `derived-mode', they would run in to unexpected
>>> results, that might result in more buffers being killed than intended,
>>> potentially data being lost that the user might not expect because of an
>>> update.
>>
>> I see that this point hasn't been addressed at all.
>>
>> Eli?
> 
> Do you agree with Philip that deprecating or removing major-mode would
> be a problem for users of project-kill-buffer-conditions etc.?

Yes. I also mentioned that here: 
https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00650.html

Not for all users, only for some of those who customized the value, 
etc... but we're usually more concerned about backward compatibility 
than the present development seems to have indicated.

If we can argue over several days (or weeks?) about removing a key 
binding that nobody uses, changing DSL in an incompatible way has to be 
considered a problem.

> I kinda gave up because Philip felt extremely uneasy with dropping
> major-mode, leaving us with just derived-mode, but I still think we
> shouldn't encourage Lisp programs to use major-mode.  So if you don't
> think it would be a problem, I'd be happier if we removed major-mode
> and used derived-mode instead.

If I didn't think Philip's point had merit, I wouldn't have brought it 
up now.

The idea of discouraging the (eq major-mode ...) semantics (instead of 
derived-mode-p) has merit, but so far we have one complex prior art for 
this functionality, and it did find a use for this distinction. Two 
uses, to be exact.

Another example where we might migrate to buffer-matches-p is 
'turn-on-font-lock-if-desired'. And that one also currently uses

   (memq major-mode font-lock-global-modes)

rather than derived-mode-p. There probably are reasons for this as well.

We could try some methods that discourage the use of 'major-mode' as 
opposed to 'derived-mode'. Maybe by calling them 'major-mode-eq' and 
'major-mode' (where the longer version feels more awkward and thus less 
desirable). But FWIW I like the current couple of 'derived-mode' and 
'major-mode' because it feels "transparent" for an experienced Elisp 
programmer. Rather than having one guess that 'major-mode' uses the 
derived-mode-p predicate or search for that info in the docs.

And if we change the keywords in project-kill-buffer-conditions, I 
suppose it will require some migration strategy. Which is going to be a 
pain.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Tue, 14 Jun 2022 21:43:41 +0300
> Cc: philipk@posteo.net, 54296@debbugs.gnu.org, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 13.06.2022 15:04, Eli Zaretskii wrote:
> >> Date: Mon, 13 Jun 2022 03:21:13 +0300
> >> Cc: 54296@debbugs.gnu.org, larsi@gnus.org
> >> From: Dmitry Gutov <dgutov@yandex.ru>
> >>
> >> On 14.03.2022 15:38, Philip Kaludercic wrote:
> >>> The issue isn't that the default value couldn't be updated, but that if
> >>> anyone has e.g. been using `project-kill-buffer-conditions' over the
> >>> last 1½ years and has relied on the specific distinction between
> >>> `major-mode' and `derived-mode', they would run in to unexpected
> >>> results, that might result in more buffers being killed than intended,
> >>> potentially data being lost that the user might not expect because of an
> >>> update.
> >>
> >> I see that this point hasn't been addressed at all.
> >>
> >> Eli?
> > 
> > Do you agree with Philip that deprecating or removing major-mode would
> > be a problem for users of project-kill-buffer-conditions etc.?
> 
> Yes. I also mentioned that here: 
> https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00650.html

So then everything is okay, AFAIU, because eventually Philip left that
in the code.  Right?

> > I kinda gave up because Philip felt extremely uneasy with dropping
> > major-mode, leaving us with just derived-mode, but I still think we
> > shouldn't encourage Lisp programs to use major-mode.  So if you don't
> > think it would be a problem, I'd be happier if we removed major-mode
> > and used derived-mode instead.
> 
> If I didn't think Philip's point had merit, I wouldn't have brought it 
> up now.

OK, but I didn't press my POV, so where's the problem, and why the
emotions?

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 14.06.2022 21:47, Eli Zaretskii wrote:

>>> Do you agree with Philip that deprecating or removing major-mode would
>>> be a problem for users of project-kill-buffer-conditions etc.?
>>
>> Yes. I also mentioned that here:
>> https://lists.gnu.org/archive/html/emacs-devel/2022-06/msg00650.html
> 
> So then everything is okay, AFAIU, because eventually Philip left that
> in the code.  Right?

No, he didn't.

He tried to migrate project-kill-buffer-conditions to the new format, 
with predicted unfortunate side-effects. And an unpredicted one (which 
is easier to fix, though).

See commit 1a3bad431d.

>>> I kinda gave up because Philip felt extremely uneasy with dropping
>>> major-mode, leaving us with just derived-mode, but I still think we
>>> shouldn't encourage Lisp programs to use major-mode.  So if you don't
>>> think it would be a problem, I'd be happier if we removed major-mode
>>> and used derived-mode instead.
>>
>> If I didn't think Philip's point had merit, I wouldn't have brought it
>> up now.
> 
> OK, but I didn't press my POV, so where's the problem, and why the
> emotions?

Your POV was the one that got implemented. buffer-match-p has 
'major-mode' but not 'derived-mode'. And 'major-mode' behaves like 
'derived-mode' behaved in project-kill-buffer-conditions.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Tue, 14 Jun 2022 22:36:55 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 14.06.2022 21:47, Eli Zaretskii wrote:
> 
> > So then everything is okay, AFAIU, because eventually Philip left that
> > in the code.  Right?
> 
> No, he didn't.
> 
> He tried to migrate project-kill-buffer-conditions to the new format, 
> with predicted unfortunate side-effects. And an unpredicted one (which 
> is easier to fix, though).
> 
> See commit 1a3bad431d.

Which part?

AFAICT, major-mode is still in the code:

  (defun buffer-match-p (condition buffer-or-name &optional arg)
    "Return non-nil if BUFFER-OR-NAME matches CONDITION.
  CONDITION is either:
  - a regular expression, to match a buffer name,
  - a predicate function that takes a buffer object and ARG as
    arguments, and returns non-nil if the buffer matches,
  - a cons-cell, where the car describes how to interpret the cdr.
    The car can be one of the following:
    * `major-mode': the buffer matches if the buffer's major
      mode is derived from the major mode denoted by the cons-cell's
      cdr
  [...]
		       ((eq (car-safe condition) 'major-mode)
			(provided-mode-derived-p
			 (buffer-local-value 'major-mode buffer)
			 (cdr condition)))


> > OK, but I didn't press my POV, so where's the problem, and why the
> > emotions?
> 
> Your POV was the one that got implemented. buffer-match-p has 
> 'major-mode' but not 'derived-mode'. And 'major-mode' behaves like 
> 'derived-mode' behaved in project-kill-buffer-conditions.

This seems to be some misunderstanding.  I wanted us to have only
'derived-mode', not 'major-mode'.  But Philip was unhappy about that,
due to existing usage of 'major-mode', and eventually 'major-mode' was
left in the code.  Or at least this is my recollection of the
discussion and its resolution.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 15.06.2022 05:33, Eli Zaretskii wrote:
> This seems to be some misunderstanding.  I wanted us to have only
> 'derived-mode', not 'major-mode'.

Okay?...

> But Philip was unhappy about that,
> due to existing usage of 'major-mode', and eventually 'major-mode' was
> left in the code.  Or at least this is my recollection of the
> discussion and its resolution.

'major-mode' was left in the code and in the docs, but its meaning was 
changed. Hence the backward compatibility concern.

And 'derived-mode' was dropped.

Or you could say that 'major-mode' was dropped, and 'derived-mode' was 
left it, but renamed.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Wed, 15 Jun 2022 14:48:52 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 15.06.2022 05:33, Eli Zaretskii wrote:
> > This seems to be some misunderstanding.  I wanted us to have only
> > 'derived-mode', not 'major-mode'.
> 
> Okay?...
> 
> > But Philip was unhappy about that,
> > due to existing usage of 'major-mode', and eventually 'major-mode' was
> > left in the code.  Or at least this is my recollection of the
> > discussion and its resolution.
> 
> 'major-mode' was left in the code and in the docs, but its meaning was 
> changed. Hence the backward compatibility concern.
> 
> And 'derived-mode' was dropped.
> 
> Or you could say that 'major-mode' was dropped, and 'derived-mode' was 
> left it, but renamed.

Then I'm sorry, but I don't think I have anything to do with that
change.  If I somehow cause Philip to make such changes, I apologize,
because I never meant anything even close (and cannot understand how
what I wrote could have interpreted to that effect, but that's water
under the bridge).

Once again, my only comment about the code was that I thought (and
still think) we should not use major-mode matching, but derived-mode
matching instead.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 15.06.2022 16:20, Eli Zaretskii wrote:
> Then I'm sorry, but I don't think I have anything to do with that
> change.  If I somehow cause Philip to make such changes, I apologize,
> because I never meant anything even close (and cannot understand how
> what I wrote could have interpreted to that effect, but that's water
> under the bridge).

That's easy to fix, but:

> Once again, my only comment about the code was that I thought (and
> still think) we should not use major-mode matching, but derived-mode
> matching instead.

A lot of my emails in this thread are about "we should not use 
major-mode matching". That we maybe should. Because we do.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Wed, 15 Jun 2022 18:56:03 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > Once again, my only comment about the code was that I thought (and
> > still think) we should not use major-mode matching, but derived-mode
> > matching instead.
> 
> A lot of my emails in this thread are about "we should not use 
> major-mode matching". That we maybe should. Because we do.

Did you see that "was" part in what I wrote above?  I was only
describing what I meant to say back then, that's all.  Why beat that
long-dead horse 3 months after it died?

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 15.06.2022 19:17, Eli Zaretskii wrote:
> Did you see that "was" part in what I wrote above?  I was only
> describing what I meant to say back then, that's all.

Because your opinion made it into reality.

The main problem was caused by the divergence from your suggestion 
(apparently), but I'm also discussing more minor ones.

> Why beat that
> long-dead horse 3 months after it died?

Are you going to be fine with me changing 'buffer-match-p' to match the 
older contract of 'project-kill-buffer-conditions', with both 
'major-mode' and 'derived-mode' matchers included?

If yes, no more questions from me.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Wed, 15 Jun 2022 19:51:42 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 15.06.2022 19:17, Eli Zaretskii wrote:
> > Did you see that "was" part in what I wrote above?  I was only
> > describing what I meant to say back then, that's all.
> 
> Because your opinion made it into reality.

Sorry about that, but it was a misunderstanding of my opinion.

> Are you going to be fine with me changing 'buffer-match-p' to match the 
> older contract of 'project-kill-buffer-conditions', with both 
> 'major-mode' and 'derived-mode' matchers included?

I don't think I understand what you are suggesting.  What is that
"older contract"?  Can you show a patch?

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 15.06.2022 20:29, Eli Zaretskii wrote:
>> Date: Wed, 15 Jun 2022 19:51:42 +0300
>> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 15.06.2022 19:17, Eli Zaretskii wrote:
>>> Did you see that "was" part in what I wrote above?  I was only
>>> describing what I meant to say back then, that's all.
>>
>> Because your opinion made it into reality.
> 
> Sorry about that, but it was a misunderstanding of my opinion.
> 
>> Are you going to be fine with me changing 'buffer-match-p' to match the
>> older contract of 'project-kill-buffer-conditions', with both
>> 'major-mode' and 'derived-mode' matchers included?
> 
> I don't think I understand what you are suggesting.  What is that
> "older contract"? 

The value format that 'project-kill-buffer-conditions' supported before 
the recent change. You can look at its docstring and definition inside 
Emacs 28.1, for instance.

> Can you show a patch?

Please look at the patch attached to the very first message in this bug 
report.

https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;bug=54296;msg=5;filename=0001-Generalise-buffer-matching-from-project.el.patch

The function name has changed after discussion, but the docstring is 
still relevant, and it mirrors 'project-kill-buffer-conditions'.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Thu, 16 Jun 2022 03:47:15 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > Can you show a patch?
> 
> Please look at the patch attached to the very first message in this bug 
> report.
> 
> https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;bug=54296;msg=5;filename=0001-Generalise-buffer-matching-from-project.el.patch
> 
> The function name has changed after discussion, but the docstring is 
> still relevant, and it mirrors 'project-kill-buffer-conditions'.

Other things changed as well, even in the doc string, and not just in
name changes.  Going back to the original patch makes little sense to
me.  So I'd still prefer to see an actual patch, before I tell whether
I'm okay with it.

In any case, if we will keep both major-mode and derived-mode
conditions in subr.el, let's at least document that derived-mode
should be preferred where it can do the job, and major-mode used only
where derived-mode will not DTRT.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

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

On 16.06.2022 08:51, Eli Zaretskii wrote:
>> Date: Thu, 16 Jun 2022 03:47:15 +0300
>> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>>> Can you show a patch?
>>
>> Please look at the patch attached to the very first message in this bug
>> report.
>>
>> https://debbugs.gnu.org/cgi/bugreport.cgi?att=1;bug=54296;msg=5;filename=0001-Generalise-buffer-matching-from-project.el.patch
>>
>> The function name has changed after discussion, but the docstring is
>> still relevant, and it mirrors 'project-kill-buffer-conditions'.
> 
> Other things changed as well, even in the doc string, and not just in
> name changes.  Going back to the original patch makes little sense to
> me.  So I'd still prefer to see an actual patch, before I tell whether
> I'm okay with it.
> 
> In any case, if we will keep both major-mode and derived-mode
> conditions in subr.el, let's at least document that derived-mode
> should be preferred where it can do the job, and major-mode used only
> where derived-mode will not DTRT.

Sure. Here you go.

The meat of the change is intermixed here with documentation fixes (and 
the addition of support for lambdas in project-kill-buffer-conditions), 
but they should be easy enough to tell apart.

[-- Attachment #2: buffer-match-p.diff --]
[-- Type: text/x-patch, Size: 5529 bytes --]

diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi
index 1cbe8bc093..25e8e96d88 100644
--- a/doc/lispref/buffers.texi
+++ b/doc/lispref/buffers.texi
@@ -981,13 +981,18 @@ Buffer List
 Satisfied if @var{expr} doesn't satisfy @code{buffer-match-p} with
 the same buffer and @code{arg}.
 @item or
-Satisfied if @var{oper} is a list and @emph{any} condition if
+Satisfied if @var{expr} is a list and @emph{any} condition in
 @var{expr} satisfies @code{buffer-match-p}, with the same buffer and
 @code{arg}.
 @item and
-Satisfied if @var{oper} is a list and @emph{all} condition if
-@var{expr} satisfies @code{buffer-match-p}, with the same buffer and
+Satisfied if @var{expr} is a list and @emph{all} conditions in
+@var{expr} satisfy @code{buffer-match-p}, with the same buffer and
 @code{arg}.
+@item derived-mode
+Satisfied if the buffer's major mode derives from @var{expr}.
+@item major-mode
+Satisfied if the buffer's major mode is eq to @var{expr}.  Prefer
+using @code{derived-mode} instead when both can work.
 @end table
 @end itemize
 @end defun
diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index f4d6742ed8..30f51704dc 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1221,22 +1221,18 @@ project-display-buffer-other-frame
   (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:
-    ,(lambda (buf)
-       (not (eq (buffer-local-value 'major-mode buf)
-                'fundamental-mode)))
+    (major-mode . fundamental-mode)
     ;; non-text buffer such as xref, occur, vc, log, ...
-    (and (major-mode . special-mode)
-         ,(lambda (buf)
-            (not (eq (buffer-local-value 'major-mode buf)
-                     'help-mode))))
-    (major-mode . compilation-mode)
-    (major-mode . dired-mode)
-    (major-mode . diff-mode)
-    (major-mode . comint-mode)
-    (major-mode . eshell-mode)
-    (major-mode . change-log-mode))
+    (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))
   "List of conditions to kill buffers related to a project.
 This list is used by `project-kill-buffers'.
 Each condition is either:
@@ -1246,11 +1242,9 @@ project-kill-buffer-conditions
 - 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 buffer's major
-    mode is derived from the major mode denoted by the cons-cell's
-    cdr.
+    mode is eq to the cons-cell's cdr.
   * `derived-mode': the buffer is killed if the buffer's major
-    mode is eq to the cons-cell's cdr (this is deprecated and will
-    result in a warning if used).
+    mode is derived from the major mode in 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.
@@ -1308,15 +1302,12 @@ project--buffer-check
       (when (cond
              ((stringp c)
               (string-match-p c (buffer-name buf)))
-             ((symbolp c)
+             ((functionp c)
               (funcall c buf))
-             ((eq (car-safe c) 'derived-mode)
-              (warn "The use of `derived-mode' in \
-`project--buffer-check' is deprecated.")
-              (provided-mode-derived-p
-               (buffer-local-value 'major-mode buf)
-               (cdr c)))
              ((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)))
diff --git a/lisp/subr.el b/lisp/subr.el
index 50ae357a13..c1c9759b03 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -6855,9 +6855,11 @@ buffer-match-p
   arguments, and returns non-nil if the buffer matches,
 - a cons-cell, where the car describes how to interpret the cdr.
   The car can be one of the following:
-  * `major-mode': the buffer matches if the buffer's major
-    mode is derived from the major mode denoted by the cons-cell's
-    cdr
+  * `derived-mode': the buffer matches if the buffer's major mode
+    is derived from the major mode in the cons-cell's cdr.
+  * `major-mode': the buffer matches if the buffer's major mode
+    is eq to the cons-cell's cdr.  Prefer using `derived-mode'
+    instead when both can work.
   * `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.
@@ -6877,6 +6879,10 @@ buffer-match-p
                           (funcall condition buffer)
                         (funcall condition buffer arg)))
                      ((eq (car-safe condition) 'major-mode)
+                      (eq
+                       (buffer-local-value 'major-mode buffer)
+                       (cdr condition)))
+                     ((eq (car-safe condition) 'derived-mode)
                       (provided-mode-derived-p
                        (buffer-local-value 'major-mode buffer)
                        (cdr condition)))

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Fri, 17 Jun 2022 04:21:04 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > In any case, if we will keep both major-mode and derived-mode
> > conditions in subr.el, let's at least document that derived-mode
> > should be preferred where it can do the job, and major-mode used only
> > where derived-mode will not DTRT.
> 
> Sure. Here you go.

Fine with me, with one nit:

> +Satisfied if the buffer's major mode is eq to @var{expr}.  Prefer

Using "eq" as a word in the manual is not a good idea.  We use "is
equal" elsewhere in the manual.

Thanks.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 17.06.2022 08:48, Eli Zaretskii wrote:
>> Date: Fri, 17 Jun 2022 04:21:04 +0300
>> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>>> In any case, if we will keep both major-mode and derived-mode
>>> conditions in subr.el, let's at least document that derived-mode
>>> should be preferred where it can do the job, and major-mode used only
>>> where derived-mode will not DTRT.
>>
>> Sure. Here you go.
> 
> Fine with me, with one nit:
> 
>> +Satisfied if the buffer's major mode is eq to @var{expr}.  Prefer
> 
> Using "eq" as a word in the manual is not a good idea.  We use "is
> equal" elsewhere in the manual.

Thanks, done. Backward compatibility and capability are restored.

Now, if we wanted to downplay the option of doing equality comparison 
for major-mode, we could perhaps introduce a more general condition like 
'value-equal'.

That would change the definition of project-kill-buffer-conditions very 
slightly, still keeping it readable. And a gradual deprecation of the 
'major-mode' matcher could be done.

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index 30f51704dc..dea11d4231 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1223,10 +1223,10 @@ project-display-buffer-other-frame
  (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)
+    (value-equal major-mode . fundamental-mode)
      ;; non-text buffer such as xref, occur, vc, log, ...
      (and (derived-mode . special-mode)
-         (not (major-mode . help-mode)))
+         (not (value-equal major-mode . help-mode)))
      (derived-mode . compilation-mode)
      (derived-mode . dired-mode)
      (derived-mode . diff-mode)

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Wed, 15 Jun 2022 14:48:52 +0300
>> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> 
>> On 15.06.2022 05:33, Eli Zaretskii wrote:
>> > This seems to be some misunderstanding.  I wanted us to have only
>> > 'derived-mode', not 'major-mode'.
>> 
>> Okay?...
>> 
>> > But Philip was unhappy about that,
>> > due to existing usage of 'major-mode', and eventually 'major-mode' was
>> > left in the code.  Or at least this is my recollection of the
>> > discussion and its resolution.
>> 
>> 'major-mode' was left in the code and in the docs, but its meaning was 
>> changed. Hence the backward compatibility concern.
>> 
>> And 'derived-mode' was dropped.
>> 
>> Or you could say that 'major-mode' was dropped, and 'derived-mode' was 
>> left it, but renamed.
>
> Then I'm sorry, but I don't think I have anything to do with that
> change.  If I somehow cause Philip to make such changes, I apologize,
> because I never meant anything even close (and cannot understand how
> what I wrote could have interpreted to that effect, but that's water
> under the bridge).
>
> Once again, my only comment about the code was that I thought (and
> still think) we should not use major-mode matching, but derived-mode
> matching instead.

I have to apologise myself, first of all for the mistake in the current
implementation and second of all for the late response.  I should have
clarified what I was doing, and had assumed your comments were blocking
requirements for me to push any changes.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 15.06.2022 19:59, Philip Kaludercic wrote:
> and had assumed your comments were blocking
> requirements for me to push any changes

FWIW, I wouldn't blame you for having that impression.

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> Date: Mon, 14 Mar 2022 08:21:56 +0000
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> >> From: Philip Kaludercic <philipk@posteo.net>
>> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> >> Date: Sun, 13 Mar 2022 20:40:49 +0000
>> >> 
>> >> Eli Zaretskii <eliz@gnu.org> writes:
>> >> 
>> >> >> From: Philip Kaludercic <philipk@posteo.net>
>> >> >> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> >> >> Date: Fri, 11 Mar 2022 16:21:06 +0000
>> >> >> 
>> >> >> > I think we want in general avoid comparison with major-mode, and
>> >> >> > prefer derived-mode instead, and if so, IMO we had better
>> >> >> > did as we
>> >> >> > say and not exposed comparison to major mode unless we
>> >> >> > absolutely
>> >> >> > must.
>> >> >> 
>> >> >> Would it be enough to clarify this point in the documentation
>> >> >> string?
>> >> >
>> >> > What would you like to clarify?
>> >> 
>> >> To clarify that the usage of (major-mode . foo-mode) might not be
>> >> what
>> >> the user intends, and that in most cases derived-mode is preferable.
>> >
>> > I suggested to "clarify" that by not providing the 'major-mode'
>> > predicate at all.  I still don't think I understand why it is so
>> > important that we should provide a special case for it.
>> 
>> It is not inherently important, it is just that if the predicate would
>> also be used in project.el, then compatibility would have to be broken,
>> as the distinction between `major-mode' and `derived-mode' exists there.
>
> Then project.el could use the predicate route, right?  It's quite a
> special case, AFAIU, so having a special solution is OK.

I have updated the commits as you recommended, and add a commit
deprecating the use of `derived-mode' in project.el.

-- 
	Philip Kaludercic


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Generalise-buffer-matching-from-project.el.patch --]
[-- Type: text/x-patch, Size: 3420 bytes --]

From f218d19837ce53e5b94d16356594a5f48bd8f3d1 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Mon, 7 Mar 2022 20:49:42 +0100
Subject: [PATCH 1/3] Generalise buffer matching from project.el

* subr.el (buffer-match): Add function to check if a buffer satisfies
a condition.
(match-buffers): Returns all buffers that satisfy a condition.
---
 lisp/subr.el | 59 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 59 insertions(+)

diff --git a/lisp/subr.el b/lisp/subr.el
index e7d5d36461..d0b73db019 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -6651,4 +6651,63 @@ string-lines
 If OMIT-NULLS, empty lines will be removed from the results."
   (split-string string "\n" omit-nulls))
 
+(defun buffer-match-p (condition buffer-or-name &optional arg)
+  "Return non-nil if BUFFER-OR-NAME matches CONDITION.
+CONDITION is either:
+- a regular expression, to match a buffer name,
+- a predicate function that takes a buffer object and ARG as
+  arguments, and returns non-nil if the buffer matches,
+- a cons-cell, where the car describes how to interpret the cdr.
+  The car can be one of the following:
+  * `major-mode': the buffer matches if the buffer's 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 condition, of which at
+    least one has to be met."
+  (letrec
+      ((buffer (get-buffer buffer-or-name))
+       (match
+        (lambda (conditions)
+          (catch 'match
+            (dolist (condition conditions)
+              (when (cond
+                     ((stringp condition)
+                      (string-match-p condition (buffer-name buffer)))
+                     ((functionp condition)
+                      (if (eq 1 (cdr (func-arity condition)))
+                          (funcall condition buffer)
+                        (funcall condition buffer arg)))
+                     ((eq (car-safe condition) 'major-mode)
+                      (provided-mode-derived-p
+                       (buffer-local-value 'major-mode buffer)
+                       (cdr condition)))
+                     ((eq (car-safe condition) 'not)
+                      (not (funcall match (cdr condition))))
+                     ((eq (car-safe condition) 'or)
+                      (funcall match (cdr condition)))
+                     ((eq (car-safe condition) 'and)
+                      (catch 'fail
+                        (dolist (c conditions)
+                          (unless (funcall match c)
+                            (throw 'fail nil)))
+                        t)))
+                (throw 'match t)))))))
+    (funcall match (list condition))))
+
+(defun match-buffers (condition &optional buffers arg)
+  "Return a list of buffers that match CONDITION.
+See `buffer-match' for details on CONDITION.  By default all
+buffers are checked, this can be restricted by passing an
+optional argument BUFFERS, set to a list of buffers to check.
+ARG is passed to `buffer-match', for predicate conditions in
+CONDITION."
+  (let (bufs)
+    (dolist (buf (or buffers (buffer-list)))
+      (when (buffer-match-p condition (get-buffer buf) arg)
+        (push buf bufs)))
+    bufs))
+
 ;;; subr.el ends here
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-window.el-display-buffer-assq-regexp-Use-buffer-matc.patch --]
[-- Type: text/x-patch, Size: 1543 bytes --]

From b6e03a6d4dda8db47405475675f2defb5b8520d9 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 10 Mar 2022 10:59:52 +0100
Subject: [PATCH 2/3] * window.el (display-buffer-assq-regexp): Use
 buffer-match

---
 lisp/window.el | 15 ++++-----------
 1 file changed, 4 insertions(+), 11 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index dd297a3169..2da2f8bb2c 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7498,19 +7498,12 @@ display-buffer-fallback-action
 (defun display-buffer-assq-regexp (buffer-name alist action)
   "Retrieve ALIST entry corresponding to BUFFER-NAME.
 This returns the cdr of the alist entry ALIST if either its key
-is a string that matches BUFFER-NAME, as reported by
-`string-match-p'; or if the key is a function that returns
-non-nil when called with three arguments: the ALIST key,
-BUFFER-NAME and ACTION.  ACTION should have the form of the
-action argument passed to `display-buffer'."
+satisfied a BUFFER-NAME per `buffer-match'.  ACTION should have
+the form of the action argument passed to `display-buffer'."
   (catch 'match
     (dolist (entry alist)
-      (let ((key (car entry)))
-	(when (or (and (stringp key)
-		       (string-match-p key buffer-name))
-		  (and (functionp key)
-		       (funcall key buffer-name action)))
-	  (throw 'match (cdr entry)))))))
+      (when (buffer-match-p (car entry) buffer-name action)
+        (throw 'match (cdr entry))))))
 
 (defvar display-buffer--same-window-action
   '(display-buffer-same-window
-- 
2.34.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0003-Update-project-kill-buffer-conditions-to-match-buffe.patch --]
[-- Type: text/x-patch, Size: 3427 bytes --]

From 1f2e73890f378ee4a25a6bcd2683f922115a4a58 Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk@posteo.net>
Date: Thu, 14 Apr 2022 10:24:27 +0200
Subject: [PATCH 3/3] 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 | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
index daaf86f327..1d04c19906 100644
--- a/lisp/progmodes/project.el
+++ b/lisp/progmodes/project.el
@@ -1203,16 +1203,20 @@ project-display-buffer-other-frame
 (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)
+    (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)
+         (lambda (buf)
+           (not (eq (buffer-local-value 'major-mode buf)
+                    'help-mode))))
+    (major-mode . compilation-mode)
+    (major-mode . dired-mode)
+    (major-mode . diff-mode)
+    (major-mode . comint-mode)
+    (major-mode . eshell-mode)
+    (major-mode . change-log-mode))
   "List of conditions to kill buffers related to a project.
 This list is used by `project-kill-buffers'.
 Each condition is either:
@@ -1222,10 +1226,11 @@ project-kill-buffer-conditions
 - 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 buffer's major
-    mode is eq to the cons-cell's cdr
-  * `derived-mode': the buffer is killed if the buffer's major
     mode is derived from the major mode denoted by the cons-cell's
-    cdr
+    cdr.
+  * `derived-mode': the buffer is killed if the buffer's major
+    mode is eq to the cons-cell's cdr (this is deprecated and will
+    result in a warning if used).
   * `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.
@@ -1285,10 +1290,13 @@ project--buffer-check
               (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)
+              (warn "The use of `derived-mode' in \
+`project--buffer-check' is deprecated.")
+              (provided-mode-derived-p
+               (buffer-local-value 'major-mode buf)
+               (cdr c)))
+             ((eq (car-safe c) 'major-mode)
               (provided-mode-derived-p
                (buffer-local-value 'major-mode buf)
                (cdr c)))
-- 
2.34.0

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> From: Philip Kaludercic <philipk@posteo.net>
> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
> Date: Thu, 14 Apr 2022 08:25:58 +0000
> 
> >> > I suggested to "clarify" that by not providing the 'major-mode'
> >> > predicate at all.  I still don't think I understand why it is so
> >> > important that we should provide a special case for it.
> >> 
> >> It is not inherently important, it is just that if the predicate would
> >> also be used in project.el, then compatibility would have to be broken,
> >> as the distinction between `major-mode' and `derived-mode' exists there.
> >
> > Then project.el could use the predicate route, right?  It's quite a
> > special case, AFAIU, so having a special solution is OK.
> 
> I have updated the commits as you recommended, and add a commit
> deprecating the use of `derived-mode' in project.el.

Thanks, LGTM.

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Philip Kaludercic <philipk@posteo.net>
>> Cc: larsi@gnus.org,  54296@debbugs.gnu.org
>> Date: Thu, 14 Apr 2022 08:25:58 +0000
>> 
>> >> > I suggested to "clarify" that by not providing the 'major-mode'
>> >> > predicate at all.  I still don't think I understand why it is so
>> >> > important that we should provide a special case for it.
>> >> 
>> >> It is not inherently important, it is just that if the predicate would
>> >> also be used in project.el, then compatibility would have to be broken,
>> >> as the distinction between `major-mode' and `derived-mode' exists there.
>> >
>> > Then project.el could use the predicate route, right?  It's quite a
>> > special case, AFAIU, so having a special solution is OK.
>> 
>> I have updated the commits as you recommended, and add a commit
>> deprecating the use of `derived-mode' in project.el.
>
> Thanks, LGTM.

Great!  The commits have been pushed.

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 14.03.2022 05:21, Eli Zaretskii wrote:
> I still don't think I understand why it is so
> important that we should provide a special case for it.

It's pretty valuable to be able to provide a default value of 
project-kill-buffer-conditions which can be read at a glance (including 
in compiled Emacs) and easily edited via customize-variable by a user 
with little to no knowledge of Elisp.

And 'major-mode' was used twice inside project-kill-buffer-conditions, 
not just once.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Mon, 13 Jun 2022 03:23:47 +0300
> Cc: 54296@debbugs.gnu.org, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 14.03.2022 05:21, Eli Zaretskii wrote:
> > I still don't think I understand why it is so
> > important that we should provide a special case for it.
> 
> It's pretty valuable to be able to provide a default value of 
> project-kill-buffer-conditions which can be read at a glance (including 
> in compiled Emacs) and easily edited via customize-variable by a user 
> with little to no knowledge of Elisp.
> 
> And 'major-mode' was used twice inside project-kill-buffer-conditions, 
> not just once.

I think I lost context, so please bear with me.

My "still don't think I understand" comment was about the need to
support major-mode when apparently the _only_ user of that is
fundamental-mode, where we cannot use derived-mode instead.  If indeed
fundamental-mode is the only case where major-mode is useful, I asked
why not use a predicate that tests explicitly for that case.

And if I understand your reasoning above, then doesn't

  (eq major-mode 'fundamental-mode)

provide an easy default that can be read at a glance?

Or what am I missing?

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 13.06.2022 15:13, Eli Zaretskii wrote:
> My "still don't think I understand" comment was about the need to
> support major-mode when apparently the_only_  user of that is
> fundamental-mode, where we cannot use derived-mode instead.

There are two uses of it inside project-kill-buffer-conditions.

One is

   (major-mode . fundamental-mode)

, and another is

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

I suppose in the latter it doesn't really matter whether the check uses 
'derived-mode-p', but the current way is more conservative.

Also see the turn-on-font-lock-if-desired example from the other email.

> If indeed
> fundamental-mode is the only case where major-mode is useful, I asked
> why not use a predicate that tests explicitly for that case.

If a predicate is a lambda, then it's not as readable.

> And if I understand your reasoning above, then doesn't
> 
>    (eq major-mode 'fundamental-mode)
> 
> provide an easy default that can be read at a glance?

Are you proposing adding a "keyword" 'eq' which would have a variable 
name as its first arg and the value as its second arg?

That would work (aside from the backward compatibility concerns). It 
might look confusing to a Lisp programmer, resembling "regular" code too 
much, creating false expectations.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Tue, 14 Jun 2022 22:00:20 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> > And if I understand your reasoning above, then doesn't
> > 
> >    (eq major-mode 'fundamental-mode)
> > 
> > provide an easy default that can be read at a glance?
> 
> Are you proposing adding a "keyword" 'eq' which would have a variable 
> name as its first arg and the value as its second arg?

No, I proposed to use the "predicate function" method (which already
exists).

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 14.06.2022 22:17, Eli Zaretskii wrote:
> No, I proposed to use the "predicate function" method (which already
> exists).

Then the downside is "it's not readable". Or editable, really.

At this point you could say that we need to support lambas anyway (ok), 
and if people use them, we can use it for this purpose too.

But it seems to me that, by not supporting lambdas so far, over the 
couple of years since its introduction, project-kill-buffer-conditions 
has kind of proven its minimal DSL to be viable.

And if by having the 'major-mode eq' matcher we manage to avoid the use 
of lambdas in 99%+ use cases, then that's a win for usability.

bug#54296: Add buffer-matching functionality

[Eli Zaretskii]

> Date: Tue, 14 Jun 2022 22:46:49 +0300
> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
> From: Dmitry Gutov <dgutov@yandex.ru>
> 
> On 14.06.2022 22:17, Eli Zaretskii wrote:
> > No, I proposed to use the "predicate function" method (which already
> > exists).
> 
> Then the downside is "it's not readable". Or editable, really.
> 
> At this point you could say that we need to support lambas anyway (ok), 
> and if people use them, we can use it for this purpose too.
> 
> But it seems to me that, by not supporting lambdas so far, over the 
> couple of years since its introduction, project-kill-buffer-conditions 
> has kind of proven its minimal DSL to be viable.
> 
> And if by having the 'major-mode eq' matcher we manage to avoid the use 
> of lambdas in 99%+ use cases, then that's a win for usability.

This is all a moot point, since eventually 'major-mode' was left in
the code.  Any of the suggestions I had for removing it are therefore
purely academic at this point, nothing to argue about.

bug#54296: Add buffer-matching functionality

[Dmitry Gutov]

On 15.06.2022 05:34, Eli Zaretskii wrote:
>> Date: Tue, 14 Jun 2022 22:46:49 +0300
>> Cc: 54296@debbugs.gnu.org, philipk@posteo.net, larsi@gnus.org
>> From: Dmitry Gutov <dgutov@yandex.ru>
>>
>> On 14.06.2022 22:17, Eli Zaretskii wrote:
>>> No, I proposed to use the "predicate function" method (which already
>>> exists).
>>
>> Then the downside is "it's not readable". Or editable, really.
>>
>> At this point you could say that we need to support lambas anyway (ok),
>> and if people use them, we can use it for this purpose too.
>>
>> But it seems to me that, by not supporting lambdas so far, over the
>> couple of years since its introduction, project-kill-buffer-conditions
>> has kind of proven its minimal DSL to be viable.
>>
>> And if by having the 'major-mode eq' matcher we manage to avoid the use
>> of lambdas in 99%+ use cases, then that's a win for usability.
> 
> This is all a moot point, since eventually 'major-mode' was left in
> the code.  Any of the suggestions I had for removing it are therefore
> purely academic at this point, nothing to argue about.

'major-mode' that was left works like 'derived-mode' did.

If we take your suggestion as-is, though (and rename 'major-mode' back 
to 'derived-mode'), that would at least fix the main backward 
compatibility concern.

And with that, we could go back to discussing the stuff I wrote above.

bug#54296: Add buffer-matching functionality

[Augusto Stoffel]

Coincidentally, I've been discussing with Stefan a similar mini-language
for the 'font-lock-ignore' feature.  Here are some thoughts

On Thu, 10 Mar 2022 at 13:53, Eli Zaretskii <eliz@gnu.org> wrote:

>> +- a cons-cell, where the car describes how to interpret the cdr.
>> +  The car can be one of the following:
>> +  * `major-mode': the buffer matches if the buffer's major
>> +    mode is eq to the cons-cell's cdr
>> +  * `derived-mode': the buffer matches if the buffer's major
>> +    mode is derived from the major mode denoted by the cons-cell's
>> +    cdr
>
> Do we really need both major-mode and derived-mode?

It seems handy to allow (mode MODE) to match a buffer with major mode
derived from MODE or if MODE is bound and true as a variable in that
buffer (say, it's the name of a minor mode).

>> +  * `not': the cdr is interpreted as a negation of a condition.
>> +  * `and': the cdr is a list of recursive condition, that all have
>> +    to be met.                            ^^^^^^^^^
>
> Typo: should be "conditions".
>
>> +  * `or': the cdr is a list of recursive condition, of which at
>> +    least one has to be met."            ^^^^^^^^^

In 'font-lock-ignore' you can prefix an or-condition with !, with the
gitignore semantics.  This seems very appropriate for a buffer-matching
functionality.

But then `or' is not the familiar short-circuiting
operation, and you need to evaluate the conditions backwards...

On a more bikesheddy note, I'm also debating a bit what is more
readable:

    (or cond1 cond2 ! cond2)

or else
  
    (or cond1 cond2 (except cond2))

bug#54296: Add buffer-matching functionality

[Philip Kaludercic]

Augusto Stoffel <arstoffel@gmail.com> writes:

>>> +  * `not': the cdr is interpreted as a negation of a condition.
>>> +  * `and': the cdr is a list of recursive condition, that all have
>>> +    to be met.                            ^^^^^^^^^
>>
>> Typo: should be "conditions".
>>
>>> +  * `or': the cdr is a list of recursive condition, of which at
>>> +    least one has to be met."            ^^^^^^^^^
>
> In 'font-lock-ignore' you can prefix an or-condition with !, with the
> gitignore semantics.  This seems very appropriate for a buffer-matching
> functionality.
>
> But then `or' is not the familiar short-circuiting
> operation, and you need to evaluate the conditions backwards...
>
> On a more bikesheddy note, I'm also debating a bit what is more
> readable:
>
>     (or cond1 cond2 ! cond2)
>
> or else
>   
>     (or cond1 cond2 (except cond2))

If this would be included, I would certainly prefer the latter.

-- 
	Philip Kaludercic

bug#54296: Add buffer-matching functionality

[martin rudalics]

 > This seems to work, and updating window.el was also pretty trivial
 > (unless I have missed something):

IIUC the doc-string would now say that

   This returns the cdr of the alist entry ALIST if either its key
   satisfied a BUFFER-NAME per `buffer-match'.

which doesn't appear correct.  In either case please make sure that your
change does not affect existing behavior where a buffer is not displayed
in a specific way because it does not match 'display-buffer-assq-regexp'
until now.

Thanks, martin