From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.io!.POSTED.blaine.gmane.org!not-for-mail From: Stefan Monnier via "Bug reports for GNU Emacs, the Swiss army knife of text editors" Newsgroups: gmane.emacs.bugs Subject: bug#65797: `buffer-match-p` should not use `func-arity` Date: Sat, 14 Oct 2023 10:31:29 -0400 Message-ID: References: <87v8cmct9b.fsf@breatheoutbreathe.in> <87sf7o38g1.fsf_-_@posteo.net> <871qf1rbdo.fsf@posteo.net> <87o7hz4zg2.fsf@posteo.net> <87sf7b8ker.fsf@posteo.net> <87y1h2blxq.fsf@posteo.net> <874jjqb715.fsf@posteo.net> <0371b6ff-58e4-961e-29c9-4efb65b82185@gutov.dev> <87v8bhtr3k.fsf@breatheoutbreathe.in> <3a282a0b-3efa-ec12-2143-0496ebf6c828@gutov.dev> <831qdxu3y8.fsf@gnu.org> Reply-To: Stefan Monnier Mime-Version: 1.0 Content-Type: text/plain Injection-Info: ciao.gmane.io; posting-host="blaine.gmane.org:116.202.254.214"; logging-data="1327"; mail-complaints-to="usenet@ciao.gmane.io" User-Agent: Gnus/5.13 (Gnus v5.13) Cc: 65797@debbugs.gnu.org, dmitry@gutov.dev, philipk@posteo.net, mattias.engdegard@gmail.com, joseph@breatheoutbreathe.in To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Sat Oct 14 16:32:48 2023 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane-mx.org Original-Received: from lists.gnu.org ([209.51.188.17]) by ciao.gmane.io with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from ) id 1qrfh9-000Adu-Dj for geb-bug-gnu-emacs@m.gmane-mx.org; Sat, 14 Oct 2023 16:32:47 +0200 Original-Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1qrfh1-00057j-P4; Sat, 14 Oct 2023 10:32:39 -0400 Original-Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1qrfh0-00057a-MY for bug-gnu-emacs@gnu.org; Sat, 14 Oct 2023 10:32:38 -0400 Original-Received: from debbugs.gnu.org ([2001:470:142:5::43]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.90_1) (envelope-from ) id 1qrfh0-0003d9-Eh for bug-gnu-emacs@gnu.org; Sat, 14 Oct 2023 10:32:38 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1qrfhO-0005s1-10 for bug-gnu-emacs@gnu.org; Sat, 14 Oct 2023 10:33:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Stefan Monnier Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Sat, 14 Oct 2023 14:33:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 65797 X-GNU-PR-Package: emacs Original-Received: via spool by 65797-submit@debbugs.gnu.org id=B65797.169729392822497 (code B ref 65797); Sat, 14 Oct 2023 14:33:01 +0000 Original-Received: (at 65797) by debbugs.gnu.org; 14 Oct 2023 14:32:08 +0000 Original-Received: from localhost ([127.0.0.1]:50091 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qrfgV-0005qn-Mm for submit@debbugs.gnu.org; Sat, 14 Oct 2023 10:32:08 -0400 Original-Received: from mailscanner.iro.umontreal.ca ([132.204.25.50]:42051) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1qrfgT-0005qJ-US for 65797@debbugs.gnu.org; Sat, 14 Oct 2023 10:32:06 -0400 Original-Received: from pmg1.iro.umontreal.ca (localhost.localdomain [127.0.0.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id 52EAA10013E; Sat, 14 Oct 2023 10:31:36 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=iro.umontreal.ca; s=mail; t=1697293890; bh=LoyPDc7+qPZFGQz56vIW0dC4zAGPmQrRDTEASxOvQJU=; h=From:To:Cc:Subject:In-Reply-To:References:Date:From; b=b8LIrLOPIgRxDXFSR8M2PhQyKPUOVuuXxeb/Trk3pt6UMWDAPCCVMJHdfYtWhLxRj 3QRotg3rADkrwnVma+8TKi+PSSFp/erAGjhSKDS75xj36A4QJvledXwrn0h0zuHpkL ahuRBYosiPtf0qEZKEf+b6W4IFbgzI6EjdiUGxuKiuwYSJpHa5qPbv6kKavDjQ4Kjk l7FR63RryKK/o5Fx9qkLIlniWlcw06eYuUdFyL2UqNmAOFiUofaPEybitnVJuIMbJD S2rmLFJr6/f8YI/dccyv4n6+OFXmF3DYhRFyJbpggaff/xVyKHAYZ3utL0PUIU0XgM FCnmdx4BfICfA== Original-Received: from mail01.iro.umontreal.ca (unknown [172.31.2.1]) by pmg1.iro.umontreal.ca (Proxmox) with ESMTP id BC18C100061; Sat, 14 Oct 2023 10:31:30 -0400 (EDT) Original-Received: from pastel (unknown [45.72.216.111]) by mail01.iro.umontreal.ca (Postfix) with ESMTPSA id 81BFD1204A6; Sat, 14 Oct 2023 10:31:30 -0400 (EDT) In-Reply-To: <831qdxu3y8.fsf@gnu.org> (Eli Zaretskii's message of "Sat, 14 Oct 2023 09:13:19 +0300") X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Original-Sender: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane-mx.org@gnu.org Xref: news.gmane.io gmane.emacs.bugs:272426 Archived-At: > 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))