GNU bug report logs - #58951
[PATCH] ; Fix handling of 'not' by 'buffer-match-p'

Previous Next

Package: emacs;

Reported by: Philip Kaludercic <philipk <at> posteo.net>

Date: Tue, 1 Nov 2022 19:37:02 UTC

Severity: normal

Tags: patch

Fixed in version 29.1

Done: Philip Kaludercic <philipk <at> posteo.net>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 58951 in the body.
You can then email your comments to 58951 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Tue, 01 Nov 2022 19:37:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Philip Kaludercic <philipk <at> posteo.net>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Tue, 01 Nov 2022 19:37:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: bug-gnu-emacs <at> gnu.org
Subject: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'
Date: Tue, 01 Nov 2022 19:36:12 +0000
[Message part 1 (text/plain, inline)]
Tags: patch


In preparing bug#58950 I noticed that the 'not' clause is confusing, and
is misused on the place I could find it being used in the core
(show-paren-predicate).  The current implementation would require a
negation to be written as

    (not . CONDITION)

while it is more natural to write

    (not CONDITION)

which is more in line with (and ...) and (or ...).

The issue appears to go back to `project--buffer-check', that takes a
list of conditions instead of a single one.  This means that the above
are equivalent. since (not CONDITION) will check each element in the
unary list in (not . (CONDITION)).

I believe this is preferable to fixing `show-paren-predicate', as this
is the kind of issue a lot of people could trip over.

This patch is based on the patch from bug#58950, but can be back-ported
to the previous implementations if there are any issues with that report.

In GNU Emacs 29.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.30, cairo version 1.16.0) of 2022-10-31 built on heron
Repository revision: 462a66e79edcc34ecbeef7cc1604765adfdc038e
Repository branch: feature/package+vc
System Description: Guix System

Configured using:
 'configure --with-pgtk --with-imagemagick
 PKG_CONFIG_PATH=/gnu/store/ssg343s6ldqdwh30136pnawhbgd0cb6i-profile/lib/pkgconfig:/gnu/store/ssg343s6ldqdwh30136pnawhbgd0cb6i-profile/share/pkgconfig'

[0001-Fix-handling-of-not-by-buffer-match-p.patch (text/patch, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Tue, 01 Nov 2022 20:20:02 GMT) Full text and rfc822 format available.

Message #8 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: help-debbugs <at> gnu.org (GNU bug Tracking System)
Cc: 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: Acknowledgement ([PATCH] ; Fix handling of 'not' by
 'buffer-match-p')
Date: Tue, 01 Nov 2022 20:19:06 +0000
[Message part 1 (text/plain, inline)]
I forgot to update the documentation in buffers.texi:

[0001-Fix-handling-of-not-by-buffer-match-p.patch (text/x-patch, inline)]
From 279f9561dd115d707b086efdbc19072fceb7a99f Mon Sep 17 00:00:00 2001
From: Philip Kaludercic <philipk <at> posteo.net>
Date: Tue, 1 Nov 2022 20:27:17 +0100
Subject: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'

* lisp/subr.el (buffer-match-p): Look up the cadr instead of the cdr
for the negation in 'not'.
* doc/lispref/buffers.texi (Buffer List): Update documentation.
---
 doc/lispref/buffers.texi | 18 ++++++++----------
 lisp/subr.el             |  4 ++--
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi
index c40e088293..8405e28387 100644
--- a/doc/lispref/buffers.texi
+++ b/doc/lispref/buffers.texi
@@ -977,17 +977,15 @@ Buffer List
 A cons-cell @code{(@var{oper} . @var{expr})} where @var{oper} is one
 of
 @table @code
-@item not
-Satisfied if @var{expr} doesn't satisfy @code{buffer-match-p} with
+@item (not @var{cond})
+Satisfied if @var{cond} doesn't satisfy @code{buffer-match-p} with
 the same buffer and @code{arg}.
-@item or
-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{expr} is a list and @emph{all} conditions in
-@var{expr} satisfy @code{buffer-match-p}, with the same buffer and
-@code{arg}.
+@item (or @var{conds}@dots{})
+Satisfied if and @emph{any} condition in @var{conds} satisfies
+@code{buffer-match-p}, with the same buffer and @code{arg}.
+@item (and @var{conds}@dots{})
+Satisfied if @emph{all} conditions in @var{conds} 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
diff --git a/lisp/subr.el b/lisp/subr.el
index b667339db9..0252e66686 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7023,7 +7023,7 @@ string-lines
                `(provided-mode-derived-p
                  (buffer-local-value 'major-mode ,buffer-sym)
                  ',mode))
-              (`(not . ,cond)
+              (`(not ,cond)
                `(not ,(funcall translate cond)))
               (`(or . ,conds)
                `(or ,@(mapcar translate conds)))
@@ -7045,7 +7045,7 @@ string-lines
   * `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.
+  * `not': the cadr 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
-- 
2.38.0


Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Tue, 01 Nov 2022 22:10:01 GMT) Full text and rfc822 format available.

Message #11 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Philip Kaludercic <philipk <at> posteo.net>, 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'
Date: Wed, 2 Nov 2022 00:09:11 +0200
On 01.11.2022 21:36, Philip Kaludercic wrote:
> I believe this is preferable to fixing `show-paren-predicate', as this
> is the kind of issue a lot of people could trip over.
> 
> This patch is based on the patch from bug#58950, but can be back-ported
> to the previous implementations if there are any issues with that report.

I do agree that (not foo) is more convenient than (not . foo). Though 
the latter would be more regular compared to the rest of the syntax. I 
made just that mistake yesterday.

Let's change 'pcase' to 'pcase-exhaustive', though? So it will signal an 
error on invalid syntax.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Tue, 01 Nov 2022 22:19:01 GMT) Full text and rfc822 format available.

Message #14 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'
Date: Tue, 01 Nov 2022 22:18:04 +0000
Dmitry Gutov <dgutov <at> yandex.ru> writes:

> On 01.11.2022 21:36, Philip Kaludercic wrote:
>> I believe this is preferable to fixing `show-paren-predicate', as this
>> is the kind of issue a lot of people could trip over.
>> This patch is based on the patch from bug#58950, but can be
>> back-ported
>> to the previous implementations if there are any issues with that report.
>
> I do agree that (not foo) is more convenient than (not . foo). Though
> the latter would be more regular compared to the rest of the syntax. I
> made just that mistake yesterday.
>
> Let's change 'pcase' to 'pcase-exhaustive', though? So it will signal
> an error on invalid syntax.

Right, that is what I did in bug#58950 and what led me to notice the mistake.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Tue, 01 Nov 2022 22:30:02 GMT) Full text and rfc822 format available.

Message #17 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'
Date: Wed, 2 Nov 2022 00:29:36 +0200
On 02.11.2022 00:18, Philip Kaludercic wrote:
> Dmitry Gutov<dgutov <at> yandex.ru>  writes:
> 
>> On 01.11.2022 21:36, Philip Kaludercic wrote:
>>> I believe this is preferable to fixing `show-paren-predicate', as this
>>> is the kind of issue a lot of people could trip over.
>>> This patch is based on the patch from bug#58950, but can be
>>> back-ported
>>> to the previous implementations if there are any issues with that report.
>> I do agree that (not foo) is more convenient than (not . foo). Though
>> the latter would be more regular compared to the rest of the syntax. I
>> made just that mistake yesterday.
>>
>> Let's change 'pcase' to 'pcase-exhaustive', though? So it will signal
>> an error on invalid syntax.
> Right, that is what I did in bug#58950 and what led me to notice the mistake.

Oh, sorry. I didn't review that patch in detail yet.

LGTM then.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Wed, 02 Nov 2022 12:06:02 GMT) Full text and rfc822 format available.

Message #20 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: Acknowledgement ([PATCH] ;
 Fix handling of 'not' by 'buffer-match-p')
Date: Wed, 02 Nov 2022 14:04:45 +0200
> From: Philip Kaludercic <philipk <at> posteo.net>
> Date: Tue, 01 Nov 2022 20:19:06 +0000
> 
> +@item (or @var{conds}@dots{})
> +Satisfied if and @emph{any} condition in @var{conds} satisfies
                ^^^
That "and" should be deleted.

> +@item (and @var{conds}@dots{})
> +Satisfied if @emph{all} conditions in @var{conds} satisfy
                ^^^^^^^^^^^^^^^^^^^^^
"@emph{all} the conditions"

Thanks.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Sat, 31 Dec 2022 14:06:02 GMT) Full text and rfc822 format available.

Message #23 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'
Date: Sat, 31 Dec 2022 14:05:27 +0000
Eli Zaretskii <eliz <at> gnu.org> writes:

>> From: Philip Kaludercic <philipk <at> posteo.net>
>> Date: Tue, 01 Nov 2022 20:19:06 +0000
>> 
>> +@item (or @var{conds}@dots{})
>> +Satisfied if and @emph{any} condition in @var{conds} satisfies
>                 ^^^
> That "and" should be deleted.
>
>> +@item (and @var{conds}@dots{})
>> +Satisfied if @emph{all} conditions in @var{conds} satisfy
>                 ^^^^^^^^^^^^^^^^^^^^^
> "@emph{all} the conditions"
>
> Thanks.

I have applied these changes and will push the fix to the release
branch.




bug marked as fixed in version 29.1, send any further explanations to 58951 <at> debbugs.gnu.org and Philip Kaludercic <philipk <at> posteo.net> Request was from Philip Kaludercic <philipk <at> posteo.net> to control <at> debbugs.gnu.org. (Sat, 31 Dec 2022 14:06:02 GMT) Full text and rfc822 format available.

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Sat, 31 Dec 2022 16:27:01 GMT) Full text and rfc822 format available.

Message #28 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattiase <at> acm.org>
To: Philip Kaludercic <philipk <at> posteo.net>
Cc: 58951 <at> debbugs.gnu.org
Subject: bug#58951: [PATCH] ; Fix handling of 'not' by 'buffer-match-p' 
Date: Sat, 31 Dec 2022 17:26:02 +0100
>     (not . CONDITION)
> 
> while it is more natural to write
> 
>     (not CONDITION)

Shouldn't then the `derived-mode` and `major-mode` operators be changed in the same way?

And shouldn't there be tests? For all of `buffer-match-p`, in fact?





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#58951; Package emacs. (Sat, 31 Dec 2022 16:35:01 GMT) Full text and rfc822 format available.

Message #31 received at 58951 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattiase <at> acm.org>
Cc: philipk <at> posteo.net, 58951 <at> debbugs.gnu.org
Subject: Re: bug#58951: [PATCH] ; Fix handling of 'not' by 'buffer-match-p'
Date: Sat, 31 Dec 2022 18:34:15 +0200
> Cc: 58951 <at> debbugs.gnu.org
> From: Mattias Engdegård <mattiase <at> acm.org>
> Date: Sat, 31 Dec 2022 17:26:02 +0100
> 
> >     (not . CONDITION)
> > 
> > while it is more natural to write
> > 
> >     (not CONDITION)
> 
> Shouldn't then the `derived-mode` and `major-mode` operators be changed in the same way?

In fact, that change cause errors in timer function of
show-paren-mode, so I've reverted it for now.

> And shouldn't there be tests? For all of `buffer-match-p`, in fact?

Ideally, yes.




bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Sun, 29 Jan 2023 12:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 58 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.