bug-gnu-emacs
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

bug#65797: `buffer-match-p` should not use `func-arity`


From: Stefan Monnier
Subject: bug#65797: `buffer-match-p` should not use `func-arity`
Date: Sat, 14 Oct 2023 10:31:29 -0400
User-agent: Gnus/5.13 (Gnus v5.13)

> TBH, I have no way of assessing the risks in such a change.

Same here, which is why my first patch tried to play it safe.

> Do we have to make this change on the release branch?

That's another question, indeed.

> What bad things will happen if we leave emacs-29 with no changes?

Nothing too serious.
`buffer-match-p` is new in Emacs-29 and it is documented (in the Texinfo,
not in the docstring) to provide a behavior we're unable to implement.
So the main aim of the patch is to "fix" this new API so it can be
implemented as documented.

But the problem is somewhat corner-case, so it's not super urgent to fix it.

OTOH, the change is minor and fairly safe.
Basically the patch replaces an &optional with a &rest in the API.
Besides allowing more cases (which is mostly a non-issue in terms of
backward compatibility), this introduces just 1 potential problem:

When `buffer-match-p` is called without the formerly optional arg, it
will call the predicate functions with one fewer arg than before.

The Emacs-29.1 code has a hack that tries to detect when the predicate
expects "one fewer arg" and if so calls it without the optional arg, so
I just extended that hack to also handle that reverse problem (when
there is one fewer arg than expected by the function).

If we put it into `master`, I guess we can stick to the original hack
and hope the above "just 1 potential problem" won't bite us, but it
seems we may as well use the more backward compliant option and put it
into `emacs-29`.

See below my current "safe" choice.


        Stefan


diff --git a/lisp/subr.el b/lisp/subr.el
index 58274987d71..89bf8b44be7 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7277,13 +7275,13 @@ string-lines
             (setq start (length string)))))
       (nreverse lines))))
 
-(defun buffer-match-p (condition buffer-or-name &optional arg)
+(defun buffer-match-p (condition buffer-or-name &rest args)
   "Return non-nil if BUFFER-OR-NAME matches CONDITION.
 CONDITION is either:
 - the symbol t, to always match,
 - the symbol nil, which never matches,
 - a regular expression, to match a buffer name,
-- a predicate function that takes BUFFER-OR-NAME and ARG as
+- a predicate function that takes BUFFER-OR-NAME plus ARGS 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:
@@ -7308,9 +7306,16 @@ buffer-match-p
                       ((pred stringp)
                        (string-match-p condition (buffer-name buffer)))
                       ((pred functionp)
-                       (if (eq 1 (cdr (func-arity condition)))
-                           (funcall condition buffer-or-name)
-                         (funcall condition buffer-or-name arg)))
+                       (let ((max (cdr (func-arity condition))))
+                         (if (if args
+                                 ;; Old compatibility code present
+                                 ;; and documented in Emacs-29.1.
+                                 (and (null (cdr args)) (eq 1 max))
+                               ;; Backward compatibility with Emacs-29.1.
+                               (or (eq max 'many) (> max 1)))
+                             (apply condition buffer-or-name
+                                    (if args '(nil) nil))
+                           (apply condition buffer-or-name args))))
                       (`(major-mode . ,mode)
                        (eq
                         (buffer-local-value 'major-mode buffer)
@@ -7332,17 +7337,17 @@ buffer-match-p
                 (throw 'match t)))))))
     (funcall match (list condition))))
 
-(defun match-buffers (condition &optional buffers arg)
+(defun match-buffers (condition &optional buffers &rest args)
   "Return a list of buffers that match CONDITION, or nil if none match.
 See `buffer-match-p' for various supported CONDITIONs.
 By default all buffers are checked, but the optional
 argument BUFFERS can restrict that: its value should be
 an explicit list of buffers to check.
-Optional argument ARG is passed to `buffer-match-p', for
+Optional arguments ARGS are passed to `buffer-match-p', for
 predicate conditions in CONDITION."
   (let (bufs)
     (dolist (buf (or buffers (buffer-list)))
-      (when (buffer-match-p condition (get-buffer buf) arg)
+      (when (apply #'buffer-match-p condition (get-buffer buf) args)
         (push buf bufs)))
     bufs))
 






reply via email to

[Prev in Thread] Current Thread [Next in Thread]