bug#67249: 30.0.50; `same-frame` equivalent for `display-buffer-alist`

[Stefan Monnier]

>>> If OT1H 'same-frame' is ignored when the selected frame is a
>>> minibuffer-only frame (so a new frame gets popped up instead) and OTOH
>>> the remaining action functions do use the last non-minibuffer frame in
>>> such case, then the behavior of 'display-buffer' is inconsistent in my
>>> regard.
>> Ah, yes, I see.
>> IIUC, the "inhibit-new-frame" semantics seems less susceptible to this
>> problem then the "same-frame", no?
> When I type M-x to invoke a custom function for displaying a buffer, I'd
> probably want 'inhibit-new-frame' to do what it advertises regardless of
> whether I'm in a stand-alone minibuffer frame or in a normal minibuffer
> window.

Indeed, but the difference between "same frame" and "inhibit new frame"
is that the meaning of "same frame" depends on the `selected-frame`
whereas "inhibit new frame" doesn't, so I think we're OK.

Here's a first candidate patch.

I introduced a new function `display-buffer--pop-up-frame` so as to
ignore `inhibit-new-frame` as a last resort.

I also modified `display-buffer--other-frame-action` to use
`display-buffer--pop-up-frame`, because it seems this is used only(?) in
response to an explicit user request where the user does expect a new
frame (like `C-x 5 b`) and so it seems to make sense to override even an
`inhibit-new-frame` coming from `display-buffer-alist`.


        Stefan


PS: Am I the only one who finds it confusing how some functions
named `display-buffer-<foo>` are meant to be used from the ACTIONs
(i.e. from within `display-buffer`) while others are implemented on top
of `display-buffer` (and thus should not be used within ACTIONS)?
Could we try and find a clear naming convention to distinguish the two,
or am I even more confused than I thought and several of those functions
can be used either way?

diff --git a/lisp/window.el b/lisp/window.el
index 0c5ccf167dc..34027c58cae 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -6860,6 +6860,7 @@ special-display-popup-frame
 If ARGS is a list whose car is a symbol, use (car ARGS) as a
 function to do the work.  Pass it BUFFER as first argument, and
 pass the elements of (cdr ARGS) as the remaining arguments."
+  (declare (obsolete display-buffer-pop-up-frame "30.1"))
   (if (and args (symbolp (car args)))
       (apply (car args) buffer (cdr args))
     (let ((window (get-buffer-window buffer 0)))
@@ -7627,7 +7628,7 @@ display-buffer-fallback-action
      display-buffer-in-previous-window
      display-buffer-use-some-window
      ;; If all else fails, pop up a new frame.
-     display-buffer-pop-up-frame))
+     display-buffer--pop-up-frame))
   "Default fallback action for `display-buffer'.
 This is the action used by `display-buffer' if no other actions
 have been specified, for example, by the user options
@@ -7657,7 +7658,7 @@ display-buffer--same-window-action
 
 (defvar display-buffer--other-frame-action
   '((display-buffer-reuse-window
-     display-buffer-pop-up-frame)
+     display-buffer--pop-up-frame)
     (reusable-frames . 0)
     (inhibit-same-window . t))
   "A `display-buffer' action for displaying in another frame.
@@ -7735,6 +7736,8 @@ display-buffer
     Note that a window manager may still raise a new frame and
     give it focus, effectively overriding the value specified
     here.
+ `inhibit-new-frame' -- A non-nil value forces the use of an
+    existing frame to display the buffer.
  `reusable-frames' -- The value specifies the set of frames to
     search for a window that already displays the buffer.
     Possible values are nil (the selected frame), t (any live
@@ -7828,12 +7831,12 @@ display-buffer
                           user-action special-action action extra-action
                           display-buffer-base-action
                           display-buffer-fallback-action))
-           (functions (apply 'append
+           (functions (apply #'append
                              (mapcar (lambda (x)
                                        (setq x (car x))
                                        (if (functionp x) (list x) x))
                                      actions)))
-           (alist (apply 'append (mapcar 'cdr actions)))
+           (alist (apply #'append (mapcar #'cdr actions)))
            window)
       (unless (buffer-live-p buffer)
         (error "Invalid buffer"))
@@ -8090,10 +8093,16 @@ display-buffer--special-action
                          (funcall special-display-function
                                   buffer (if (listp pars) pars)))))))))
 
+(defun display-buffer--pop-up-frame (buffer alist)
+  "Like `display-buffer--pop-up-frame' but ignores `inhibit-new-frame'.
+Used only in exceptional cases."
+  (display-buffer-pop-up-frame buffer `((inhibit-new-frame . nil) ,@alist)))
+
 (defun display-buffer-pop-up-frame (buffer alist)
   "Display BUFFER in a new frame.
 This works by calling `pop-up-frame-function'.  If successful,
 return the window used; otherwise return nil.
+Do nothing if ALIST's `inhibit-new-frame' is non-nil.
 
 ALIST is an association list of action symbols and values.  See
 Info node `(elisp) Buffer Display Action Alists' for details of
@@ -8107,15 +8116,16 @@ display-buffer-pop-up-frame
 node `(elisp) Buffer Display Action Functions'.  It should be
 called only by `display-buffer' or a function directly or
 indirectly called by the latter."
-  (let* ((params (cdr (assq 'pop-up-frame-parameters alist)))
-        (pop-up-frame-alist (append params pop-up-frame-alist))
-        (fun pop-up-frame-function)
+  (let* ((fun pop-up-frame-function)
         frame window)
     (when (and fun
+              (not (alist-get 'inhibit-new-frame alist))
               ;; Make BUFFER current so `make-frame' will use it as the
               ;; new frame's buffer (Bug#15133).
               (with-current-buffer buffer
-                (setq frame (funcall fun)))
+                (let* ((params (cdr (assq 'pop-up-frame-parameters alist)))
+                       (pop-up-frame-alist (append params pop-up-frame-alist)))
+                  (setq frame (funcall fun))))
               (setq window (frame-selected-window frame)))
       (prog1 (window--display-buffer buffer window 'frame alist)
        (unless (cdr (assq 'inhibit-switch-frame alist))