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

From: Stefan Monnier <[email-masked]>
Date: Mon, 16 Oct 2023 12:33:10 -0400

> Sigh.  I guess we can install this on emacs-29 and cross the
> fingers...

I'm not insisting on it.
If you think it's risky, let's just have it on `master`.

BTW, see below the actual patch, including changes to etc/NEWS, manual,
as well as the addition of a runtime warning (without which I can't see
how we'll ever be able to get rid of the ugly backward compatibility
hack).

        Stefan
diff --git a/doc/lispref/buffers.texi b/doc/lispref/buffers.texi
index 86c47ae7310..a2d0f5687ba 100644
--- a/doc/lispref/buffers.texi
+++ b/doc/lispref/buffers.texi
@@ -957,10 +957,10 @@ Buffer List
 infinite recursion.
 @end defvar
 
-@defun buffer-match-p condition buffer-or-name &optional arg
+@defun buffer-match-p condition buffer-or-name &rest args
 This function checks if a buffer designated by @code{buffer-or-name}
-satisfies the specified @code{condition}.  Optional third argument
-@var{arg} is passed to the predicate function in @var{condition}.  A
+satisfies the specified @code{condition}.  Optional arguments
+@var{args} are passed to the predicate function in @var{condition}.  A
 valid @var{condition} can be one of the following:
 @itemize @bullet{}
 @item
@@ -969,23 +969,21 @@ Buffer List
 name.
 @item
 A predicate function, which should return non-@code{nil} if the buffer
-matches.  If the function expects one argument, it is called with
-@var{buffer-or-name} as the argument; if it expects 2 arguments, the
-first argument is @var{buffer-or-name} and the second is @var{arg}
-(or @code{nil} if @var{arg} is omitted).
+matches.  It is called with
+@var{buffer-or-name} as the first argument followed by @var{args}.
 @item
 A cons-cell @code{(@var{oper} . @var{expr})} where @var{oper} is one
 of
 @table @code
 @item (not @var{cond})
 Satisfied if @var{cond} doesn't satisfy @code{buffer-match-p} with
-the same buffer and @code{arg}.
+the same buffer and @code{args}.
 @item (or @var{conds}@dots{})
 Satisfied if @emph{any} condition in @var{conds} satisfies
-@code{buffer-match-p}, with the same buffer and @code{arg}.
+@code{buffer-match-p}, with the same buffer and @code{args}.
 @item (and @var{conds}@dots{})
 Satisfied if @emph{all} the conditions in @var{conds} satisfy
-@code{buffer-match-p}, with the same buffer and @code{arg}.
+@code{buffer-match-p}, with the same buffer and @code{args}.
 @item derived-mode
 Satisfied if the buffer's major mode derives from @var{expr}.
 @item major-mode
@@ -998,14 +996,14 @@ Buffer List
 @end itemize
 @end defun
 
-@defun match-buffers condition &optional buffer-list arg
+@defun match-buffers condition &optional buffer-list &rest args
 This function returns a list of all buffers that satisfy the
 @code{condition}.  If no buffers match, the function returns
 @code{nil}.  The argument @var{condition} is as defined in
 @code{buffer-match-p} above.  By default, all the buffers are
 considered, but this can be restricted via the optional argument
 @code{buffer-list}, which should be a list of buffers to consider.
-Optional third argument @var{arg} will be passed to @var{condition} in
+Remaining arguments @var{args} will be passed to @var{condition} in
 the same way as @code{buffer-match-p} does.
 @end defun
 
diff --git a/etc/NEWS b/etc/NEWS
index 3bd47a0112b..c74b4978afe 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -945,6 +945,13 @@ the file listing's performance is still optimized.
 
 * Incompatible Lisp Changes in Emacs 30.1
 
+** `buffer-match-p and `match-buffers` take `&rest args`
+They used to take a single `&optional arg` and were documented to use
+an unreliable hack to try and accommodate condition predicates that
+don't accept this optional arg.
+The new semantics makes no such affordances, tho the code still
+supports it (with a warning) for backward compatibility.
+
 ** 'post-gc-hook' runs after updating 'gcs-done' and 'gcs-elapsed'.
 
 ---
diff --git a/lisp/subr.el b/lisp/subr.el
index 58274987d71..0732319ccd0 100644
--- a/lisp/subr.el
+++ b/lisp/subr.el
@@ -7277,13 +7277,15 @@ string-lines
             (setq start (length string)))))
       (nreverse lines))))
 
-(defun buffer-match-p (condition buffer-or-name &optional arg)
+(defvar buffer-match-p--past-warnings nil)
+
+(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 +7310,18 @@ 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)))
+                       (if (cdr args)
+                           ;; New in Emacs>29.1. no need for compatibility hack.
+                           (apply condition buffer-or-name args)
+                         (condition-case-unless-debug err
+                             (apply condition buffer-or-name args)
+                           (wrong-number-of-arguments
+                            (unless (member condition
+                                            buffer-match-p--past-warnings)
+                              (message "%s" (error-message-string err))
+                              (push condition buffer-match-p--past-warnings))
+                            (apply condition buffer-or-name
+                                   (if args nil '(nil)))))))
                       (`(major-mode . ,mode)
                        (eq
                         (buffer-local-value 'major-mode buffer)
@@ -7332,17 +7343,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))
 
diff --git a/lisp/window.el b/lisp/window.el
index 2f9b46ebb0a..12d3fb1dfe7 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -7535,10 +7535,8 @@ display-buffer-alist
   arguments: a buffer to display and an alist of the same form as
   ALIST.  See `display-buffer' for details.
 
-`display-buffer' scans this alist until it either finds a
-matching regular expression or the function specified by a
-condition returns non-nil.  In any of these cases, it adds the
-associated action to the list of actions it will try."
+`display-buffer' scans this alist until the CONDITION is satisfied
+and adds the associated ACTION to the list of actions it will try."
   :type `(alist :key-type
 		(choice :tag "Condition"
 			regexp