bug#71654: [PATCH] Fix display-buffer-override-next-command

bug#71654: [PATCH] Fix display-buffer-override-next-command

[kassick]

[-- Attachment #1.1: Type: text/plain, Size: 1421 bytes --]

When display-buffer-overriding-action's car is set to a function instead of
a list, display-buffer-override-next-command must ensure that the car is a
list before pushing a new element to it, otherwise the car will become a
itself a cons-cell and the clear-fun will trigger a wrong-type-argumen
error when trying to delq the action.

The error can be triggered by using some minor mode that
updates display-buffer-override-next-command (such as purpose-mode) and
then calling other-window-prefix.

The patch was created using the `emacs-29` branch.

----

In GNU Emacs 29.3.50 (build 8, x86_64-pc-linux-gnu, GTK+ Version
 3.24.41, cairo version 1.18.0) of 2024-05-27 built on thnkpd
System Description: Fedora Linux 39 (Workstation Edition)

Configured using:
 'configure --build x86_64-linux-gnu --prefix=/usr/local
 --sharedstatedir=/var/lib --localstatedir=/var/lib --enable-libsystemd
 --with-pop=yes --build x86_64-linux-gnu
 --enable-locallisppath=/etc/emacs:/usr/local/share/emacs/29/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/29/site-lisp:/usr/share/emacs/site-lisp
 --with-sound=alsa --without-gconf --with-mailutils --with-cairo
 --with-x=yes --with-x-toolkit=gtk3 --with-pgtk
 --with-toolkit-scroll-bars --without-xwidgets --with-imagemagick
 --with-native-compilation --with-tree-sitter --with-xinput2'

-- 
Rodrigo Virote Kassick
------------------------------------------------------------

[-- Attachment #1.2: Type: text/html, Size: 1765 bytes --]

[-- Attachment #2: 0001-Fix-display-buffer-override-next-command.patch --]
[-- Type: text/x-patch, Size: 1246 bytes --]

From 8fa11a451949be743d4bdedd5117571d912e2209 Mon Sep 17 00:00:00 2001
From: Rodrigo Kassick <kassick@gmail.com>
Date: Wed, 19 Jun 2024 11:46:35 -0300
Subject: [PATCH] Fix display-buffer-override-next-command

* lisp/window.el (display-buffer-override-next-command):
display-buffer-overriding-action's car can be either a function or a
list of functions. When a function,
(push action (car display-buffer-overriding-action)) will create a
cons-cell (action . function)  instead of a list. In the
clear-fun callback, trying to
(delq action (car display-buffer-overriding-action)) causes an
wrong-type-argument error.
---
 lisp/window.el | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lisp/window.el b/lisp/window.el
index 13fe1feba10..8db0584c910 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9109,6 +9109,8 @@ display-buffer-override-next-command
     (when echofun
       (add-hook 'prefix-command-echo-keystrokes-functions echofun))
     (setq switch-to-buffer-obey-display-actions t)
+    (unless (listp (car display-buffer-overriding-action))
+        (setcar display-buffer-overriding-action (list (car display-buffer-overriding-action))))
     (push action (car display-buffer-overriding-action))
     exitfun))
 
-- 
2.45.1

bug#71654: [PATCH] Fix display-buffer-override-next-command

[Jeremy Bryant via Bug reports for GNU Emacs, the Swiss army knife of text editors]

"kassick@gmail.com" <kassick@gmail.com> writes:

> When display-buffer-overriding-action's car is set to a function
> instead of a list, display-buffer-override-next-command must ensure
> that the car is a list before pushing a new element to it, otherwise
> the car will become a itself a cons-cell and the clear-fun will
> trigger a wrong-type-argumen error when trying to delq the action.
>
> The error can be triggered by using some minor mode that
> updates display-buffer-override-next-command (such as purpose-mode)
> and then calling other-window-prefix.
>

To reproduce the bug, what other built-in modes (instead of
purpose-mode) can be used?

bug#71654: [PATCH] Fix display-buffer-override-next-command

[kassick]

[-- Attachment #1: Type: text/plain, Size: 1270 bytes --]

No need for any mode, just

- M-S-:  (setq display-buffer-overriding-action (cons #'ignore nil))
RET      ;; Eval
- C-x 4 4   ;;  (other-window-prefix)
- C-x C-f  ;; (find-file)

and you should already see the error.

After that, you probably will end up with an unusable emacs, so just eval
(setq display-buffer-overriding-action (cons nil nil)) and you'll be good
to go.

Em qua., 19 de jun. de 2024 às 18:17, Jeremy Bryant <jb@jeremybryant.net>
escreveu:

> "kassick@gmail.com" <kassick@gmail.com> writes:
>
> > When display-buffer-overriding-action's car is set to a function
> > instead of a list, display-buffer-override-next-command must ensure
> > that the car is a list before pushing a new element to it, otherwise
> > the car will become a itself a cons-cell and the clear-fun will
> > trigger a wrong-type-argumen error when trying to delq the action.
> >
> > The error can be triggered by using some minor mode that
> > updates display-buffer-override-next-command (such as purpose-mode)
> > and then calling other-window-prefix.
> >
>
> To reproduce the bug, what other built-in modes (instead of
> purpose-mode) can be used?
>


-- 
Rodrigo Virote Kassick
------------------------------------------------------------

[-- Attachment #2: Type: text/html, Size: 1993 bytes --]

bug#71654: [PATCH] Fix display-buffer-override-next-command

[Andrea Corallo]

"kassick@gmail.com" <kassick@gmail.com> writes:

> When display-buffer-overriding-action's car is set to a function instead of a list, display-buffer-override-next-command
> must ensure that the car is a list before pushing a new element to it, otherwise the car will become a itself a cons-cell
> and the clear-fun will trigger a wrong-type-argumen error when trying to delq the action.
>
> The error can be triggered by using some minor mode that updates display-buffer-override-next-command (such as
> purpose-mode) and then calling other-window-prefix.

Hi Rodrigo,

the patch LGTM so I applied it after some indentation massage to master.

Closing then, happy to reopen if some more work is needed.

Thanks

  Andrea

bug#71654: [PATCH] Fix display-buffer-override-next-command

[kassick]

[-- Attachment #1: Type: text/plain, Size: 1027 bytes --]

Awesome!

Any chance of cherry-picking it on emacs-29?

Em qua., 19 de jun. de 2024 às 18:32, Andrea Corallo <acorallo@gnu.org>
escreveu:

> "kassick@gmail.com" <kassick@gmail.com> writes:
>
> > When display-buffer-overriding-action's car is set to a function instead
> of a list, display-buffer-override-next-command
> > must ensure that the car is a list before pushing a new element to it,
> otherwise the car will become a itself a cons-cell
> > and the clear-fun will trigger a wrong-type-argumen error when trying to
> delq the action.
> >
> > The error can be triggered by using some minor mode that updates
> display-buffer-override-next-command (such as
> > purpose-mode) and then calling other-window-prefix.
>
> Hi Rodrigo,
>
> the patch LGTM so I applied it after some indentation massage to master.
>
> Closing then, happy to reopen if some more work is needed.
>
> Thanks
>
>   Andrea
>


-- 
Rodrigo Virote Kassick
------------------------------------------------------------

[-- Attachment #2: Type: text/html, Size: 1643 bytes --]

bug#71654: [PATCH] Fix display-buffer-override-next-command

[Andrea Corallo]

"kassick@gmail.com" <kassick@gmail.com> writes:

> Awesome!
>
> Any chance of cherry-picking it on emacs-29?

We could do that if we feel sure this has no implications, but there
will be no more emacs 29 releases anyway and we are about to branch 30,
so I'm not sure it's worth of even if the change looks simple.

  Andrea

bug#71654: [PATCH] Fix display-buffer-override-next-command

[kassick]

[-- Attachment #1: Type: text/plain, Size: 722 bytes --]

Ah, time for me to update here, then!

Anyone having this issue in 29 or older could just use advice to patch the
function and move on, don't see any reason to backport it.



Em qua., 19 de jun. de 2024 às 19:11, Andrea Corallo <acorallo@gnu.org>
escreveu:

> "kassick@gmail.com" <kassick@gmail.com> writes:
>
> > Awesome!
> >
> > Any chance of cherry-picking it on emacs-29?
>
> We could do that if we feel sure this has no implications, but there
> will be no more emacs 29 releases anyway and we are about to branch 30,
> so I'm not sure it's worth of even if the change looks simple.
>
>   Andrea
>


-- 
Rodrigo Virote Kassick
------------------------------------------------------------

[-- Attachment #2: Type: text/html, Size: 1353 bytes --]

bug#71654: [PATCH] Fix display-buffer-override-next-command

[Andrea Corallo]

"kassick@gmail.com" <kassick@gmail.com> writes:

> Ah, time for me to update here, then!
>
> Anyone having this issue in 29 or older could just use advice to patch the function and move on, don't see any reason to
> backport it.

BTW for this we could rely on the copyright paperwork exempt because the
change was tiny, but if in the future you plan to contribute more to
Emacs might be good to do the FSF copyright paperwork so we are covered.

In case please let me know and I can send you the form.

Thanks for your contribution.

  Andrea

bug#71654: [PATCH] Fix display-buffer-override-next-command

[Eli Zaretskii]

> Cc: 71654-done@debbugs.gnu.org
> From: Andrea Corallo <acorallo@gnu.org>
> Date: Wed, 19 Jun 2024 18:11:27 -0400
> 
> "kassick@gmail.com" <kassick@gmail.com> writes:
> 
> > Awesome!
> >
> > Any chance of cherry-picking it on emacs-29?
> 
> We could do that if we feel sure this has no implications, but there
> will be no more emacs 29 releases anyway and we are about to branch 30,
> so I'm not sure it's worth of even if the change looks simple.

Agreed.