Subject: Re: bug#33870: 27.0.50; xref-goto-xref not configurable

From: martin rudalics <[email-masked]>
Date: Wed, 09 Jan 2019 11:03:15 +0100

>> We have 'display-buffer-alist' for quite some time now.
 >> So please consider making this an action alist entry.
 >
 > Yes, it would be much better, but it never seems to reach the top of my
 > todo list.
 >
 >> That way a user can decide whether all buffers displayed by
 >> 'display-buffer' should be dedicated or only certain ones and which
 >> 'dedicated' value they should get.
 >
 > Historically, special-display-buffer-alist always caused
 > the created frames/windows to be dedicated, so
 > display-buffer-mark-dedicated extends this to those windows created for
 > other reasons.
 >
 > I haven't looked in detail, but this seems to make it less trivial to
 > just add a new action alist parameter: it should default to `t` if we
 > matched in display-buffer-alist but to nil if we only rely on
 > display-buffer-base-action?

I'm missing you here.  An ALIST argument is equally passed to all
buffer display actions regardless of whether they are specifed by
'display-buffer-base-action' or by someone else.  It's their choice
whether they want to obey or disregard it.  The same currently holds
for 'display-buffer-mark-dedicated'.

 > Also, some (all?) let-bindings of display-buffer-mark-dedicated should

I don't see any such bindings in our current code base.

 > now be unnecessary (because of the features you added so bury-buffer (or
 > was it quit-window?) automatically deletes the window).

This use case of dedicated windows should be no more necessary indeed.

I attach a patch of my proposed changes.  After applying that I have
no more objections against renaming 'window--display-buffer' any way
people want.

martin
diff --git a/lisp/window.el b/lisp/window.el
index 37d82c0..e53cc2b 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -700,8 +700,7 @@ display-buffer-in-atom-window
 	(set-window-parameter window 'window-atom 'main))
       (set-window-parameter new 'window-atom side)
       ;; Display BUFFER in NEW and return NEW.
-      (window--display-buffer
-       buffer new 'window alist display-buffer-mark-dedicated))))
+      (window--display-buffer buffer new 'window alist))))
 
 (defun window--atom-check-1 (window)
   "Subroutine of `window--atom-check'."
@@ -958,7 +957,11 @@ window--make-major-side-window
 	 ;; window and not make a new parent window unless needed.
 	 (window-combination-resize 'side)
 	 (window-combination-limit nil)
-	 (window (split-window-no-error next-to nil on-side)))
+	 (window (split-window-no-error next-to nil on-side))
+         (alist (if (or display-buffer-mark-dedicated
+                        (assq 'dedicated alist))
+                    alist
+                  (cons '(dedicated . side) alist))))
     (when window
       ;; Initialize `window-side' parameter of new window to SIDE and
       ;; make that parameter persistent.
@@ -985,7 +988,7 @@ window--make-major-side-window
       (with-current-buffer buffer
         (setq window--sides-shown t))
       ;; Install BUFFER in new window and return WINDOW.
-      (window--display-buffer buffer window 'window alist 'side))))
+      (window--display-buffer buffer window 'window alist))))
 
 (defun display-buffer-in-side-window (buffer alist)
   "Display BUFFER in a side window of the selected frame.
@@ -1019,10 +1022,7 @@ display-buffer-in-side-window
 explicitly provided via a `window-parameters' entry in ALIST."
   (let* ((side (or (cdr (assq 'side alist)) 'bottom))
          (slot (or (cdr (assq 'slot alist)) 0))
-         (left-or-right (memq side '(left right)))
-         ;; Softly dedicate window to BUFFER unless
-         ;; `display-buffer-mark-dedicated' already asks for it.
-         (dedicated (or display-buffer-mark-dedicated 'side)))
+         (left-or-right (memq side '(left right))))
     (cond
      ((not (memq side '(top bottom left right)))
       (error "Invalid side %s specified" side))
@@ -1055,7 +1055,11 @@ display-buffer-in-side-window
 		  ((eq side 'bottom) 3))
 		 window-sides-slots))
            (window--sides-inhibit-check t)
-	   window this-window this-slot prev-window next-window
+           (alist (if (or display-buffer-mark-dedicated
+                          (assq 'dedicated alist))
+                      alist
+                    (cons '(dedicated . side) alist)))
+           window this-window this-slot prev-window next-window
 	   best-window best-slot abs-slot)
 
       (cond
@@ -1113,8 +1117,7 @@ display-buffer-in-side-window
 		 ;; Reuse `this-window'.
                  (with-current-buffer buffer
                    (setq window--sides-shown t))
-		 (window--display-buffer
-                  buffer this-window 'reuse alist dedicated))
+		 (window--display-buffer buffer this-window 'reuse alist))
 	    (and (or (not max-slots) (< slots max-slots))
 		 (or (and next-window
 			  ;; Make new window before `next-window'.
@@ -1131,8 +1134,7 @@ display-buffer-in-side-window
 		   (set-window-parameter window 'window-slot slot)
                    (with-current-buffer buffer
                      (setq window--sides-shown t))
-		   (window--display-buffer
-                    buffer window 'window alist dedicated))
+		   (window--display-buffer buffer window 'window alist))
 	    (and best-window
 		 ;; Reuse `best-window'.
 		 (progn
@@ -1141,7 +1143,7 @@ display-buffer-in-side-window
                    (with-current-buffer buffer
                      (setq window--sides-shown t))
                    (window--display-buffer
-		    buffer best-window 'reuse alist dedicated)))))))))
+                    buffer best-window 'reuse alist)))))))))
 
 (defun window-toggle-side-windows (&optional frame)
   "Toggle display of side windows on specified FRAME.
@@ -6748,20 +6750,47 @@ window--even-window-sizes
            (/ (- (window-total-height window) (window-total-height)) 2))
         (error nil))))))
 
-(defun window--display-buffer (buffer window type &optional alist dedicated)
+(defun window--display-buffer (buffer window type &optional alist)
   "Display BUFFER in WINDOW.
-TYPE must be one of the symbols `reuse', `window' or `frame' and
-is passed unaltered to `display-buffer-record-window'.  ALIST is
-the alist argument of `display-buffer'.  Set `window-dedicated-p'
-to DEDICATED if non-nil.  Return WINDOW if BUFFER and WINDOW are
-live."
+TYPE must be one of the following symbols: 'reuse' (which means
+WINDOW existed before the call of `display-buffer' and may
+already show BUFFER or not), 'window' (WINDOW was created on an
+existing frame) or 'frame' (WINDOW was created on a new frame)
+and is passed unaltered to `display-buffer-record-window'.  ALIST
+is the action alist compiled by `display-buffer'.
+
+Handle WINDOW's dedicated flag as follows: If WINDOW already
+shows BUFFER, leave it alone.  Otherwise, if ALIST contains a
+'dedicated' entry and the window is either new or the cdr of that
+entry equals 'side', set WINDOW's dedicated flag to the cdr of
+that entry.  Otherwise, if 'display-buffer-mark-dedicated' is
+non-nil and TYPE equals 'window' of 'frame', set WINDOW's
+dedicated flag to the value of 'display-buffer-mark-dedicated'.
+In any other case, reset WINDOW's dedicated flag to nil.
+
+Return WINDOW if BUFFER and WINDOW are live."
+  (setq display-buffer--type type)
   (when (and (buffer-live-p buffer) (window-live-p window))
     (display-buffer-record-window type window buffer)
     (unless (eq buffer (window-buffer window))
+      ;; Reset WINDOW's dedicated status unless it already shows
+      ;; BUFFER.
       (set-window-dedicated-p window nil)
       (set-window-buffer window buffer))
-    (when dedicated
-      (set-window-dedicated-p window dedicated))
+    (let ((alist-dedicated (assq 'dedicated alist)))
+      ;; Maybe dedicate WINDOW to BUFFER if asked for.
+      (cond
+       ;; Don't dedicate WINDOW if it is dedicated because it shows
+       ;; BUFFER already or it is reused and is not a side window.
+       ((or (window-dedicated-p window)
+            (and (eq type 'reuse) (not (eq alist-dedicated 'side)))))
+       ;; Otherwise, if ALIST contains a 'dedicated' entry, use that.
+       (alist-dedicated
+        (set-window-dedicated-p window (cdr alist-dedicated)))
+       ;; Otherwise, if 'display-buffer-mark-dedicated' is non-nil,
+       ;; use that.
+       ((and display-buffer-mark-dedicated (memq type '(window frame)))
+        (set-window-dedicated-p window display-buffer-mark-dedicated))))
     (when (memq type '(window frame))
       (set-window-prev-buffers window nil))
     (let ((quit-restore (window-parameter window 'quit-restore))
@@ -7190,8 +7219,7 @@ display-buffer-use-some-frame
                 frame nil (cdr (assq 'inhibit-same-window alist))))))
     (when window
       (prog1
-          (window--display-buffer
-           buffer window 'reuse alist display-buffer-mark-dedicated)
+          (window--display-buffer buffer window 'reuse alist)
         (unless (cdr (assq 'inhibit-switch-frame alist))
           (window--maybe-raise-frame frame))))))
 
@@ -7356,8 +7384,7 @@ display-buffer-pop-up-frame
 	       (with-current-buffer buffer
 		 (setq frame (funcall fun)))
 	       (setq window (frame-selected-window frame)))
-      (prog1 (window--display-buffer
-	      buffer window 'frame alist display-buffer-mark-dedicated)
+      (prog1 (window--display-buffer buffer window 'frame alist)
 	(unless (cdr (assq 'inhibit-switch-frame alist))
 	  (window--maybe-raise-frame frame))))))
 
@@ -7386,8 +7413,7 @@ display-buffer-pop-up-window
 				(window--try-to-split-window
 				 (get-lru-window frame t) alist))))
 
-      (prog1 (window--display-buffer
-	      buffer window 'window alist display-buffer-mark-dedicated)
+      (prog1 (window--display-buffer buffer window 'window alist)
 	(unless (cdr (assq 'inhibit-switch-frame alist))
 	  (window--maybe-raise-frame (window-frame window)))))))
 
@@ -7435,7 +7461,7 @@ display-buffer-in-child-frame
 	 (parent (or (assq 'parent-frame parameters)
                      (selected-frame)))
          (share (assq 'share-child-frame parameters))
-         share1 frame window)
+         share1 frame window type)
     (with-current-buffer buffer
       (when (frame-live-p parent)
         (catch 'frame
@@ -7448,12 +7474,14 @@ display-buffer-in-child-frame
                 (throw 'frame t))))))
 
       (if frame
-          (setq window (frame-selected-window frame))
+          (progn
+            (setq window (frame-selected-window frame))
+            (setq type 'reuse))
         (setq frame (make-frame parameters))
-        (setq window (frame-selected-window frame))))
+        (setq window (frame-selected-window frame))
+        (setq type 'frame)))
 
-    (prog1 (window--display-buffer
-	    buffer window 'frame alist display-buffer-mark-dedicated)
+    (prog1 (window--display-buffer buffer window type alist)
       (unless (cdr (assq 'inhibit-switch-frame alist))
 	(window--maybe-raise-frame frame)))))
 
@@ -7492,16 +7520,14 @@ display-buffer-below-selected
 		   split-width-threshold)
                (setq window (window--try-to-split-window
                              (selected-window) alist)))
-             (window--display-buffer
-              buffer window 'window alist display-buffer-mark-dedicated))
+             (window--display-buffer buffer window 'window alist))
 	(and (setq window (window-in-direction 'below))
              (not (window-dedicated-p window))
 	     (or (not (numberp min-height))
 		 ;; A window that showed another buffer before cannot
 		 ;; be resized.
 		 (>= (window-height window) min-height))
-	     (window--display-buffer
-              buffer window 'reuse alist display-buffer-mark-dedicated)))))
+	     (window--display-buffer buffer window 'reuse alist)))))
 
 (defun display-buffer--maybe-at-bottom (buffer alist)
   (let ((alist (append alist `(,(if temp-buffer-resize-mode
@@ -7533,21 +7559,17 @@ display-buffer-at-bottom
 	 (setq bottom-window window))))
      nil nil 'nomini)
     (or (and bottom-window-shows-buffer
-	     (window--display-buffer
-	      buffer bottom-window 'reuse alist display-buffer-mark-dedicated))
+	     (window--display-buffer buffer bottom-window 'reuse alist))
 	(and (not (frame-parameter nil 'unsplittable))
-	     (let (split-width-threshold)
+	     (let (split-height-threshold)
 	       (setq window (window--try-to-split-window bottom-window alist)))
-	     (window--display-buffer
-	      buffer window 'window alist display-buffer-mark-dedicated))
+	     (window--display-buffer buffer window 'window alist))
 	(and (not (frame-parameter nil 'unsplittable))
 	     (setq window (split-window-no-error (window-main-window)))
-	     (window--display-buffer
-	      buffer window 'window alist display-buffer-mark-dedicated))
+	     (window--display-buffer buffer window 'window alist))
 	(and (setq window bottom-window)
 	     (not (window-dedicated-p window))
-	     (window--display-buffer
-	      buffer window 'reuse alist display-buffer-mark-dedicated)))))
+	     (window--display-buffer buffer window 'reuse alist)))))
 
 (defun display-buffer-in-previous-window (buffer alist)
   "Display BUFFER in a window previously showing it.
@@ -7596,7 +7618,8 @@ display-buffer-in-previous-window
     ;; anything we found so far.
     (when (and (setq window (cdr (assq 'previous-window alist)))
 	       (window-live-p window)
-	       (not (window-dedicated-p window)))
+	       (or (eq buffer (window-buffer window))
+                   (not (window-dedicated-p window))))
       (if (eq window (selected-window))
 	  (unless inhibit-same-window
 	    (setq second-best-window window))