all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: "João Távora" <joaotavora@gmail.com>
To: Eli Zaretskii <eliz@gnu.org>
Cc: philipk@posteo.net, 62417@debbugs.gnu.org
Subject: bug#62417: ; Regression: 59ecf25fc860 is the first bad commit
Date: Mon, 27 Mar 2023 14:08:17 +0000	[thread overview]
Message-ID: <CALDnm50TBrKKzG7XTzqqVxSyWXoaUXAwqtAKfYU_jsbU+1K9sA@mail.gmail.com> (raw)
In-Reply-To: <838rfi9udc.fsf@gnu.org>

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	other threads:[~2023-03-27 14:08 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-24 13:16 bug#62417: 30.0.50; Regression: 59ecf25fc860 is the first bad commit João Távora
2023-03-24 15:22 ` João Távora
2023-03-24 16:05   ` Philip Kaludercic
2023-03-24 16:07     ` João Távora
2023-03-24 19:48       ` bug#62417: ; " João Távora
2023-03-25 12:55         ` Eli Zaretskii
2023-03-25 13:04           ` João Távora
2023-03-25 13:20             ` Eli Zaretskii
2023-03-25 13:56               ` João Távora
2023-03-25 14:13                 ` Eli Zaretskii
2023-03-25 14:15                   ` João Távora
2023-03-26 20:22                   ` João Távora
2023-03-26 21:23                     ` Philip Kaludercic
2023-03-27  2:24                     ` Eli Zaretskii
2023-03-27 12:06                       ` João Távora
2023-03-27 13:49                         ` Eli Zaretskii
2023-03-27 14:08                           ` João Távora [this message]
2023-03-27 15:20                             ` Eli Zaretskii
2023-03-27 16:33                               ` Eli Zaretskii
2023-03-27 16:42                                 ` João Távora
2023-03-27 17:09                                   ` Eli Zaretskii
2023-03-27 19:26                                     ` Philip Kaludercic
2023-03-28 11:11                                       ` Eli Zaretskii
2023-03-27 16:38                               ` João Távora
2023-03-25 13:17           ` Philip Kaludercic
2023-03-25 13:29             ` Eli Zaretskii

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CALDnm50TBrKKzG7XTzqqVxSyWXoaUXAwqtAKfYU_jsbU+1K9sA@mail.gmail.com \
    --to=joaotavora@gmail.com \
    --cc=62417@debbugs.gnu.org \
    --cc=eliz@gnu.org \
    --cc=philipk@posteo.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.