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

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

bug#62417: ; Regression: 59ecf25fc860 is the first bad commit


From: João Távora
Subject: bug#62417: ; Regression: 59ecf25fc860 is the first bad commit
Date: Mon, 27 Mar 2023 14:08:17 +0000

On Mon, Mar 27, 2023 at 2:49 PM Eli Zaretskii <eliz@gnu.org> wrote:
>
> > From: João Távora <joaotavora@gmail.com>
> > Cc: philipk@posteo.net,  62417@debbugs.gnu.org
> > Date: Mon, 27 Mar 2023 13:06:06 +0100
> >
> > > Please install on emacs-29, and thanks.
> >
> > Done. I took the liberty of pushing this additional change to the patch
> > I showed, which is essential to make this work.  I had forgotten to show
> > it.  Unless there are objections, we can close this bug.
>
> I don't understand why is it essential,

It's very easy to reproduce this problem.  Just see the code snippet
I included as part of the commit.

Author: João Távora <joaotavora@gmail.com>
Date:   Mon Mar 27 12:25:16 2023 +0100

    Fix accidental backward-incompatible change (bug#62417)

    This code used to work, but with the change of 59ecf25fc860 it stopped
    working:

       (defun foop (buffer-name _alist) (string-match "foop" buffer-name))
       (add-to-list 'display-buffer-alist '(foop . display-buffer-other-frame))

    This change makes it work again, restoring compatibility.

    * lisp/subr.el (buffer-match-p): Fix and adjust docstring.
    * lisp/window.el (display-buffer-alist): Adjust docstring.
    (display-buffer-assq-regexp): Make good on promise of display-buffer-alist.


If you remove the extra part and try that snippet in both emacs-28
and emacs-29, you'll reach the same conclusion as I

> since buffer-match-p accepts
> both buffers and their names.  Please explain.

In the patch I showed, which you and Philip approved, the docstring of
the variable display-buffer-alist was clarified to state that it is a buffer
name string, and _not_ a buffer object, that is passed to buffer-match-p.
This is absolutely necessary, and we've already been through this.

But naturally it's not enough to simply state that fact in a docstring.
You have to actually make good on the promise by actually passing a
buffer name to buffer-match-p, and not a buffer.  Otherwise, the user
functions that the user places in display-buffer-alist WILL be called with a
buffer _object_ always.  And for people programming against Emacs < 29,
those functions are always passed a buffer name _string_.

Do you understand?

I think there is still confusion.  It's understandable, as this new
buffer-match-p protocol makes what was previously a relatively simple
protocol is much harder to understand, because there's an added level
of indirection.  Presumably added in the name of flexibility, but that
flexibility actually already existed in Emacs 28, the buffer-match-p
mini-language just adds so-called syntactic sugar.

As I said: there are other perfectly plausible ways to address this
problem, including removing buffer-match-p from display-buffer-alist
logic and losing this particular sugar.

> (And I wish you
> explained this before pushing, since there's no special rush anyway.)

There are people with broken SLYs in the Emacs 29 builds and master
for a long time.  See the original link. I wish I didn't let it get
this far, that was my bad, but this is hurting users today.

João





reply via email to

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