Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Dmitry Gutov]

Hi Sam,

On 29.12.2021 01:30, Sam Steingold wrote:
> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
> index 3b634471ac..62dba7b393 100644
> --- a/lisp/progmodes/project.el
> +++ b/lisp/progmodes/project.el
> @@ -1015,7 +1015,7 @@ if one already exists."
>            (default-project-shell-name (project-prefixed-buffer-name "shell"))
>            (shell-buffer (get-buffer default-project-shell-name)))
>       (if (and shell-buffer (not current-prefix-arg))
> -        (pop-to-buffer-same-window shell-buffer)
> +        (pop-to-buffer shell-buffer display-comint-buffer-action)
>         (shell (generate-new-buffer-name default-project-shell-name)))))
>   
>   ;;;###autoload
> @@ -1031,7 +1031,7 @@ if one already exists."
>            (eshell-buffer-name (project-prefixed-buffer-name "eshell"))
>            (eshell-buffer (get-buffer eshell-buffer-name)))
>       (if (and eshell-buffer (not current-prefix-arg))
> -        (pop-to-buffer-same-window eshell-buffer)
> +        (pop-to-buffer eshell-buffer display-comint-buffer-action)

Please add boundp fallbacks: project.el should retain compatibility with 
older Emacs releases.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[jakanakaevangeli]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi Sam,
>
> On 29.12.2021 01:30, Sam Steingold wrote:
>> diff --git a/lisp/progmodes/project.el b/lisp/progmodes/project.el
>> index 3b634471ac..62dba7b393 100644
>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -1015,7 +1015,7 @@ if one already exists."
>>            (default-project-shell-name (project-prefixed-buffer-name "shell"))
>>            (shell-buffer (get-buffer default-project-shell-name)))
>>       (if (and shell-buffer (not current-prefix-arg))
>> -        (pop-to-buffer-same-window shell-buffer)
>> +        (pop-to-buffer shell-buffer display-comint-buffer-action)
>>         (shell (generate-new-buffer-name default-project-shell-name)))))
>>   
>>   ;;;###autoload
>> @@ -1031,7 +1031,7 @@ if one already exists."
>>            (eshell-buffer-name (project-prefixed-buffer-name "eshell"))
>>            (eshell-buffer (get-buffer eshell-buffer-name)))
>>       (if (and eshell-buffer (not current-prefix-arg))
>> -        (pop-to-buffer-same-window eshell-buffer)
>> +        (pop-to-buffer eshell-buffer display-comint-buffer-action)
>
> Please add boundp fallbacks: project.el should retain compatibility with 
> older Emacs releases.

Also here:

> diff --git a/lisp/shell.el b/lisp/shell.el
> index 370532ea46..1860e4691d 100644
> --- a/lisp/shell.el
> +++ b/lisp/shell.el
> @@ -758,7 +758,7 @@ shell
>                   (current-buffer)))
>    ;; The buffer's window must be correctly set when we call comint
>    ;; (so that comint sets the COLUMNS env var properly).
> -  (pop-to-buffer-same-window buffer)
> +  (pop-to-buffer buffer)
>  
>    (with-connection-local-variables
>     ;; On remote hosts, the local `shell-file-name' might be useless.

should probably be

  (pop-to-buffer buffer display-comint-buffer-action)

to make it analogous to the other changes in the patch.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

>> --- a/lisp/progmodes/project.el
>> +++ b/lisp/progmodes/project.el
>> @@ -1015,7 +1015,7 @@ if one already exists."
>> -        (pop-to-buffer-same-window shell-buffer)
>> +        (pop-to-buffer shell-buffer display-comint-buffer-action)
>
> Please add boundp fallbacks: project.el should retain compatibility with
> older Emacs releases.

Or better (bound-and-true-p display-comint-buffer-action)

> +(defcustom display-comint-buffer-action 'display-buffer-same-window

This default value is not what pop-to-buffer-same-window uses:

  (pop-to-buffer buffer display-buffer--same-window-action norecord)

So for backward-compatibility the default value needs to be the same:

  (defcustom display-comint-buffer-action display-buffer--same-window-action

that requires a different type:

  :type 'display-buffer--action-custom-type

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Lars Ingebrigtsen]

Sam Steingold <sds@gnu.org> writes:

>     Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'
>
>     * lisp/window.el (display-comint-buffer-action): New `defcustom`,
>     defaults to 'display-buffer-same-window' for backward compatibility.

Others have noted the technical problems with this patch, but as the
discussion in bug#52467 showed, Emacs already has capabilities for
customising the behaviour here (via the general display machinery), and
adding something cross-cutting but specific to the coming-derived modes
just seems odd to me, so I'd be in favour of reverting this patch.

Any opinions?  Eli?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Eli Zaretskii]

> From: Lars Ingebrigtsen <larsi@gnus.org>
> Cc: Sam Steingold <sdsg@amazon.com>, Eli Zaretskii <eliz@gnu.org>
> Date: Wed, 29 Dec 2021 16:00:44 +0100
> 
> Sam Steingold <sds@gnu.org> writes:
> 
> >     Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'
> >
> >     * lisp/window.el (display-comint-buffer-action): New `defcustom`,
> >     defaults to 'display-buffer-same-window' for backward compatibility.
> 
> Others have noted the technical problems with this patch, but as the
> discussion in bug#52467 showed, Emacs already has capabilities for
> customising the behaviour here (via the general display machinery), and
> adding something cross-cutting but specific to the coming-derived modes
> just seems odd to me, so I'd be in favour of reverting this patch.
> 
> Any opinions?  Eli?

Martin's opinions are more relevant, I think.

Me, I think we have way too many display-buffer customization
machinery, so much so that we are confused ourselves what we have and
what we don't.  But if others think this particular addition could be
useful, I don't mind: a defcustom doesn't by itself do any harm,
except perhaps making Emacs a tad more complex.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > Martin's opinions are more relevant, I think.

Sorry, but I simply don't understand what's going on here.  Sam
Steingold's original report starts with the following statement:

   I would like to request reverting of the patch

   70b64e0d040e9c57f1a489c9ebee553264033119 "Use pop-to-buffer-same-window for shell"

   When I already have a window with shell, this patch creates a second
   such window.
   It seems much more reasonable to use pop-to-buffer in eshell rather than
   break shell's behavior.

   If you insist on your desired behavior, please add a user variable
   `shell-pop-to-buffer-action` that you would set to
   `display-buffer--same-window-action`.

Now IIUC the original code had 'pop-to-buffer' which conceptually does
create a new window.  'pop-to-buffer-same-window' OTOH conceptually
reuses the same window instead.  So how that patch could provoke this
behavior

   When I already have a window with shell, this patch creates a second
   such window.

is beyond my understanding.

Personally, I would not have installed the patch above because it
changes long-standing default behavior.  Users can always request
displaying eshell in the same window via 'display-buffer-alist'.  But
the bug report itself yet remains a mystery to me.

 > Me, I think we have way too many display-buffer customization
 > machinery, so much so that we are confused ourselves what we have and
 > what we don't.  But if others think this particular addition could be
 > useful, I don't mind: a defcustom doesn't by itself do any harm,
 > except perhaps making Emacs a tad more complex.

If such a 'defcustom' does not affect a user's customizations via
'display-buffer-alist' I agree.  But if a user is then forced to use
that defcustom _instead of_ customizing 'display-buffer-alist', it would
do harm IMHO.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > Sorry, but I simply don't understand what's going on here.  Sam
 > Steingold's original report starts with the following statement:
 >
 >    I would like to request reverting of the patch
 >
 >    70b64e0d040e9c57f1a489c9ebee553264033119 "Use pop-to-buffer-same-window for shell"
 >
 >    When I already have a window with shell, this patch creates a second
 >    such window.
 >    It seems much more reasonable to use pop-to-buffer in eshell rather than
 >    break shell's behavior.
 >
 >    If you insist on your desired behavior, please add a user variable
 >    `shell-pop-to-buffer-action` that you would set to
 >    `display-buffer--same-window-action`.
 >
 > Now IIUC the original code had 'pop-to-buffer' which conceptually does
 > create a new window.  'pop-to-buffer-same-window' OTOH conceptually
 > reuses the same window instead.  So how that patch could provoke this
 > behavior
 >
 >    When I already have a window with shell, this patch creates a second
 >    such window.
 >
 > is beyond my understanding.

Sorry again, I slowly start understanding now.  I now think that the
original patch should be reverted or that we need a general new display
buffer action that preferably (1) reuses a window already showing the
buffer (2) uses the same window and only then (3) uses another window.
Maybe Juri has an idea.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

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

 > ... we need a general new display
 > buffer action that preferably (1) reuses a window already showing the
 > buffer (2) uses the same window and only then (3) uses another window.
 > Maybe Juri has an idea.

The attached diff should illustrate what I mean here.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: display-buffer-reuse-or-same-window.diff --]
[-- Type: text/x-patch; name="display-buffer-reuse-or-same-window.diff", Size: 3557 bytes --]

diff --git a/lisp/window.el b/lisp/window.el
index edca4a2da5..d55123bbd2 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8119,6 +8119,7 @@ display-buffer--action-function-custom-type
 	   (const display-buffer-reuse-window)
 	   (const display-buffer-pop-up-window)
 	   (const display-buffer-same-window)
+	   (const display-buffer-reuse-or-same-window)
 	   (const display-buffer-pop-up-frame)
 	   (const display-buffer-in-child-frame)
 	   (const display-buffer-below-selected)
@@ -8248,6 +8249,16 @@ display-buffer--same-window-action
 Specifies to call `display-buffer-same-window'.")
 (put 'display-buffer--same-window-action 'risky-local-variable t)

+(defvar display-buffer--reuse-or-same-window-action
+  '((display-buffer-reuse-window
+     display-buffer-same-window)
+    (inhibit-same-window . nil))
+  "A `display-buffer' action for reusing a window or using the same one.
+If a window showing the buffer exists already, reuse that window.
+Otherwise, use preferably the selected window.  Specifies to call
+`display-buffer-reuse-or-same-window'.")
+(put 'display-buffer--reuse-or-same-window-action 'risky-local-variable t)
+
 (defvar display-buffer--other-frame-action
   '((display-buffer-reuse-window
      display-buffer-pop-up-frame)
@@ -8266,7 +8277,9 @@ display-buffer
  `display-buffer-same-window' -- Use the selected window.
  `display-buffer-reuse-window' -- Use a window already showing
     the buffer.
- `display-buffer-in-previous-window' -- Use a window that did
+`display-buffer-reuse-or-same-window' -- Either reuse a window or
+    use the selected one.
+`display-buffer-in-previous-window' -- Use a window that did
     show the buffer before.
  `display-buffer-use-some-window' -- Use some existing window.
  `display-buffer-use-least-recent-window' -- Try to avoid re-using
@@ -8505,6 +8518,20 @@ display-buffer-same-window
 	      (window-dedicated-p))
     (window--display-buffer buffer (selected-window) 'reuse alist)))

+(defun display-buffer-reuse-or-same-window (buffer alist)
+  "Display BUFFER reusing a window showing it or the selected one.
+ALIST is an association list of action symbols and values.  See
+Info node `(elisp) Buffer Display Action Alists' for details of
+such alists.
+
+First try to use a window already showing BUFFER.  If no such
+window exists, do like `display-buffer-same-window'."
+  (or (display-buffer-reuse-window buffer alist)
+      (unless (or (cdr (assq 'inhibit-same-window alist))
+	          (window-minibuffer-p)
+	          (window-dedicated-p))
+        (window--display-buffer buffer (selected-window) 'reuse alist))))
+
 (defun display-buffer--maybe-same-window (buffer alist)
   "Conditionally display BUFFER in the selected window.
 ALIST is an association list of action symbols and values.  See
@@ -9339,6 +9366,13 @@ pop-to-buffer-same-window
 another window."
   (pop-to-buffer buffer display-buffer--same-window-action norecord))

+(defun pop-to-buffer-reuse-or-same-window (buffer &optional norecord)
+  "Select specified BUFFER in a window showing it or the same one.
+This is like `pop-to-buffer-same-window', but tries first to
+reuse a window already showing BUFFER and only if no such window
+exists behaves like `pop-to-buffer-same-window'."
+  (pop-to-buffer buffer display-buffer--reuse-or-same-window-action norecord))
+
 (defun read-buffer-to-switch (prompt)
   "Read the name of a buffer to switch to, prompting with PROMPT.
 Return the name of the buffer as a string.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Stefan Monnier]

> The attached diff should illustrate what I mean here.

Looks like a good idea to me.

I don't understand why `display-buffer-reuse-or-same-window` doesn't use
`display-buffer--reuse-or-same-window-action`, tho.


        Stefan

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > I don't understand why `display-buffer-reuse-or-same-window` doesn't use
 > `display-buffer--reuse-or-same-window-action`, tho.

A technical issue?  'display-buffer-reuse-or-same-window' has to pass
the ALIST argument on to 'display-buffer-reuse-window', for example.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Lars Ingebrigtsen]

In any case, it sounds like we do want to revert Sam's patch, I guess?
I think we should do that sooner rather than later, since people have
started fixing up technical problems with it (see 1d2d7ee87 and some bug
reports), which will make reverting it more work.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > In any case, it sounds like we do want to revert Sam's patch, I guess?
 > I think we should do that sooner rather than later, since people have
 > started fixing up technical problems with it (see 1d2d7ee87 and some bug
 > reports), which will make reverting it more work.

Probably, given the bugs it apparently has.  I'm currently avoiding
master to avoid conflicts with Bug#52297.

Yet my primary concern is how to deal with the problematic change on
the release branch.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

> Yet my primary concern is how to deal with the problematic change on
> the release branch.

Doesn't your proposed change fix that?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 >> Yet my primary concern is how to deal with the problematic change on
 >> the release branch.
 >
 > Doesn't your proposed change fix that?

My proposed change is a compromise that (1) would need quite some
testing and (2) apparently does not satisfy Sam anyway.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

>> ... we need a general new display
>> buffer action that preferably (1) reuses a window already showing the
>> buffer (2) uses the same window and only then (3) uses another window.
>> Maybe Juri has an idea.

Some commands pop their windows in unexpected places anyway,
so often have to resort to typing a same-window-prefix key
before running such commands :-)

> The attached diff should illustrate what I mean here.
> [...]
> +(defvar display-buffer--reuse-or-same-window-action
> +  '((display-buffer-reuse-window
> +     display-buffer-same-window)
> +    (inhibit-same-window . nil))

Shouldn't the first item be display-buffer--maybe-same-window
before display-buffer-reuse-window?  This is what is used by default
in display-buffer-fallback-action, but it has a comment with a question,
so probably it's really redundant:

  (defconst display-buffer-fallback-action
    '((display-buffer--maybe-same-window  ;FIXME: why isn't this redundant?
       display-buffer-reuse-window

> +(defun pop-to-buffer-reuse-or-same-window (buffer &optional norecord)
> +  "Select specified BUFFER in a window showing it or the same one.
> +This is like `pop-to-buffer-same-window', but tries first to
> +reuse a window already showing BUFFER and only if no such window
> +exists behaves like `pop-to-buffer-same-window'."
> +  (pop-to-buffer buffer display-buffer--reuse-or-same-window-action norecord))

Grep counted 100 occurrences of pop-to-buffer-same-window in the source tree,
but I guess only shell commands could use pop-to-buffer-reuse-or-same-window?

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > Some commands pop their windows in unexpected places anyway,
 > so often have to resort to typing a same-window-prefix key
 > before running such commands :-)
 >
 >> The attached diff should illustrate what I mean here.
 >> [...]
 >> +(defvar display-buffer--reuse-or-same-window-action
 >> +  '((display-buffer-reuse-window
 >> +     display-buffer-same-window)
 >> +    (inhibit-same-window . nil))
 >
 > Shouldn't the first item be display-buffer--maybe-same-window
 > before display-buffer-reuse-window?  This is what is used by default
 > in display-buffer-fallback-action, but it has a comment with a question,
 > so probably it's really redundant:
 >
 >    (defconst display-buffer-fallback-action
 >      '((display-buffer--maybe-same-window  ;FIXME: why isn't this redundant?
 >         display-buffer-reuse-window

'display-buffer--maybe-same-window' is here for compatibility reasons
with the 'same-window-buffer-names/-regexps' options and can be used
only in an "ored" fashion as in 'display-buffer--maybe-at-bottom' or
'display-buffer-fallback-action'.  Otherwise, it would defeat
'display-buffer-alist' because it uses the same window only if that has
been advocated by 'same-window-p' which ignores 'display-buffer-alist'.
It's not redundant as long as we want to maintain compatibility for
those options.

 > Grep counted 100 occurrences of pop-to-buffer-same-window in the source tree,
 > but I guess only shell commands could use pop-to-buffer-reuse-or-same-window?

It can be used as a substitute for 'pop-to-buffer' wherever we decide
that consistency should prevail over long-standing behavior.  I have
been missing the present and the preceding thread but I think we should
revert the behavior for Emacs 28 and use 'pop-to-buffer' there.  Sam's
protest in this respect was justified - we cannot simply tell people
that they should customize 'display-buffer-alist' in order to get back
some long-standing behavior.  That's not our style.

Now 'pop-to-buffer-reuse-or-same-window' is still no 100% substitute for
'pop-to-buffer' but we can try it because it handles Sam's main argument
against 'pop-to-buffer-same-window' - that the same buffer appears twice
on the same frame.  Whether that suffices to fix all regressions in this
context should probably be answered in Emacs 29.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

> * martin rudalics <ehqnyvpf@tzk.ng> [2021-12-30 18:14:14 +0100]:
>
> Now 'pop-to-buffer-reuse-or-same-window' is still no 100% substitute for
> 'pop-to-buffer' but we can try it because it handles Sam's main argument
> against 'pop-to-buffer-same-window' - that the same buffer appears twice
> on the same frame.

Please note that this is just the most obvious situation when the "same
window" behavior is undesirable.
Personally, I would rather avoid it altogether.
This is why my preferred approach would be to

* use plain `pop-to-buffer' _everywhere_
* deprecate all `pop-to-buffer-*' functions
* modify the default value of `display-buffer-alist' to preserve the
  current behavior

One clear benefit of this approach would be giving users a sophisticated
example of `display-buffer-alist' to start their customizations from.

Thank you.

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://mideasttruth.com https://thereligionofpeace.com https://www.memritv.org
Brainwashing leads to brain drain.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > * modify the default value of `display-buffer-alist' to preserve the
 >    current behavior

'display-buffer-alist' is reserved for users.  Its default value is and
has to remain nil.

 > One clear benefit of this approach would be giving users a sophisticated
 > example of `display-buffer-alist' to start their customizations from.

If we expected users to do that, we've lost them already.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

> * martin rudalics <ehqnyvpf@tzk.ng> [2021-12-30 11:25:31 +0100]:
>
>> ... we need a general new display
>> buffer action that preferably (1) reuses a window already showing the
>> buffer (2) uses the same window and only then (3) uses another window.
>> Maybe Juri has an idea.
>
> The attached diff should illustrate what I mean here.

I cannot imagine a situation where I would want to use the same window
(i.e., show the shell buffer in the window from which I called shell).

IIUC, your patch will use the current window in some circumstances,
and, since you want to use `display-buffer-reuseor-same-window' in
`shell'-like functions, I will not be able to avoid that.

I would much prefer that we use `pop-to-buffer' or `display-buffer'
everywhere (deprecating all those `pop-to-buffer-*' functions)
and let the _users_ decide how to display _all_ buffers using the standard
`display-buffer-alist' functionality.


-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://camera.org https://mideasttruth.com https://honestreporting.com
To be popular with ladies one has to be smart, handsome & rich. Or to be a cat.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > I cannot imagine a situation where I would want to use the same window
 > (i.e., show the shell buffer in the window from which I called shell).

Neither can I.  But in your OP your only concern was

   When I already have a window with shell, this patch creates a second
   such window.

That's what my patch tried to fix.

 > IIUC, your patch will use the current window in some circumstances,

It will use it unless the shell buffer is already shown in a window on
the same frame.

 > and, since you want to use `display-buffer-reuseor-same-window' in
 > `shell'-like functions, I will not be able to avoid that.

That's an exaggeration.  You can always work around this problem via
'display-buffer-alist'.

 > I would much prefer that we use `pop-to-buffer' or `display-buffer'
 > everywhere (deprecating all those `pop-to-buffer-*' functions)

I might agree.  But we cannot simply convert everything that used
'switch-to-buffer' once (or still uses it) to use 'pop-to-buffer'
instead.  The greatest problem the 'display-buffer-alist' functionality
had to deal with was to leave the default behavior unchanged while
adding enough facilities so users could change the behavior to their
like.  That's also why I'd prefer to revert the original change and
think about a better solution we can all live with for Emacs 29.

 > and let the _users_ decide how to display _all_ buffers using the standard
 > `display-buffer-alist' functionality.

Deciding is not sufficient.  Users have to customize it and you should
have now noticed by yourself that dealing with 'display-buffer-alist' is
not entirely trivial.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

> * martin rudalics <ehqnyvpf@tzk.ng> [2021-12-31 19:42:28 +0100]:
>
>> and, since you want to use `display-buffer-reuse-or-same-window' in
>> `shell'-like functions, I will not be able to avoid that.
>
> That's an exaggeration.  You can always work around this problem via
> 'display-buffer-alist'.

Are you saying that by manipulating `display-buffer-alist' I can stop
`pop-to-buffer' from using the same window?!
How?

>> I would much prefer that we use `pop-to-buffer' or `display-buffer'
>> everywhere (deprecating all those `pop-to-buffer-*' functions)
>
> I might agree.  But we cannot simply convert everything that used
> 'switch-to-buffer' once (or still uses it) to use 'pop-to-buffer'
> instead.

That's okay, we can leave `switch-to-buffer' alone.


> The greatest problem the 'display-buffer-alist' functionality had to
> deal with was to leave the default behavior unchanged while adding
> enough facilities so users could change the behavior to their like.

IIUC, using `pop-to-buffer' for *shell* and adding something like

(("*shell*" (display-buffer-reuse-window)))

to `display-buffer-alist' is equivalent to using
`pop-to-buffer-same-window' on *shell*.

>> and let the _users_ decide how to display _all_ buffers using the standard
>> `display-buffer-alist' functionality.
>
> Deciding is not sufficient.  Users have to customize it and you should
> have now noticed by yourself that dealing with 'display-buffer-alist' is
> not entirely trivial.

Understatement of the year.

This is why I want us to use it instead of `pop-to-buffer-*' - to give
users a good starting example to modify.

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://mideasttruth.com https://jihadwatch.org
Your mouse has moved - WinNT has to be restarted for this to take effect.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 >> That's an exaggeration.  You can always work around this problem via
 >> 'display-buffer-alist'.
 >
 > Are you saying that by manipulating `display-buffer-alist' I can stop
 > `pop-to-buffer' from using the same window?!

Yes.

 > How?

(add-to-list 'display-buffer-alist
              '("\\*shell\\*" nil (inhibit-same-window . t)))

(pop-to-buffer-same-window "*shell*")

Have you ever read sections 29.15.5 and 29.15.6 of the Elisp manual?
These contain numerous examples of how to add the 'inhibit-same-window'
action alist entry to do precisely that.

 >> But we cannot simply convert everything that used
 >> 'switch-to-buffer' once (or still uses it) to use 'pop-to-buffer'
 >> instead.
 >
 > That's okay, we can leave `switch-to-buffer' alone.

Then we have to leave 'pop-to-buffer-same-window' alone too.
'pop-to-buffer-same-window' is the customizable version of
'switch-to-buffer'.

 >> The greatest problem the 'display-buffer-alist' functionality had to
 >> deal with was to leave the default behavior unchanged while adding
 >> enough facilities so users could change the behavior to their like.
 >
 > IIUC, using `pop-to-buffer' for *shell* and adding something like
 >
 > (("*shell*" (display-buffer-reuse-window)))
 >
 > to `display-buffer-alist' is equivalent to using
 > `pop-to-buffer-same-window' on *shell*.

There is no such equivalence.  Think of 'pop-to-buffer' as a proposal.
It's what the writer of an application considers the best choice for
fulfilling its customers' needs.  'display-buffer-alist' is the user's
answer for dealing with that proposal.

 >> Deciding is not sufficient.  Users have to customize it and you should
 >> have now noticed by yourself that dealing with 'display-buffer-alist' is
 >> not entirely trivial.
 >
 > Understatement of the year.

Every second specification I write fails initially.

 > This is why I want us to use it instead of `pop-to-buffer-*' - to give
 > users a good starting example to modify.

We can add that to the manual - as soon as we think it's good enough.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

> * martin rudalics <ehqnyvpf@tzk.ng> [2021-12-31 20:40:20 +0100]:
>
>>> That's an exaggeration.  You can always work around this problem via
>>> 'display-buffer-alist'.
>>
>> Are you saying that by manipulating `display-buffer-alist' I can stop
>> `pop-to-buffer' from using the same window?!
>
> Yes.

Of course.

I meant `pop-to-buffer-same-window', not `pop-to-buffer'.

Sorry.

>> How?
>
> (add-to-list 'display-buffer-alist
>              '("\\*shell\\*" nil (inhibit-same-window . t)))
>
> (pop-to-buffer-same-window "*shell*")

Huh, apparently, you have mastered ESP ;-)

> Have you ever read sections 29.15.5 and 29.15.6 of the Elisp manual?

Presumably you mean "29.13.5 Precedence of Action Functions" and
"29.13.6 The Zen of Buffer Display".
I think I did, long ago, and I will re-read them now.

> These contain numerous examples of how to add the 'inhibit-same-window'
> action alist entry to do precisely that.
>
>>> But we cannot simply convert everything that used
>>> 'switch-to-buffer' once (or still uses it) to use 'pop-to-buffer'
>>> instead.
>>
>> That's okay, we can leave `switch-to-buffer' alone.
>
> Then we have to leave 'pop-to-buffer-same-window' alone too.
> 'pop-to-buffer-same-window' is the customizable version of
> 'switch-to-buffer'.

I did not realize that, thank you for pointing this out.


-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://jihadwatch.org https://ij.org/ https://ffii.org http://think-israel.org
You won't get smarter by calling me a fool.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Dmitry Gutov]

On 30.12.2021 11:47, martin rudalics wrote:
> Sorry again, I slowly start understanding now.  I now think that the
> original patch should be reverted or that we need a general new display
> buffer action that preferably (1) reuses a window already showing the
> buffer (2) uses the same window and only then (3) uses another window.
> Maybe Juri has an idea.

That sounds like my suggestion in 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52467#37, see Lars' response.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > That sounds like my suggestion in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52467#37,

Right (I haven't yet read the proposals in this thread).

 > see Lars' response.

Why is it unpredictable?

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Lars Ingebrigtsen]

martin rudalics <rudalics@gmx.at> writes:

>> That sounds like my suggestion in
>> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=52467#37,
>
> Right (I haven't yet read the proposals in this thread).
>
>> see Lars' response.
>
> Why is it unpredictable?

Well, perhaps "unpredictable" is the wrong word.  But if we want to make
these commands behave the same (and I think we do), then some of them
have to change, and the change committed seems like the most consistent
and useful one.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > Well, perhaps "unpredictable" is the wrong word.  But if we want to make
 > these commands behave the same (and I think we do), then some of them
 > have to change, and the change committed seems like the most consistent
 > and useful one.

'pop-to-buffer-same-window' was conceived as a stand-in for
'switch-to-buffer' so that people could customize it via
'display-buffer-alist'.  Using it as stand-in for 'pop-to-buffer' is
tantamount to using 'switch-to-buffer' as stand-in for 'pop-to-buffer'.

I'm not sure whether we should want to "make these commands behave the
same".  But if we do, we have to provide some _simple_ knob to let users
get the old behavior without having to think about how to match names of
buffers they possibly never cared about with the help of a regular
expression.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Madhu]

* martin rudalics <8dfc6f22-d331-e7c1-b536-2d374197528f@gmx.at> :
Wrote on Fri, 31 Dec 2021 19:41:09 +0100:

>> Well, perhaps "unpredictable" is the wrong word.  But if we want to make
>> these commands behave the same (and I think we do), then some of them
>> have to change, and the change committed seems like the most consistent
>> and useful one.
>
> 'pop-to-buffer-same-window' was conceived as a stand-in for
> 'switch-to-buffer' so that people could customize it via
> 'display-buffer-alist'.  Using it as stand-in for 'pop-to-buffer' is
> tantamount to using 'switch-to-buffer' as stand-in for 'pop-to-buffer'.

I haven't tried your new patch yet but I found pop-to-buffer-same-window
+ switch-to-buffer-obey-display-actions inadequate for the sort of
behaviour required here, I had something like this (with the variable
set to t)

(defadvice pop-to-buffer-same-window
    (around reuse-existing-frame-advise
	    (buffer-or-name &optional norecord) activate)
  (if current-prefix-arg
      ad-do-it
  (pop-to-buffer buffer-or-name
		 '((display-buffer-reuse-window
		    display-buffer-same-window)
		   . ((reusable-frames . 0)
		      (inhibit-same-window . nil)))
		 norecord)))

This would *for the most part* obey the customizations via pop-up-frames
and pop-up-windows or via display-buffer-alist (as the manual now
suggests. the latter is not easy to customize 'graphic-only, and fails
in many ways with packages like consult and transient)


> I'm not sure whether we should want to "make these commands behave the
> same".  But if we do, we have to provide some _simple_ knob to let users
> get the old behavior without having to think about how to match names of
> buffers they possibly never cared about with the help of a regular
> expression.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > I haven't tried your new patch yet

... don't ...

 > but I found pop-to-buffer-same-window
 > + switch-to-buffer-obey-display-actions

... these are two different pairs of shoes:
'switch-to-buffer-obey-display-actions' is meant for customizing the
behavior of 'switch-to-buffer' - 'pop-to-buffer-same-window' should not
be affected by it ...

 > inadequate for the sort of
 > behaviour required here, I had something like this (with the variable
 > set to t)
 >
 > (defadvice pop-to-buffer-same-window
 >      (around reuse-existing-frame-advise
 > 	    (buffer-or-name &optional norecord) activate)
 >    (if current-prefix-arg
 >        ad-do-it
 >    (pop-to-buffer buffer-or-name
 > 		 '((display-buffer-reuse-window
 > 		    display-buffer-same-window)
 > 		   . ((reusable-frames . 0)
 > 		      (inhibit-same-window . nil)))
 > 		 norecord)))
 >
 > This would *for the most part* obey the customizations via pop-up-frames
 > and pop-up-windows or via display-buffer-alist

... IIUC this is more or less what I suggested just that I left
'reusable-frames' alone ...

 > (as the manual now
 > suggests. the latter is not easy to customize 'graphic-only, and fails
 > in many ways with packages like consult and transient)

Do you mean that we cannot separate the graphic and non-graphic cases
via 'display-buffer-alist'?  Too bad.

One problem is that 'display-buffer-pop-up-frame' has to get through as
'display-buffer-fallback-action' when everything else failed before -
even on a TTY.  So a new action alist entry like 'inhibit-pop-up-frame'
is needed - if t never pop up a new frame and if 'graphic-only pop up a
new frame on a graphic terminal only.  And ignore this entry in the
fallback case.  WDYT?

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

> I'm not sure whether we should want to "make these commands behave the
> same".  But if we do, we have to provide some _simple_ knob to let users
> get the old behavior without having to think about how to match names of
> buffers they possibly never cared about with the help of a regular
> expression.

This is how we can make customization simpler, helping to get the old behavior
back for a set of commands.  Currently there are 11 comint commands that
have invocations like:

  (pop-to-buffer "*inferior-lisp*" display-comint-buffer-action)
  (pop-to-buffer "*scheme*" display-comint-buffer-action)
  ...

To change their behavior at once the user needs to construct
a complex regexp that matches all used comint buffer names,

  "\\*inferior-lisp\\*\\|\\*scheme\\*\\|..."

This is completely unmanageable.  Instead of this, all calls could add
a special tag:

  (pop-to-buffer "*inferior-lisp*" '(nil (comint . t)))

Then a condition predicate could check for this tag:

  (defun display-buffer-comint (buffer-name action)
    (assq 'comint action))

and the user can customize all comint commands with:

  (add-to-list 'display-buffer-alist
               '(display-buffer-comint
                 (display-buffer-same-window
                  . ((reusable-frames . 0)
                     (inhibit-same-window . nil)))))

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > This is how we can make customization simpler, helping to get the old behavior
 > back for a set of commands.  Currently there are 11 comint commands that
 > have invocations like:
 >
 >    (pop-to-buffer "*inferior-lisp*" display-comint-buffer-action)
 >    (pop-to-buffer "*scheme*" display-comint-buffer-action)
 >    ...

IIUC 'shell' still has (pop-to-buffer buffer), 'sh-show-shell' has
(pop-to-buffer (process-buffer (sh-shell-process t))).  Shouldn't these
become part of the same scheme?

 > To change their behavior at once the user needs to construct
 > a complex regexp that matches all used comint buffer names,
 >
 >    "\\*inferior-lisp\\*\\|\\*scheme\\*\\|..."
 >
 > This is completely unmanageable.

*eshell*<1>, *eshell*<2> ... come to mind.

 > Instead of this, all calls could add
 > a special tag:
 >
 >    (pop-to-buffer "*inferior-lisp*" '(nil (comint . t)))
 >
 > Then a condition predicate could check for this tag:
 >
 >    (defun display-buffer-comint (buffer-name action)
 >      (assq 'comint action))
 >
 > and the user can customize all comint commands with:
 >
 >    (add-to-list 'display-buffer-alist
 >                 '(display-buffer-comint
 >                   (display-buffer-same-window
 >                    . ((reusable-frames . 0)
 >                       (inhibit-same-window . nil)))))

If we do that we should first make a list of possible tags like 'occur'
or 'info' and check whether we should provide similar functions for them
too.

Yet the problem remains how to easily get back the old behavior for
individual 'display-buffer'-based calls we decided to change.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Dmitry Gutov]

On 02.01.2022 21:40, martin rudalics wrote:
>  > Instead of this, all calls could add
>  > a special tag:
>  >
>  >    (pop-to-buffer "*inferior-lisp*" '(nil (comint . t)))
>  >
>  > Then a condition predicate could check for this tag:
>  >
>  >    (defun display-buffer-comint (buffer-name action)
>  >      (assq 'comint action))
>  >
>  > and the user can customize all comint commands with:
>  >
>  >    (add-to-list 'display-buffer-alist
>  >                 '(display-buffer-comint
>  >                   (display-buffer-same-window
>  >                    . ((reusable-frames . 0)
>  >                       (inhibit-same-window . nil)))))
> 
> If we do that we should first make a list of possible tags like 'occur'
> or 'info' and check whether we should provide similar functions for them
> too.

Perhaps the customized value structure should simply refer to major mode(s)?

Then there won't be a need to create a predefined list with mappings.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

>>  > Then a condition predicate could check for this tag:
>>  >
>>  >    (defun display-buffer-comint (buffer-name action)
>>  >      (assq 'comint action))
>>  >
>> If we do that we should first make a list of possible tags like 'occur'
>> or 'info' and check whether we should provide similar functions for them
>> too.
>
> Perhaps the customized value structure should simply refer to major mode(s)?
>
> Then there won't be a need to create a predefined list with mappings.

Indeed, this is what I use for the outbound buffer:

#+begin_src emacs-lisp
(defun display-buffer-from-grep-p (_buffer-name _action)
  (with-current-buffer (window-buffer)
    (or (derived-mode-p 'compilation-mode)
        (derived-mode-p 'xref--xref-buffer-mode))))

(add-to-list 'display-buffer-alist
             '(display-buffer-from-grep-p
               (display-buffer-same-window
                (inhibit-same-window . nil))))
#+end_src

So it's possible to do the same for the inbound buffer:

#+begin_src emacs-lisp
(defun display-buffer-to-comint-p (buffer-name _action)
  (with-current-buffer (get-buffer buffer-name)
    (derived-mode-p 'comint-mode)))

(add-to-list 'display-buffer-alist
             '(display-buffer-to-comint-p
               (display-buffer-same-window
                (inhibit-same-window . nil))))
#+end_src

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 >> Perhaps the customized value structure should simply refer to major mode(s)?
 >>
 >> Then there won't be a need to create a predefined list with mappings.
 >
 > Indeed, this is what I use for the outbound buffer:

It's not sufficiently general.  For example, 'shell' first pops up the
buffer and then sets its major mode with an explanation that goes as

   ;; The buffer's window must be correctly set when we call comint
   ;; (so that comint sets the COLUMNS env var properly).
   (pop-to-buffer buffer)

Using the tools we have on board we could do something in the spirit of

(defun display-buffer-match-comint (_buffer action)
   (cadr (assq 'comint (cdr action))))

(customize-set-variable
  'display-buffer-alist
  '((display-buffer-match-comint
     display-buffer-same-window (nil))))

(pop-to-buffer "*shell*" '(nil . ((comint t))))

which means that customizing 'display-buffer-alist' with that value on
Emacs 28 will report a mismatch but that's the only mishap that should
happen.

We also could have 'display-buffer-assq-regexp' do

	(when (or (and (stringp key)
		       (string-match-p key buffer-name))
		  (and (functionp key)
		       (funcall key buffer-name action))
                   (and (symbolp key)
                        (cadr (assq key (cdr action)))))

so we could do away with 'display-buffer-match-comint' but then we would
have to change the custom type of 'display-buffer-alist' - no great deal
either.  WDYT?

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Stefan Monnier]

> It's not sufficiently general.  For example, 'shell' first pops up the
> buffer and then sets its major mode with an explanation that goes as
>
>   ;; The buffer's window must be correctly set when we call comint
>   ;; (so that comint sets the COLUMNS env var properly).
>   (pop-to-buffer buffer)

BTW, this is a problem we should fix more thoroughly.

I *think* the right way to fix it is to first set the buffer in
comint-mode, then display the buffer (when needed), then launch
the process.


        Stefan

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > BTW, this is a problem we should fix more thoroughly.
 >
 > I *think* the right way to fix it is to first set the buffer in
 > comint-mode, then display the buffer (when needed), then launch
 > the process.

So why did you change it back then?

In either case we have a sufficient number of 'pop-to-buffer' and
'display-buffer' variants in our sources that call 'generate-new-buffer'
or 'get-buffer-create' in their first argument.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Stefan Monnier]

martin rudalics [2022-01-04 11:25:16] wrote:
>> BTW, this is a problem we should fix more thoroughly.
>> I *think* the right way to fix it is to first set the buffer in
>> comint-mode, then display the buffer (when needed), then launch
>> the process.
> So why did you change it back then?

I can't remember but I think it used to launch the process
before the buffer was displayed.


        Stefan

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

> Using the tools we have on board we could do something in the spirit of
>
> (defun display-buffer-match-comint (_buffer action)
>   (cadr (assq 'comint (cdr action))))
>
> (customize-set-variable
>  'display-buffer-alist
>  '((display-buffer-match-comint
>     display-buffer-same-window (nil))))
>
> (pop-to-buffer "*shell*" '(nil . ((comint t))))

Or to add more semantics:

  (pop-to-buffer "*shell*" '(nil . ((caller . comint))))

> which means that customizing 'display-buffer-alist' with that value on
> Emacs 28 will report a mismatch but that's the only mishap that should
> happen.
>
> We also could have 'display-buffer-assq-regexp' do
>
> 	(when (or (and (stringp key)
> 		       (string-match-p key buffer-name))
> 		  (and (functionp key)
> 		       (funcall key buffer-name action))
>                   (and (symbolp key)
>                        (cadr (assq key (cdr action)))))

That will shorten the above example to

  (customize-set-variable
   'display-buffer-alist
   '((comint
      display-buffer-same-window (nil))))

?

> so we could do away with 'display-buffer-match-comint' but then we would
> have to change the custom type of 'display-buffer-alist' - no great deal
> either.  WDYT?

The obvious downside is the need to tag hundreds of existing calls of
display-buffer, pop-to-buffer, etc.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > Or to add more semantics:
 >
 >    (pop-to-buffer "*shell*" '(nil . ((caller . comint))))

IIRC I once used the term 'label' for it.  It vanished soon.

 >> which means that customizing 'display-buffer-alist' with that value on
 >> Emacs 28 will report a mismatch but that's the only mishap that should
 >> happen.
 >>
 >> We also could have 'display-buffer-assq-regexp' do
 >>
 >> 	(when (or (and (stringp key)
 >> 		       (string-match-p key buffer-name))
 >> 		  (and (functionp key)
 >> 		       (funcall key buffer-name action))
 >>                    (and (symbolp key)
 >>                         (cadr (assq key (cdr action)))))
 >
 > That will shorten the above example to
 >
 >    (customize-set-variable
 >     'display-buffer-alist
 >     '((comint
 >        display-buffer-same-window (nil))))
 >
 > ?

Yes.  Although we should be careful with the symbol.  'shell' would be
interpreted as a function to call via (funcall key buffer-name action).
We could wrap that in a 'condition-case' but what if the function has
some nasty side-effect before it errs out?  So maybe *comint* or
something similar ...

 >> so we could do away with 'display-buffer-match-comint' but then we would
 >> have to change the custom type of 'display-buffer-alist' - no great deal
 >> either.  WDYT?
 >
 > The obvious downside is the need to tag hundreds of existing calls of
 > display-buffer, pop-to-buffer, etc.

Just the ones we care about.  With our usual speed of progress in this
area I won't live long enough to see that happen.  Think of pearls like

	  (select-window			; to return to
	   (prog1 (selected-window)	; WoMan window
	     (select-window (display-buffer (current-buffer)))

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

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

 > IIRC I once used the term 'label' for it.  It vanished soon.

After trying to implement this idea I immediately started to dislike it
for technical reasons ('pop-to-buffer-same-window' never had an ACTION
argument so I had to invent some clumsy extra argument to make things
work).  I attach the diffs in case we still want to implement something
the like.

Otherwise, I think that harm has been done - 'pop-to-buffer-same-window'
was invented as stand-in for 'switch-to-buffer' and should never have
replaced 'pop-to-buffer' - but I missed that back then so I'm not in a
good position to ask for reverting that change now.

Sam's patch, although it does not restore the previous behavior, still
looks like the least obtrusive solution in this regard.  It's partially
a misnomer - to my knowledge at least 'eshell' has no connection to
"comint" - so we apparently cannot put 'display-comint-buffer-action'
into comint.el where it would belong.  Yet I'd tend to keep it, after
applying Morgan's fix from Bug#52878, obviously.

martin

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: display-buffer-label.diff --]
[-- Type: text/x-patch; name="display-buffer-label.diff", Size: 13557 bytes --]

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 6d156ef861..dd09ddf931 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -2969,7 +2969,7 @@ Switching Buffers
 @code{display-buffer} will affect it as well.  @xref{Choosing Window},
 for the documentation of @code{display-buffer}.

-@deffn Command pop-to-buffer buffer-or-name &optional action norecord
+@deffn Command pop-to-buffer buffer-or-name &optional action norecord label
 This function makes @var{buffer-or-name} the current buffer and
 displays it in some window, preferably not the window currently
 selected.  It then selects the displaying window.  If that window is
@@ -2990,6 +2990,9 @@ Switching Buffers
 window other than the selected one---even if the buffer is already
 displayed in the selected window.

+If @var{label} is non-@code{nil}, it should specify the name of a buffer
+display label and is passed on unaltered to @code{display-buffer}.
+
 Like @code{switch-to-buffer}, this function updates the buffer list
 unless @var{norecord} is non-@code{nil}.
 @end deffn
@@ -3065,7 +3068,7 @@ Choosing Window
 of them manages to display the buffer and returns a non-@code{nil}
 value.

-@deffn Command display-buffer buffer-or-name &optional action frame
+@deffn Command display-buffer buffer-or-name &optional action frame label
 This command makes @var{buffer-or-name} appear in some window, without
 selecting the window or making the buffer current.  The argument
 @var{buffer-or-name} must be a buffer or the name of an existing
@@ -3139,6 +3142,11 @@ Choosing Window
 . @var{frame})}} to the action alist of @var{action} (@pxref{Buffer
 Display Action Alists}).  The @var{frame} argument is provided for
 compatibility reasons, Lisp programs should not use it.
+
+The optional argument @var{label}, if non-@code{nil}, should specify the
+name of a buffer display label (@pxref{Buffer Display Action Alists})
+and is prepended as a cons of that name and the value @code{t} to the
+action alist specified of @var{action}.
 @end deffn

 @defvar display-buffer-overriding-action
@@ -3150,11 +3158,15 @@ Choosing Window
 @defopt display-buffer-alist
 The value of this option is an alist mapping conditions to display
 actions.  Each condition may be either a regular expression matching a
-buffer name or a function that takes two arguments: a buffer name and
-the @var{action} argument passed to @code{display-buffer}.  If either
-the name of the buffer passed to @code{display-buffer} matches a
-regular expression in this alist, or the function specified by a
-condition returns non-@code{nil}, then @code{display-buffer} uses the
+buffer name; a function that takes two arguments: a buffer name and the
+@var{action} argument passed to @code{display-buffer}; or a symbol
+naming a buffer display label (@pxref{Buffer Display Action Alists}).
+
+If either the name of the buffer passed to @code{display-buffer} matches
+a regular expression in this alist, the function specified by a
+condition returns non-@code{nil}, or the symbol specified by a condition
+has a non-nil association found via @code{eq} in the action alist passed
+to @code{display-buffer}, then @code{display-buffer} uses the
 corresponding display action to display the buffer.
 @end defopt

@@ -3770,7 +3782,7 @@ Buffer Display Action Alists
 buffer and never was used to show another buffer until it was reused
 by the current invocation of @code{display-buffer}.

-If no @code{window-height}, @code{window-width} or @code{window-size}
+  If no @code{window-height}, @code{window-width} or @code{window-size}
 entry was specified, the window may still be resized automatically when
 the buffer is temporary and @code{temp-buffer-resize-mode} has been
 enabled, @ref{Temporary Displays}.  In that case, the @sc{cdr} of a
@@ -3779,6 +3791,45 @@ Buffer Display Action Alists
 @code{temp-buffer-resize-mode} for specific buffers or invocations of
 @code{display-buffer}.

+@cindex buffer display label
+  In addition, action alists may contain @dfn{buffer display label}s -
+entries handled and passed on by @code{display-buffer} as described
+above.  The @sc{car} of a label - a symbol - is the label's name and its
+@sc{cdr} is its value.  If that value is non-@code{nil}, it will advise
+@code{display-buffer} to handle this call specially, provided it finds a
+matching entry in @code{display-buffer-alist}.
+
+  If, for example, a call to @code{pop-to-buffer} is coded as
+
+@example
+(pop-to-buffer "foo" '(nil . ((*foo* . t))))
+@end example
+
+@noindent
+specifying a label named @code{*foo*} whose value is @code{t} and
+@code{display-buffer-alist} was customized as
+
+@example
+@group
+(customize-set-variable
+ 'display-buffer-alist
+ '((*foo* display-buffer-same-window (nil))))
+@end group
+@end example
+
+@noindent
+then @code{display-buffer} will try to pop to @code{foo} in the selected
+window and not in a new window as originally proposed by
+@code{pop-to-buffer}.
+
+  Lisp programmers should be careful when choosing the name for a buffer
+display label to avoid that it matches any of the other entries
+mentioned in this section (including entries that may be added to it in
+the future) or the name of a function that could be used as condition in
+@code{display-buffer-alist}.  In our example, the use of asterisks in
+@code{*foo*} is an attempt to avoid such confusions.
+
+
 @node Choosing Window Options
 @subsection Additional Options for Displaying Buffers

diff --git a/lisp/window.el b/lisp/window.el
index 57861b091f..7f9983b998 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8169,9 +8169,10 @@ display-buffer-alist
 If non-nil, this is an alist of elements (CONDITION . ACTION),
 where:

- CONDITION is either a regexp matching buffer names, or a
-  function that takes two arguments - a buffer name and the
-  ACTION argument of `display-buffer' - and returns a boolean.
+ CONDITION is either a regexp matching buffer names; a function
+  that takes two arguments - a buffer name and the ACTION
+  argument of `display-buffer' - and returns a boolean; or a
+  symbol that is interpreted as a label.

  ACTION is a cons cell (FUNCTIONS . ALIST), where FUNCTIONS is an
   action function or a list of action functions and ALIST is an
@@ -8180,13 +8181,16 @@ display-buffer-alist
   ALIST.  See `display-buffer' for details.

 `display-buffer' scans this alist until it either finds a
-matching regular expression or the function specified by a
-condition returns non-nil.  In any of these cases, it adds the
-associated action to the list of actions it will try."
+matching regular expression, the function specified by a
+condition returns non-nil, or the label specified by a condition
+has an non-nil association found via `eq' in the action alist
+passed to this `display-buffer' call.  In any of these cases, it
+adds the associated action to the list of actions it will try."
   :type `(alist :key-type
 		(choice :tag "Condition"
 			regexp
-			(function :tag "Matcher function"))
+			(function :tag "Matcher function")
+                        (symbol :tag "Label"))
 		:value-type ,display-buffer--action-custom-type)
   :risky t
   :version "24.1"
@@ -8238,15 +8242,21 @@ display-buffer-assq-regexp
 is a string that matches BUFFER-NAME, as reported by
 `string-match-p'; or if the key is a function that returns
 non-nil when called with three arguments: the ALIST key,
-BUFFER-NAME and ACTION.  ACTION should have the form of the
-action argument passed to `display-buffer'."
+BUFFER-NAME and ACTION; or if the key is a symbol for which
+'assq' finds an non-nil association in the action alist specified
+by ACTION.
+
+ACTION should have the form of the action argument passed to
+`display-buffer'."
   (catch 'match
     (dolist (entry alist)
       (let ((key (car entry)))
 	(when (or (and (stringp key)
 		       (string-match-p key buffer-name))
 		  (and (functionp key)
-		       (funcall key buffer-name action)))
+		       (funcall key buffer-name action))
+                  (and (symbolp key)
+                       (cdr (assq key (cdr action)))))
 	  (throw 'match (cdr entry)))))))

 (defvar display-buffer--same-window-action
@@ -8266,7 +8276,7 @@ display-buffer--other-frame-action
 fails, call `display-buffer-pop-up-frame'.")
 (put 'display-buffer--other-frame-action 'risky-local-variable t)

-(defun display-buffer (buffer-or-name &optional action frame)
+(defun display-buffer (buffer-or-name &optional action frame label)
   "Display BUFFER-OR-NAME in some window, without selecting it.
 To change which window is used, set `display-buffer-alist'
 to an expression containing one of these \"action\" functions:
@@ -8390,6 +8400,13 @@ display-buffer
 `preserve-size' are applied only when the window used for
 displaying the buffer never showed another buffer before.

+An action alist entry may also specify a buffer display label - a
+cons cell whose car specifies the name of the label and whose cdr
+its value.  If `display-buffer-alist' contains an member whose
+car names that label and the value of that label is non-nil, the
+cdr of the `display-buffer-alist' specifies the action to
+perform.
+
 The ACTION argument can also have a non-nil and non-list value.
 This means to display the buffer in a window other than the
 selected one, even if it is already displayed in the selected
@@ -8398,12 +8415,14 @@ display-buffer

 The optional third argument FRAME, if non-nil, acts like a
 \(reusable-frames . FRAME) entry appended to the action alist
-specified by the ACTION argument."
+specified by the ACTION argument.
+
+The optional argument LABEL, if non-nil, specifies the name of a
+buffer display label and is prepended as a cons of that name and
+the value t to the action alist."
   (interactive (list (read-buffer "Display buffer: " (other-buffer))
 		     (if current-prefix-arg t)))
-  (let ((buffer (if (bufferp buffer-or-name)
-		    buffer-or-name
-		  (get-buffer buffer-or-name)))
+  (let ((buffer (get-buffer buffer-or-name))
 	;; Make sure that when we split windows the old window keeps
 	;; point, bug#14829.
 	(split-window-keep-point t)
@@ -8416,7 +8435,10 @@ display-buffer
       ;; Otherwise, use the defined actions.
       (let* ((user-action
 	      (display-buffer-assq-regexp
-	       (buffer-name buffer) display-buffer-alist action))
+	       (buffer-name buffer) display-buffer-alist
+               (if label
+                   (cons (car action) (cons `(,label . t) (cdr action)))
+                 action)))
              (special-action (display-buffer--special-action buffer))
 	     ;; Extra actions from the arguments to this function:
 	     (extra-action
@@ -9288,7 +9310,7 @@ display-buffer-no-window
     'fail))

 ;;; Display + selection commands:
-(defun pop-to-buffer (buffer-or-name &optional action norecord)
+(defun pop-to-buffer (buffer-or-name &optional action norecord label)
   "Display buffer specified by BUFFER-OR-NAME and select its window.
 BUFFER-OR-NAME may be a buffer, a string (a buffer name), or nil.
 If it is a string not naming an existent buffer, create a buffer
@@ -9307,12 +9329,15 @@ pop-to-buffer
 input focus.

 Optional third arg NORECORD non-nil means do not put this buffer
-at the front of the list of recently selected ones."
+at the front of the list of recently selected ones.
+
+Optional fourth argument LABEL, if non-nil, must specify the name of a
+buffer display label and is passed on to `display-buffer'."
   (interactive (list (read-buffer "Pop to buffer: " (other-buffer))
 		     (if current-prefix-arg t)))
   (let* ((buffer (window-normalize-buffer-to-switch-to buffer-or-name))
          (old-frame (selected-frame))
-	 (window (display-buffer buffer action)))
+	 (window (display-buffer buffer action nil label)))
     ;; Don't assume that `display-buffer' has supplied us with a window
     ;; (Bug#24332).
     (if window
@@ -9328,7 +9353,7 @@ pop-to-buffer
     ;; Return BUFFER even when we got no window.
     buffer))

-(defun pop-to-buffer-same-window (buffer &optional norecord)
+(defun pop-to-buffer-same-window (buffer &optional norecord label)
   "Select buffer BUFFER in some window, preferably the same one.
 BUFFER may be a buffer, a string (a buffer name), or nil.  If it
 is a string not naming an existent buffer, create a buffer with
@@ -9338,6 +9363,9 @@ pop-to-buffer-same-window
 Optional argument NORECORD, if non-nil means do not put this
 buffer at the front of the list of recently selected ones.

+Optional fourth argument LABEL, if non-nil, must specify the name
+of a buffer display label and is passed on to `pop-to-buffer'.
+
 Unlike `pop-to-buffer', this function prefers using the selected
 window over popping up a new window or frame.  Specifically, if
 the selected window is neither a minibuffer window (as reported
@@ -9345,7 +9373,8 @@ pop-to-buffer-same-window
 (see `window-dedicated-p'), BUFFER will be displayed in the
 currently selected window; otherwise it will be displayed in
 another window."
-  (pop-to-buffer buffer display-buffer--same-window-action norecord))
+  (pop-to-buffer
+   buffer display-buffer--same-window-action norecord label))

 (defun read-buffer-to-switch (prompt)
   "Read the name of a buffer to switch to, prompting with PROMPT.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

>> IIRC I once used the term 'label' for it.  It vanished soon.
>
> After trying to implement this idea I immediately started to dislike it
> for technical reasons ('pop-to-buffer-same-window' never had an ACTION
> argument so I had to invent some clumsy extra argument to make things
> work).  I attach the diffs in case we still want to implement something
> the like.

So in addition to

  (pop-to-buffer "*shell*" '(nil . ((label . *shell*))))

it will also allow

  (pop-to-buffer "*shell*" nil nil '*shell*)

and

  (pop-to-buffer-same-window "*shell*" nil '*shell*)

?

> Otherwise, I think that harm has been done - 'pop-to-buffer-same-window'
> was invented as stand-in for 'switch-to-buffer' and should never have
> replaced 'pop-to-buffer' - but I missed that back then so I'm not in a
> good position to ask for reverting that change now.

Indeed, it's just

  (defun pop-to-buffer-same-window (buffer &optional norecord)
    (pop-to-buffer buffer display-buffer--same-window-action norecord))

> Sam's patch, although it does not restore the previous behavior, still
> looks like the least obtrusive solution in this regard.  It's partially
> a misnomer - to my knowledge at least 'eshell' has no connection to
> "comint" - so we apparently cannot put 'display-comint-buffer-action'
> into comint.el where it would belong.  Yet I'd tend to keep it, after
> applying Morgan's fix from Bug#52878, obviously.

For consistency with 'pop-to-buffer-same-window', shouldn't then
new 'display-comint-buffer-action' have a similar function:

  (defun pop-to-comint-buffer (buffer &optional norecord)
    (pop-to-buffer buffer display-comint-buffer-action norecord))

?

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > So in addition to
 >
 >    (pop-to-buffer "*shell*" '(nil . ((label . *shell*))))
 >
 > it will also allow
 >
 >    (pop-to-buffer "*shell*" nil nil '*shell*)
 >
 > and
 >
 >    (pop-to-buffer-same-window "*shell*" nil '*shell*)
 >
 > ?

Yes.  Looks neither pretty nor intuitive.

Note that a 'pop-to-buffer' call can add an arbitrary element like a
label to the action alist and 'display-buffer-alist' can, via a
function, interpret that label in any which way.  So we do not have to
add anything new to make such things work.  The important aspects are
rather discoverability and ease of documentation.

 > For consistency with 'pop-to-buffer-same-window', shouldn't then
 > new 'display-comint-buffer-action' have a similar function:
 >
 >    (defun pop-to-comint-buffer (buffer &optional norecord)
 >      (pop-to-buffer buffer display-comint-buffer-action norecord))

We could do that.  But let's fix the customization type of
'display-comint-buffer-action' first.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

Hi,
Please replace the @amazon.com email address in this thread with
sds@gnu.org or at least remove the @amazon address.
Thank you!

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://memri.org https://jij.org https://www.dhimmitude.org https://camera.org
If you do not move, you will not feel the shackles.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Eli Zaretskii]

> From: Sam Steingold <sds@gnu.org>
> Date: Fri, 07 Jan 2022 13:49:59 -0500
> 
> Please replace the @amazon.com email address in this thread with
> sds@gnu.org or at least remove the @amazon address.

I don't think this is possible.  At least I don't know how to do it.
Sorry.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

> * Eli Zaretskii <ryvm@tah.bet> [2022-01-07 21:03:25 +0200]:
>
>> From: Sam Steingold <sds@gnu.org>
>> Date: Fri, 07 Jan 2022 13:49:59 -0500
>> 
>> Please replace the @amazon.com email address in this thread with
>> sds@gnu.org or at least remove the @amazon address.
>
> I don't think this is possible.  At least I don't know how to do it.

All I am asking is for people to please edit the CC list before sending
replies. TIA.

Obviously, modifying history is not an option.

Sorry about the misunderstanding.

-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://iris.org.il https://www.dhimmitude.org
Time would have been the best Teacher, if it did not kill all its students.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

> Note that a 'pop-to-buffer' call can add an arbitrary element like a
> label to the action alist and 'display-buffer-alist' can, via a
> function, interpret that label in any which way.  So we do not have to
> add anything new to make such things work.  The important aspects are
> rather discoverability and ease of documentation.

Indeed, this would be nice to have:

  (pop-to-buffer "*shell*" '(display-buffer-reuse-or-same-window .
                             ((label . *shell*))))

>> For consistency with 'pop-to-buffer-same-window', shouldn't then
>> new 'display-comint-buffer-action' have a similar function:
>>
>>    (defun pop-to-comint-buffer (buffer &optional norecord)
>>      (pop-to-buffer buffer display-comint-buffer-action norecord))
>
> We could do that.  But let's fix the customization type of
> 'display-comint-buffer-action' first.

This is fixed now.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 >> Note that a 'pop-to-buffer' call can add an arbitrary element like a
 >> label to the action alist and 'display-buffer-alist' can, via a
 >> function, interpret that label in any which way.  So we do not have to
 >> add anything new to make such things work.  The important aspects are
 >> rather discoverability and ease of documentation.
 >
 > Indeed, this would be nice to have:
 >
 >    (pop-to-buffer "*shell*" '(display-buffer-reuse-or-same-window .
 >                               ((label . *shell*))))

The question is who we think should take care of this:
'display-buffer-assq-regexp' in association with 'display-buffer-alist'?
Or just a function specified via 'display-buffer-alist'?  The latter can
be already done now with some documentation support maybe.  The former
also requires a few code changes and a decision how to handle

(pop-to-buffer "*shell*" '(display-buffer-reuse-or-same-window
			   . ((label . *shell*) (label . *comint*))))

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

>>> Note that a 'pop-to-buffer' call can add an arbitrary element like a
>>> label to the action alist and 'display-buffer-alist' can, via a
>>> function, interpret that label in any which way.  So we do not have to
>>> add anything new to make such things work.  The important aspects are
>>> rather discoverability and ease of documentation.
>>
>> Indeed, this would be nice to have:
>>
>>    (pop-to-buffer "*shell*" '(display-buffer-reuse-or-same-window .
>>                               ((label . *shell*))))
>
> The question is who we think should take care of this:
> 'display-buffer-assq-regexp' in association with 'display-buffer-alist'?
> Or just a function specified via 'display-buffer-alist'?  The latter can
> be already done now with some documentation support maybe.

The latter, that already can be used.  Only one convenience change
could be added to allow symbols in conditions:

  (add-to-list 'display-buffer-alist
               '((*comint* display-buffer-reuse-or-same-window (nil))))

> The former also requires a few code changes and a decision how to
> handle
>
> (pop-to-buffer "*shell*" '(display-buffer-reuse-or-same-window
> 			   . ((label . *shell*) (label . *comint*))))

Maybe then allow a list: ((label . '(*shell* *comint*)))

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[martin rudalics]

 > The latter, that already can be used.  Only one convenience change
 > could be added to allow symbols in conditions:
 >
 >    (add-to-list 'display-buffer-alist
 >                 '((*comint* display-buffer-reuse-or-same-window (nil))))

As mentioned before, 'display-buffer-assq-regexp' would have to
recognize that too.  ‘functionp’ fails on *comint*.

 >> The former also requires a few code changes and a decision how to
 >> handle
 >>
 >> (pop-to-buffer "*shell*" '(display-buffer-reuse-or-same-window
 >> 			   . ((label . *shell*) (label . *comint*))))
 >
 > Maybe then allow a list: ((label . '(*shell* *comint*)))

Then we could simply write '*shell-or-comint*'.  What I had in mind are
cascaded calls in the sense that 'pop-to-shell' calls 'pop-to-comint'
and both would be allowed to add their own label.

martin

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Juri Linkov]

>> This is how we can make customization simpler, helping to get the old behavior
>> back for a set of commands.  Currently there are 11 comint commands that
>> have invocations like:
>>
>>    (pop-to-buffer "*inferior-lisp*" display-comint-buffer-action)
>>    (pop-to-buffer "*scheme*" display-comint-buffer-action)
>>    ...
>
> IIUC 'shell' still has (pop-to-buffer buffer), 'sh-show-shell' has
> (pop-to-buffer (process-buffer (sh-shell-process t))).  Shouldn't these
> become part of the same scheme?

Right, this omission should be fixed.

> Yet the problem remains how to easily get back the old behavior for
> individual 'display-buffer'-based calls we decided to change.

There was an idea to create a package that will revert all changes
in the default values since the last release.  So e.g. antinews-29.el
could contain:

    ;; Revert variable-pitch mode-line back to fixed:
    (set-face-attribute 'mode-line nil :inherit 'fixed-pitch)

    ;; Display comint buffers in another window again:
    (add-to-list 'display-buffer-alist
                 '("\\*inferior-lisp\\*\\|\\*scheme\\*\\|..."
                   (display-buffer-pop-up-window
                    . ((inhibit-same-window . t)))))
    ...

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Sam Steingold]

> * Lars Ingebrigtsen <ynefv@tahf.bet> [2021-12-29 16:00:44 +0100]:
>
> Sam Steingold <sds@gnu.org> writes:
>
>>     Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'
>>
>>     * lisp/window.el (display-comint-buffer-action): New `defcustom`,
>>     defaults to 'display-buffer-same-window' for backward compatibility.
>
> Others have noted the technical problems with this patch, but as the
> discussion in bug#52467 showed, Emacs already has capabilities for
> customising the behaviour here (via the general display machinery), and
> adding something cross-cutting but specific to the coming-derived modes
> just seems odd to me, so I'd be in favour of reverting this patch.

`display-comint-buffer-action' was my concession to the people who wanted
comint buffers displayed in the same window.

Personally, I would _love_ to kill it and use plain `pop-to-buffer'
everywhere.



-- 
Sam Steingold (http://sds.podval.org/) on darwin Ns 10.3.2022
http://childpsy.net http://calmchildstories.com http://steingoldpsychology.com
https://ij.org/ https://honestreporting.com https://jihadwatch.org
Do not tell me what to do and I will not tell you where to go.

Re: master 18b680cfd1: Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

[Glenn Morris]

> branch: master
> commit 18b680cfd177e877991be2bd70ead628bbdc0aa0

>     Fix bug#52467 by adding a new custom variable 'display-comint-buffer-action'

emacs -Q
M-x customize-variable RET display-comint-buffer-action RET

  -> widget-apply: Symbol's function definition is void: nil