bug#68081: 30.0.50; derived-mode and display-buffer-alist

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[German Pacenza]

Hi,
display-buffer-alist rules that use derived-mode or major-mode are
ignored on first use.

emacs -Q:

(setq display-buffer-alist '(((derived-mode . Info-mode)
                              (display-buffer-in-side-window))))
"C-h i"
The info buffer takes the whole window
"q"
"C-h i"
The info buffer opens in side window as expected

Info and Compilation modes are affected, but Help and Man work as expected


In GNU Emacs 30.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version
 3.24.39, cairo version 1.18.0) of 2023-12-27 built on KRONOS
Repository revision: 9e0eeb2d49ccd443bb667be9231fe932be67bb10
Repository branch: master
System Description: Manjaro Linux

Configured using:
 'configure --with-pgtk --with-native-compilation --without-gsettings
 --without-sound --without-compress-install'

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM HARFBUZZ JPEG JSON LCMS2
LIBOTF LIBSYSTEMD LIBXML2 MODULES NATIVE_COMP NOTIFY INOTIFY PDUMPER
PGTK PNG RSVG SECCOMP SQLITE3 THREADS TIFF TOOLKIT_SCROLL_BARS
TREE_SITTER WEBP XIM GTK3 ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Messages

Minor modes in effect:
  savehist-mode: t
  vertico-mode: t
  delete-selection-mode: t
  spacious-padding-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  minibuffer-regexp-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/home/german/.emacs.d/elpa/ef-themes-1.4.1/theme-loaddefs hides /home/german/.emacs.d/elpa/standard-themes-2.0.1/theme-loaddefs
/home/german/.emacs.d/elpa/transient-20231216.1908/transient hides /usr/local/share/emacs/30.0.50/lisp/transient
/home/german/.emacs.d/elpa/ef-themes-1.4.1/theme-loaddefs hides /usr/local/share/emacs/30.0.50/lisp/theme-loaddefs

Features:
(shadow sort mail-extr emacsbug message mailcap yank-media puny dired
dired-loaddefs rfc822 mml mml-sec password-cache epa derived epg rfc6068
epg-config gnus-util mm-decode mm-bodies mm-encode mail-parse rfc2231
mailabbrev gmm-utils mailheader sendmail rfc2047 rfc2045 ietf-drums
mm-util mail-prsvr mail-utils mule-util ibuffer ibuffer-loaddefs pulse
color misearch multi-isearch executable cl-print debug backtrace
find-func cl-macs time-date comp comp-cstr warnings icons subr-x
disp-table view jka-compr woman tabify files-x imenu compile comint
ansi-osc ring man ansi-color shortdoc pcase help-fns radix-tree
thingatpt cl-seq project byte-opt gv comp-run bytecomp byte-compile
comp-common rx cl-extra help-mode orderless cursor-sensor vc-git
diff-mode easy-mmode vc-dispatcher consult bookmark text-property-search
pp cl-loaddefs cl-lib completion-preview elec-pair savehist init
g3r-windows g3r-vertico vertico compat g3r-vc g3r-shells g3r-settings
delsel g3r-search g3r-project g3r-package g3r-modeline g3r-minibuffer
g3r-mail g3r-functions g3r-expand_region g3r-erc g3r-embark g3r-elfeed
g3r-consult g3r-completion g3r-bindings g3r-appearance spacious-padding
fontaine g3r-dark-theme init-dir info cape-autoloads
color-theme-sanityinc-tomorrow-autoloads consult-autoloads
dirvish-autoloads do-at-point-autoloads doom-themes-autoloads
eat-autoloads ef-themes-autoloads elfeed-autoloads embark-autoloads
expand-region-autoloads fontaine-autoloads golden-ratio-autoloads
hc-zenburn-theme-autoloads init-dir-autoloads kkp-autoloads
magit-autoloads git-commit-autoloads magit-section-autoloads
dash-autoloads markdown-mode-autoloads orderless-autoloads
rainbow-mode-autoloads spacious-padding-autoloads
standard-themes-autoloads sudoku-autoloads transient-dwim-autoloads
transient-autoloads vertico-autoloads with-editor-autoloads
compat-autoloads zenburn-theme-autoloads early-init rmc iso-transl
tooltip cconv eldoc paren electric uniquify ediff-hook vc-hooks
lisp-float-type elisp-mode mwheel term/pgtk-win pgtk-win term/common-win
pgtk-dnd tool-bar dnd fontset image regexp-opt fringe tabulated-list
replace newcomment text-mode lisp-mode prog-mode register page tab-bar
menu-bar rfn-eshadow isearch easymenu timer select scroll-bar mouse
jit-lock font-lock syntax font-core term/tty-colors frame minibuffer
nadvice seq simple cl-generic indonesian philippine cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite emoji-zwj charscript charprop
case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads dbusbind inotify dynamic-setting font-render-setting cairo gtk
pgtk lcms2 multi-tty move-toolbar make-network-process native-compile
emacs)

Memory information:
((conses 16 278958 357388) (symbols 48 12466 8)
 (strings 32 71116 11685) (string-bytes 1 2237572) (vectors 16 24796)
 (vector-slots 8 483019 124700) (floats 8 207 5606)
 (intervals 56 561 412) (buffers 992 12))

-- 
German Pacenza

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> From: German Pacenza <germanp82@hotmail.com>
> Date: Thu, 28 Dec 2023 10:26:37 -0300
> 
> display-buffer-alist rules that use derived-mode or major-mode are
> ignored on first use.
> 
> emacs -Q:
> 
> (setq display-buffer-alist '(((derived-mode . Info-mode)
>                               (display-buffer-in-side-window))))
> "C-h i"
> The info buffer takes the whole window
> "q"
> "C-h i"
> The info buffer opens in side window as expected
> 
> Info and Compilation modes are affected, but Help and Man work as expected

Martin, any comments or suggestions?

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 >> display-buffer-alist rules that use derived-mode or major-mode are
 >> ignored on first use.
 >>
 >> emacs -Q:
 >>
 >> (setq display-buffer-alist '(((derived-mode . Info-mode)
 >>                                (display-buffer-in-side-window))))
 >> "C-h i"
 >> The info buffer takes the whole window
 >> "q"
 >> "C-h i"
 >> The info buffer opens in side window as expected
 >>
 >> Info and Compilation modes are affected, but Help and Man work as expected
 >
 > Martin, any comments or suggestions?

C-h i does

   (info-setup file-or-node
	      (pop-to-buffer-same-window (or buffer "*info*"))))

In the first call BUFFER is nil and the

                        (provided-mode-derived-p
                         (buffer-local-value 'major-mode buffer)
                         mode))

rigmarole in 'buffer-match-p' won't report a match because the major
mode of *info* is still fundamental mode.  In later calls the *info*
buffer exists already, is in Info-mode, and 'buffer-match-p' will
produce the desired result.

We could try to call 'Info-mode' _before_ calling 'display-buffer' but
I'm not sure of the consequences this would have.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Fri, 29 Dec 2023 10:02:50 +0100
> Cc: 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> display-buffer-alist rules that use derived-mode or major-mode are
>  >> ignored on first use.
>  >>
>  >> emacs -Q:
>  >>
>  >> (setq display-buffer-alist '(((derived-mode . Info-mode)
>  >>                                (display-buffer-in-side-window))))
>  >> "C-h i"
>  >> The info buffer takes the whole window
>  >> "q"
>  >> "C-h i"
>  >> The info buffer opens in side window as expected
>  >>
>  >> Info and Compilation modes are affected, but Help and Man work as expected
>  >
>  > Martin, any comments or suggestions?
> 
> C-h i does
> 
>    (info-setup file-or-node
> 	      (pop-to-buffer-same-window (or buffer "*info*"))))
> 
> In the first call BUFFER is nil and the
> 
>                         (provided-mode-derived-p
>                          (buffer-local-value 'major-mode buffer)
>                          mode))
> 
> rigmarole in 'buffer-match-p' won't report a match because the major
> mode of *info* is still fundamental mode.  In later calls the *info*
> buffer exists already, is in Info-mode, and 'buffer-match-p' will
> produce the desired result.
> 
> We could try to call 'Info-mode' _before_ calling 'display-buffer' but
> I'm not sure of the consequences this would have.

Thanks.  I tend to document this subtlety, and otherwise leave it
alone.

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 > Thanks.  I tend to document this subtlety, and otherwise leave it
 > alone.

This was not an issue when 'display-buffer' was rewritten in 2011.  It
became an issue when it started to use 'buffer-match-p' in 2022.

A fairly safe fix would be

diff --git a/lisp/info.el b/lisp/info.el
index 51e9eb72edf..e0d35591ee5 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -768,6 +768,12 @@ info
      ;; of names that might have been wrapped (in emails, etc.).
      (setq file-or-node
            (string-replace "\n" " " file-or-node)))
+
+  (unless (or buffer (derived-mode-p 'Info-mode))
+    (setq buffer (get-buffer-create "*info*"))
+    (with-current-buffer buffer
+      (Info-mode)))
+
    (info-setup file-or-node
  	      (pop-to-buffer-same-window (or buffer "*info*"))))

A still conservative but more advanced fix (that should DTRT in the case
no window can be found) would be

diff --git a/lisp/info.el b/lisp/info.el
index 51e9eb72edf..11e228b9bf8 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -768,8 +768,16 @@ info
      ;; of names that might have been wrapped (in emails, etc.).
      (setq file-or-node
            (string-replace "\n" " " file-or-node)))
-  (info-setup file-or-node
-	      (pop-to-buffer-same-window (or buffer "*info*"))))
+
+  (unless buffer
+    (if (derived-mode-p 'Info-mode)
+	(setq buffer "*info*")
+      (setq buffer (get-buffer-create "*info*"))
+      (with-current-buffer buffer
+	(Info-mode))))
+
+  (pop-to-buffer-same-window buffer)
+  (info-setup file-or-node buffer))

  (defun info-setup (file-or-node buffer)
    "Display Info node FILE-OR-NODE in BUFFER."

Otherwise, you could suggest using

(setq display-buffer-alist '(((derived-mode . Info-mode)
                               (display-buffer-in-side-window))
			     ("*info*" (display-buffer-in-side-window))))

as a fallback.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Sat, 30 Dec 2023 10:30:40 +0100
> Cc: germanp82@hotmail.com, 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Thanks.  I tend to document this subtlety, and otherwise leave it
>  > alone.
> 
> This was not an issue when 'display-buffer' was rewritten in 2011.  It
> became an issue when it started to use 'buffer-match-p' in 2022.

I guess it's a problem on the emacs-29 branch as well, then?

> A fairly safe fix would be
> 
> diff --git a/lisp/info.el b/lisp/info.el
> index 51e9eb72edf..e0d35591ee5 100644
> --- a/lisp/info.el
> +++ b/lisp/info.el
> @@ -768,6 +768,12 @@ info
>       ;; of names that might have been wrapped (in emails, etc.).
>       (setq file-or-node
>             (string-replace "\n" " " file-or-node)))
> +
> +  (unless (or buffer (derived-mode-p 'Info-mode))
> +    (setq buffer (get-buffer-create "*info*"))
> +    (with-current-buffer buffer
> +      (Info-mode)))
> +
>     (info-setup file-or-node
>   	      (pop-to-buffer-same-window (or buffer "*info*"))))
> 
> A still conservative but more advanced fix (that should DTRT in the case
> no window can be found) would be
> 
> diff --git a/lisp/info.el b/lisp/info.el
> index 51e9eb72edf..11e228b9bf8 100644
> --- a/lisp/info.el
> +++ b/lisp/info.el
> @@ -768,8 +768,16 @@ info
>       ;; of names that might have been wrapped (in emails, etc.).
>       (setq file-or-node
>             (string-replace "\n" " " file-or-node)))
> -  (info-setup file-or-node
> -	      (pop-to-buffer-same-window (or buffer "*info*"))))
> +
> +  (unless buffer
> +    (if (derived-mode-p 'Info-mode)
> +	(setq buffer "*info*")
> +      (setq buffer (get-buffer-create "*info*"))
> +      (with-current-buffer buffer
> +	(Info-mode))))
> +
> +  (pop-to-buffer-same-window buffer)
> +  (info-setup file-or-node buffer))
> 
>   (defun info-setup (file-or-node buffer)
>     "Display Info node FILE-OR-NODE in BUFFER."
> 
> Otherwise, you could suggest using
> 
> (setq display-buffer-alist '(((derived-mode . Info-mode)
>                                (display-buffer-in-side-window))
> 			     ("*info*" (display-buffer-in-side-window))))
> 
> as a fallback.

Thanks.  Would you recommend any of the above for the emacs-29 branch?
Or are these not safe enough there?

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 >> This was not an issue when 'display-buffer' was rewritten in 2011.  It
 >> became an issue when it started to use 'buffer-match-p' in 2022.
 >
 > I guess it's a problem on the emacs-29 branch as well, then?

Yes.

 > Thanks.  Would you recommend any of the above for the emacs-29 branch?
 > Or are these not safe enough there?

The "fixes" I proposed are not "nice" since they depend on the
idempotence of 'Info-mode' which in the case we talk about here would be
called twice - once by 'info' and once by 'info-setup'.  Whether this
makes the fixes less "safe" is something I wouldn't try to experiment
with on a release branch.

So for Emacs-29 I would recommend to add to the Elisp manual, where it
says that "Each condition is passed to ‘buffer-match-p’ ...", that using
'derived-mode' and 'major-mode' as conditions might not work if
'display-buffer' is called before the major mode of the buffer has been
established.  And I would add a similar remark to the description of
'buffer-match-p' itself.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[German Pacenza]

martin rudalics <rudalics@gmx.at> writes:

> So for Emacs-29 I would recommend to add to the Elisp manual, where it
> says that "Each condition is passed to ‘buffer-match-p’ ...", that using
> 'derived-mode' and 'major-mode' as conditions might not work if
> 'display-buffer' is called before the major mode of the buffer has been
> established.  And I would add a similar remark to the description of
> 'buffer-match-p' itself.

I agree, Info mode is only one of the affected modes, compilation-mode
also shows this behaviour and others might as well.

-- 
German Pacenza

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 > I agree, Info mode is only one of the affected modes, compilation-mode
 > also shows this behaviour and others might as well.

And these cannot be reasonably fixed all by moving the -mode calls in
front of the 'display-buffer' call in a consistent manner.

The original problem is with 'window-normalize-buffer-to-switch-to'.  It
should never be called in non-interactive use, so 'info' would have been
forced to provide a buffer as argument and not just a name.

I'm afraid that this inconsistency makes 'buffer-match-p' pretty useless
for 'pop-to-buffer' and its like.  A strong warning seems appropriate.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Mon, 1 Jan 2024 10:38:09 +0100
> Cc: Eli Zaretskii <eliz@gnu.org>, 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > I agree, Info mode is only one of the affected modes, compilation-mode
>  > also shows this behaviour and others might as well.
> 
> And these cannot be reasonably fixed all by moving the -mode calls in
> front of the 'display-buffer' call in a consistent manner.
> 
> The original problem is with 'window-normalize-buffer-to-switch-to'.  It
> should never be called in non-interactive use, so 'info' would have been
> forced to provide a buffer as argument and not just a name.
> 
> I'm afraid that this inconsistency makes 'buffer-match-p' pretty useless
> for 'pop-to-buffer' and its like.  A strong warning seems appropriate.

Thanks, but now I wonder whether we should revert the change which
made display-buffer call buffer-match-p.  It sounds like fixing the
breakage in any other way is either hard or fragile or nigh
impossible.

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 > Thanks, but now I wonder whether we should revert the change which
 > made display-buffer call buffer-match-p.

The problem is not with 'display-buffer'.  The problem is with
'pop-to-buffer' and 'switch-to-buffer'.  What would you tell people who
already customized 'display-buffer-alist' and are happy with how it
works with 'display-buffer'?

 > It sounds like fixing the
 > breakage in any other way is either hard or fragile or nigh
 > impossible.

'info' initially used 'switch-to-buffer'

     (if (get-buffer "*info*")
	(switch-to-buffer "*info*")
       (Info-directory))))

Later it called 'pop-to-buffer' as

     (if (get-buffer "*info*")
	(pop-to-buffer "*info*")
       (Info-directory))))

The breakage occurred when it started to call

   (pop-to-buffer "*info*")

without checking whether that buffer exists.  It sometimes backfires to
use a feature meant for interactive use (like 'pop-to-buffer' creating
its buffer autonomously) in non-interactive calls.  Sometimes it happens
decades after that feature was misused.

BTW note that C-h F ignores _any_ efforts you put into customizing
'display-buffer-alist' like say

(customize-set-variable
  'display-buffer-alist
  '(("\\*info\\*" display-buffer-pop-up-window (inhibit-same-window . t))))

So there is worse than C-h i.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Tue, 2 Jan 2024 11:46:26 +0100
> Cc: germanp82@hotmail.com, 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Thanks, but now I wonder whether we should revert the change which
>  > made display-buffer call buffer-match-p.
> 
> The problem is not with 'display-buffer'.  The problem is with
> 'pop-to-buffer' and 'switch-to-buffer'.  What would you tell people who
> already customized 'display-buffer-alist' and are happy with how it
> works with 'display-buffer'?
> 
>  > It sounds like fixing the
>  > breakage in any other way is either hard or fragile or nigh
>  > impossible.
> 
> 'info' initially used 'switch-to-buffer'
> 
>      (if (get-buffer "*info*")
> 	(switch-to-buffer "*info*")
>        (Info-directory))))
> 
> Later it called 'pop-to-buffer' as
> 
>      (if (get-buffer "*info*")
> 	(pop-to-buffer "*info*")
>        (Info-directory))))
> 
> The breakage occurred when it started to call
> 
>    (pop-to-buffer "*info*")
> 
> without checking whether that buffer exists.  It sometimes backfires to
> use a feature meant for interactive use (like 'pop-to-buffer' creating
> its buffer autonomously) in non-interactive calls.  Sometimes it happens
> decades after that feature was misused.

Do other places that are affected by the same change do the same
mistake of unconditionally calling pop-to-buffer?

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 >> 'info' initially used 'switch-to-buffer'
 >>
 >>       (if (get-buffer "*info*")
 >> 	(switch-to-buffer "*info*")
 >>         (Info-directory))))
 >>
 >> Later it called 'pop-to-buffer' as
 >>
 >>       (if (get-buffer "*info*")
 >> 	(pop-to-buffer "*info*")
 >>         (Info-directory))))
 >>
 >> The breakage occurred when it started to call
 >>
 >>     (pop-to-buffer "*info*")
 >>
 >> without checking whether that buffer exists.  It sometimes backfires to
 >> use a feature meant for interactive use (like 'pop-to-buffer' creating
 >> its buffer autonomously) in non-interactive calls.  Sometimes it happens
 >> decades after that feature was misused.
 >
 > Do other places that are affected by the same change do the same
 > mistake of unconditionally calling pop-to-buffer?

Maybe my formulation was not clear.  Basically, all calls of 'info'
without first argument are affected by the change.  But note that the
"callers" do not call 'pop-to-buffer' - 'info' does that.  And the
problem became virulent only when 'buffer-match-p' started to ask for
the buffer's derived mode.  With other words: Emacs versions before that
change were prepared for the eventual use of 'buffer-match-p'.  Later
versions were not.

The real crux is that non-interactively, 'info' displays the buffer at
all.  This considerably confuses programmers.  Take this (arbitrarily
taken) snippet from 'prolog-build-info-alist' and note how the author
tries to override the buffer display behavior of 'info' with the help of
two window excursions.

           (save-excursion
             (save-window-excursion
               ;; select any window but the minibuffer (as we cannot switch
               ;; buffers in minibuffer window.
               ;; I am not sure this is the right/best way
               (if (active-minibuffer-window)  ; nil if none active
                   (select-window (next-window)))
               ;; Do this after going away from minibuffer window
               (save-window-excursion
                 (info))
               (Info-goto-node prolog-info-predicate-index)

Or a similar snippet from 'cperl-info-buffer':

       (save-window-excursion
	;; Get Info running
	(require 'info)
	(cond (oldbuf
	       (set-buffer oldbuf)
	       (rename-buffer "*info-perl-tmp*")))
	(save-window-excursion
	  (info))
	(Info-find-node cperl-info-page (if type "perlvar" "perlfunc"))

Now if a user does

(customize-set-variable
  'display-buffer-alist
  '(("\\*info\\*" display-buffer-pop-up-frame)))

to display 'info' in a separate frame, restoring the previous window
configuration in these cases will not restore anything.  All these
usually "work" by virtue of one fact: That 'display-buffer' by default
reuses a window that shows the info buffer already and that the node
eventually found will be displayed in that window regardless of which
frame it is on.  All this is fragile though and will fail as soon as a
users starts to more thoroughly customize 'display-buffer-alist'.

The idea that info inherently works on displayed buffers only is also a
problem for info itself: 'Info-next' does

   (save-window-excursion
     (or (derived-mode-p 'Info-mode) (switch-to-buffer "*info*"))
     (Info-goto-node (Info-extract-pointer "next"))))

If you call it with the info window not selected, this first displays
*info* in the selected window and goes to the next node there.  Then it
shows the original buffer in the selected window again in the hope that
the effect shows up in any other window that displays *info*.  I have no
idea why info does not just work on the info buffer here as also Chong's
change log entry

     * info.el (Info-next, Info-prev, Info-up): Select info buffer, in
             case the user clicks on the link while another window is selected.

would indicate.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Thu, 4 Jan 2024 11:21:17 +0100
> Cc: germanp82@hotmail.com, 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> 'info' initially used 'switch-to-buffer'
>  >>
>  >>       (if (get-buffer "*info*")
>  >> 	(switch-to-buffer "*info*")
>  >>         (Info-directory))))
>  >>
>  >> Later it called 'pop-to-buffer' as
>  >>
>  >>       (if (get-buffer "*info*")
>  >> 	(pop-to-buffer "*info*")
>  >>         (Info-directory))))
>  >>
>  >> The breakage occurred when it started to call
>  >>
>  >>     (pop-to-buffer "*info*")
>  >>
>  >> without checking whether that buffer exists.  It sometimes backfires to
>  >> use a feature meant for interactive use (like 'pop-to-buffer' creating
>  >> its buffer autonomously) in non-interactive calls.  Sometimes it happens
>  >> decades after that feature was misused.
>  >
>  > Do other places that are affected by the same change do the same
>  > mistake of unconditionally calling pop-to-buffer?
> 
> Maybe my formulation was not clear.  Basically, all calls of 'info'
> without first argument are affected by the change.

Thanks, but I was asking about callers of pop-to-buffer other than
'info'.  German said in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68081#26 that other
callers, in addition to 'info' are also affected.

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 > Thanks, but I was asking about callers of pop-to-buffer other than
 > 'info'.  German said in
 > https://debbugs.gnu.org/cgi/bugreport.cgi?bug=68081#26 that other
 > callers, in addition to 'info' are also affected.

He mentioned 'compilation-mode' but I need an example for reproducing
the bug.  The only occurrence of 'pop-to-buffer' in compile.el is in
'compilation-goto-locus' and I don't think the problem happens there.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[German Pacenza]

> He mentioned 'compilation-mode' but I need an example for reproducing
> the bug.  The only occurrence of 'pop-to-buffer' in compile.el is in
> 'compilation-goto-locus' and I don't think the problem happens there.

(setq display-buffer-alist
    '(((derived-mode . compilation-mode)
       (display-buffer-in-side-window)
       (side . top))))

Run 'vc-update' command which creates a compilation-mode buffer.

-- 
German Pacenza

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 > (setq display-buffer-alist
 >      '(((derived-mode . compilation-mode)
 >         (display-buffer-in-side-window)
 >         (side . top))))
 >
 > Run 'vc-update' command which creates a compilation-mode buffer.

Thanks, but this doesn't create a compilation-mode buffer here.
'vc-log-internal-common' has these lines

     ;; Display after setting up major-mode, so display-buffer-alist can know
     ;; the major-mode.
     (pop-to-buffer buffer)

so vc.el seems to be aware of the problem.  Can you please spot the
'pop-to-buffer' call responsible for the misbehavior in your scenario?

Thanks, martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[German Pacenza]

martin rudalics <rudalics@gmx.at> writes:

> Thanks, but this doesn't create a compilation-mode buffer here.

On second inspection, you are right. The buffer is created in
fundamental-mode and then converted in compilation-mode. Commands
like 'compile' that create buffers in compilation-mode work as expected.
So I think there is nothing to fix here.

Thanks

-- 
German Pacenza

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> From: German Pacenza <germanp82@hotmail.com>
> Cc: martin rudalics via Bug reports for GNU "Emacs," the Swiss army knife of
>  text editors <bug-gnu-emacs@gnu.org>,  Eli Zaretskii <eliz@gnu.org>,
>   68081@debbugs.gnu.org
> Date: Sat, 06 Jan 2024 07:35:33 -0300
> 
> martin rudalics <rudalics@gmx.at> writes:
> 
> > Thanks, but this doesn't create a compilation-mode buffer here.
> 
> On second inspection, you are right. The buffer is created in
> fundamental-mode and then converted in compilation-mode. Commands
> like 'compile' that create buffers in compilation-mode work as expected.
> So I think there is nothing to fix here.

Thanks.

is there anything else to do here, or can we now close this bug?

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[German Pacenza]

Eli Zaretskii <eliz@gnu.org> writes:

> is there anything else to do here, or can we now close this bug?

Everything looks correct.

Thanks

-- 
German Pacenza

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> From: German Pacenza <germanp82@hotmail.com>
> Cc: 68081@debbugs.gnu.org,  rudalics@gmx.at
> Date: Sat, 06 Jan 2024 08:55:01 -0300
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > is there anything else to do here, or can we now close this bug?
> 
> Everything looks correct.

Thanks, closing.

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Cc: 68081@debbugs.gnu.org, rudalics@gmx.at
> Date: Sat, 06 Jan 2024 13:39:49 +0200
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: German Pacenza <germanp82@hotmail.com>
> > Cc: martin rudalics via Bug reports for GNU "Emacs," the Swiss army knife of
> >  text editors <bug-gnu-emacs@gnu.org>,  Eli Zaretskii <eliz@gnu.org>,
> >   68081@debbugs.gnu.org
> > Date: Sat, 06 Jan 2024 07:35:33 -0300
> > 
> > martin rudalics <rudalics@gmx.at> writes:
> > 
> > > Thanks, but this doesn't create a compilation-mode buffer here.
> > 
> > On second inspection, you are right. The buffer is created in
> > fundamental-mode and then converted in compilation-mode. Commands
> > like 'compile' that create buffers in compilation-mode work as expected.
> > So I think there is nothing to fix here.
> 
> Thanks.
> 
> is there anything else to do here, or can we now close this bug?

No further comments, so I'm no closing this bug.

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

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

Attached find a patch for info.el on master that should handle all cases
I found in a fairly sane way.  Please test it with the info commands you
know of.

I've not eliminated things like

   (save-window-excursion
     (or (derived-mode-p 'Info-mode) (info-pop-to-buffer))
     (Info-goto-node (Info-extract-pointer "next"))))

While these will hardly DTRT with multiple frames and occasionally
produce some flickering here, they allow to navigate info buffers that
are not displayed (however useful that is).

As for compilation modes, please provide an example of how they
misbehave.

Thanks, martin

[-- Attachment #2: info.el.diff --]
[-- Type: text/x-patch, Size: 5334 bytes --]

diff --git a/lisp/info.el b/lisp/info.el
index 51e9eb72edf..6f69af99958 100644
--- a/lisp/info.el
+++ b/lisp/info.el
@@ -732,8 +732,53 @@ info-other-window
 		    (read-file-name "Info file name: " nil nil t))
 		(if (numberp current-prefix-arg)
 		    (format "*info*<%s>" current-prefix-arg))))
-  (info-setup file-or-node
-	      (switch-to-buffer-other-window (or buffer "*info*"))))
+  (info-pop-to-buffer file-or-node buffer t))
+
+(defun info-pop-to-buffer (&optional file-or-node buffer-or-name other-window)
+  "Put Info node FILE-OR-NODE in specified buffer and display it.
+Optional argument FILE-OR-NODE is as for `info'.
+
+If the optional argument BUFFER-OR-NAME is a buffer, use that
+buffer.  If it is a string, use that string as the name of the
+buffer, creating it if it does not exist.  Otherwise, use a
+buffer with the name `*info*', creating it if it does not exist.
+
+Optional argument OTHER-WINDOW nil means to prefer the selected
+window.  OTHER-WINDOW non-nil means to prefer another window.
+Select the window used, if it has been made."
+  (let ((buffer (cond
+		 ((bufferp buffer-or-name)
+		  buffer-or-name)
+		 ((stringp buffer-or-name)
+		  (get-buffer-create buffer-or-name))
+		 (t
+		  (get-buffer-create "*info*")))))
+    (with-current-buffer buffer
+      (unless (derived-mode-p 'Info-mode)
+	(Info-mode)))
+
+    (let* ((window
+	    (display-buffer buffer
+			    (if other-window
+				'(nil (inhibit-same-window . t))
+			      '(display-buffer-same-window)))))
+      (with-current-buffer buffer
+	(if file-or-node
+	    ;; If argument already contains parentheses, don't add another set
+	    ;; since the argument will then be parsed improperly.  This also
+	    ;; has the added benefit of allowing node names to be included
+	    ;; following the parenthesized filename.
+	    (Info-goto-node
+	     (if (and (stringp file-or-node) (string-match "(.*)" file-or-node))
+		 file-or-node
+               (concat "(" file-or-node ")")))
+	  (if (and (zerop (buffer-size))
+		   (null Info-history))
+	      ;; If we just created the Info buffer, go to the directory.
+	      (Info-directory))))
+
+      (when window
+	(select-window window)))))
 
 ;;;###autoload (put 'info 'info-file (purecopy "emacs"))
 ;;;###autoload
@@ -768,8 +813,8 @@ info
     ;; of names that might have been wrapped (in emails, etc.).
     (setq file-or-node
           (string-replace "\n" " " file-or-node)))
-  (info-setup file-or-node
-	      (pop-to-buffer-same-window (or buffer "*info*"))))
+
+  (info-pop-to-buffer file-or-node buffer))
 
 (defun info-setup (file-or-node buffer)
   "Display Info node FILE-OR-NODE in BUFFER."
@@ -789,6 +834,8 @@ info-setup
 	;; If we just created the Info buffer, go to the directory.
 	(Info-directory))))
 
+(make-obsolete 'info-setup "use `info-pop-to-buffer' instead" "30.1")
+
 ;;;###autoload
 (defun info-emacs-manual ()
   "Display the Emacs manual in Info mode."
@@ -927,7 +974,7 @@ Info-find-node
   (setq nodename (info--node-canonicalize-whitespace nodename))
   (setq filename (Info-find-file filename noerror))
   ;; Go into Info buffer.
-  (or (derived-mode-p 'Info-mode) (switch-to-buffer "*info*"))
+  (or (derived-mode-p 'Info-mode) (info-pop-to-buffer filename))
   ;; Record the node we are leaving, if we were in one.
   (and (not no-going-back)
        Info-current-file
@@ -957,7 +1004,7 @@ Info-revert-find-node
   "Go to an Info node FILENAME and NODENAME, re-reading disk contents.
 When *info* is already displaying FILENAME and NODENAME, the window position
 is preserved, if possible."
-  (or (derived-mode-p 'Info-mode) (switch-to-buffer "*info*"))
+  (or (derived-mode-p 'Info-mode) (info-pop-to-buffer filename))
   (let ((old-filename Info-current-file)
 	(old-nodename Info-current-node)
 	(window-selected (eq (selected-window) (get-buffer-window)))
@@ -2290,7 +2337,7 @@ Info-next
   (interactive nil Info-mode)
   ;; In case another window is currently selected
   (save-window-excursion
-    (or (derived-mode-p 'Info-mode) (switch-to-buffer "*info*"))
+    (or (derived-mode-p 'Info-mode) (info-pop-to-buffer))
     (Info-goto-node (Info-extract-pointer "next"))))
 
 (defun Info-prev ()
@@ -2299,7 +2346,7 @@ Info-prev
   (interactive nil Info-mode)
   ;; In case another window is currently selected
   (save-window-excursion
-    (or (derived-mode-p 'Info-mode) (switch-to-buffer "*info*"))
+    (or (derived-mode-p 'Info-mode) (info-pop-to-buffer))
     (Info-goto-node (Info-extract-pointer "prev[ious]*" "previous"))))
 
 (defun Info-up (&optional same-file)
@@ -2308,7 +2355,7 @@ Info-up
   (interactive nil Info-mode)
   ;; In case another window is currently selected
   (save-window-excursion
-    (or (derived-mode-p 'Info-mode) (switch-to-buffer "*info*"))
+    (or (derived-mode-p 'Info-mode) (info-pop-to-buffer))
     (let ((old-node Info-current-node)
 	  (old-file Info-current-file)
 	  (node (Info-extract-pointer "up")) p)
@@ -5485,7 +5532,7 @@ info-display-manual
                 (raise-frame (window-frame window))
                 (select-frame-set-input-focus (window-frame window))
                 (select-window window))
-	    (switch-to-buffer found)))
+	    (info-pop-to-buffer nil found)))
       ;; The buffer doesn't exist; create it.
       (info-initialize)
       (info (Info-find-file manual)

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Wed, 3 Jan 2024 11:35:25 +0100
> Cc: germanp82@hotmail.com, 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
> Attached find a patch for info.el on master that should handle all cases
> I found in a fairly sane way.  Please test it with the info commands you
> know of.

Thanks.  This is a significant change, so probably not suitable for
the release branch.  Do you think the simple change of using the old

     (if (get-buffer "*info*")
	(pop-to-buffer "*info*")
       (Info-directory))))

is okay for installing on the release branch, or would you recommend
to leave the release branch without a fix and only fix this on master?

bug#68081: 30.0.50; derived-mode and display-buffer-alist

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

 > Thanks.  This is a significant change, so probably not suitable for
 > the release branch.  Do you think the simple change of using the old
 >
 >       (if (get-buffer "*info*")
 > 	(pop-to-buffer "*info*")
 >         (Info-directory))))
 >
 > is okay for installing on the release branch,

Where would we do that?  The context for that snippet has vanished long
ago and we need a buffer we can pass to 'info-setup'.

 > or would you recommend
 > to leave the release branch without a fix and only fix this on master?

Only fix this on master.  The warning for 'buffer-match-p' would be
needed for both - release branch and master - anyway.  Who knows how
many cases similar to this we have.

martin

bug#68081: 30.0.50; derived-mode and display-buffer-alist

[Eli Zaretskii]

> Date: Wed, 3 Jan 2024 11:35:25 +0100
> Cc: germanp82@hotmail.com, 68081@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
> Attached find a patch for info.el on master that should handle all cases
> I found in a fairly sane way.  Please test it with the info commands you
> know of.

Thanks, installed on master.