[RFC PATCH] Changes to pop-up source buffers

[Jack Kamm <jackkamm@gmail.com>]

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

This patch changes some implementation details of
org-src-switch-to-buffer and org-edit-src-exit, to more consistently use
pop-to-buffer to open the source buffer, and quit-restore-window to
close it. It also adds a new default option to org-src-window-setup.

I'll explain some details and motivation for this now.

First, on killing the source buffer. Currently, we kill it with
kill-buffer, then restore the previous saved window configuration with
set-window-configuration. The downside is that, if changes to the
windows were made while editing the source buffer, for example if a new
window is opened to refer to some other file, then these changes will be
lost after the original window configuration is restored.

By contrast, this patch uses quit-restore-window to close the source
window, which won't affect other windows that may have been
changed. quit-restore-window is also smart enough to decide whether the
source window should be deleted (if the source buffer was popped up in a
new window), or whether the window should be kept and switched to some
previous buffer (if the source buffer was popped up on an existing
window).

Next, I changed org-src-window-setup to consistently use pop-to-buffer
to create the source window. This is needed for quit-restore-window to
figure out whether to delete the source window. Otherwise, if we
separately create the window and switch to it, then quit-restore-window
won't know whether the window was previously used for something else.

Finally, I added a new default option to org-src-window-setup, to use
the user's default configuration of display-buffer. Personally, I have
configured my own display-buffer-base-action and would like org-babel to
use it. Also, if a user is not satisfied with any of the options in
org-src-window-setup, this allows them to use their own configuration.

In most cases, the new default for org-src-window-setup will behave
similarly to the previous default (reorganize-frame). It will only
behave differently when 3 or more windows are currently open.

A final advantage I'd like to note for pop-to-buffer and
quit-restore-window, is that these are the mechanisms used by many
built-in Emacs functions to pop up and close windows, such as *Help*
windows.

There was one previous option for org-src-window-setup,
reorganize-frame, that I couldn't reimplement using pop-to-buffer. That
option required keeping around some logic for deleting windows and
restoring the configuration.

I tested the new implementation for org-src-switch-to-buffer, for all
options of org-src-window-setup, and it appears to work correctly. I
tested this on Emacs 26.3.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-org-src-use-display-buffer-quit-restore-window-for-s.patch --]
[-- Type: text/x-patch, Size: 6355 bytes --]

From fe2df20f3ddb72ca083c75eee7ece302abecf75a Mon Sep 17 00:00:00 2001
From: Jack Kamm <jackkamm@gmail.com>
Date: Sat, 18 Jan 2020 07:55:48 -0800
Subject: [PATCH] org-src: use display-buffer, quit-restore-window for source
 buffers

---
 etc/ORG-NEWS    | 14 ++++++++---
 lisp/org-src.el | 65 +++++++++++++++++++++++--------------------------
 2 files changed, 42 insertions(+), 37 deletions(-)

diff --git a/etc/ORG-NEWS b/etc/ORG-NEWS
index 67c3ca2ed..cbbd50cf0 100644
--- a/etc/ORG-NEWS
+++ b/etc/ORG-NEWS
@@ -30,10 +30,18 @@ group new datetime entries by month.
 Babel Java blocks recognize header argument =:cmdargs= and pass its
 value in call to =java=.
 
-*** Refinement in window behavior on exiting Org source buffer
+*** Improved window behavior for Org source buffer
 
-After editing a source block, Org will restore the window layout when
-~org-src-window-setup~ is set to a value that modifies the layout.
+More consistently use the ~display-buffer~ framework for popping up
+source buffers.
+
+Added an option ~default~ to ~org-src-window-setup~, that respects the
+user's default configuration of ~display-buffer~ to pop up
+buffers. This is the new default value for ~org-src-window-setup~.
+
+The option ~reorganize-frame~ was also reverted to the previous
+behavior of restoring the window configuration after exiting the
+source buffer.
 
 ** New functions
 *** ~org-columns-toggle-or-columns-quit~
diff --git a/lisp/org-src.el b/lisp/org-src.el
index 878821b14..949638fa0 100644
--- a/lisp/org-src.el
+++ b/lisp/org-src.el
@@ -144,26 +144,24 @@ the existing edit buffer."
   :package-version '(Org . "8.0")
   :type 'boolean)
 
-(defcustom org-src-window-setup 'reorganize-frame
+(defcustom org-src-window-setup 'default
   "How the source code edit buffer should be displayed.
 Possible values for this option are:
 
+default            Use default `display-buffer' action.
 current-window     Show edit buffer in the current window, keeping all other
                    windows.
 split-window-below Show edit buffer below the current window, keeping all
                    other windows.
 split-window-right Show edit buffer to the right of the current window,
                    keeping all other windows.
-other-window       Use `switch-to-buffer-other-window' to display edit buffer.
+other-window       Use some other window to display edit buffer.
 reorganize-frame   Show only two windows on the current frame, the current
-                   window and the edit buffer.
-other-frame        Use `switch-to-buffer-other-frame' to display edit buffer.
-                   Also, when exiting the edit buffer, kill that frame.
-
-Values that modify the window layout (reorganize-frame, split-window-below,
-split-window-right) will restore the layout after exiting the edit buffer."
+                   window and the edit buffer. Restore windows after exiting.
+other-frame        Use some other frame to display edit buffer."
   :group 'org-edit-structure
   :type '(choice
+	  (const default)
 	  (const current-window)
 	  (const split-window-below)
 	  (const split-window-right)
@@ -474,9 +472,7 @@ When REMOTE is non-nil, do not try to preserve point or mark when
 moving from the edit area to the source.
 
 Leave point in edit buffer."
-  (when (memq org-src-window-setup '(reorganize-frame
-				     split-window-below
-				     split-window-right))
+  (when (eql org-src-window-setup 'reorganize-frame)
     (setq org-src--saved-temp-window-config (current-window-configuration)))
   (let* ((area (org-src--contents-area datum))
 	 (beg (copy-marker (nth 0 area)))
@@ -801,34 +797,35 @@ Raise an error when current buffer is not a source editing buffer."
 
 (defun org-src-switch-to-buffer (buffer context)
   (pcase org-src-window-setup
+    (`default (pop-to-buffer buffer))
     (`current-window (pop-to-buffer-same-window buffer))
     (`other-window
-     (switch-to-buffer-other-window buffer))
+     (pop-to-buffer buffer '(nil
+			     ((reusable-frames . nil)
+			      (inhibit-same-window . t)))))
     (`split-window-below
-     (if (eq context 'exit)
-	 (delete-window)
-       (select-window (split-window-vertically)))
-     (pop-to-buffer-same-window buffer))
+     (let ((split-width-threshold)
+	   (split-height-threshold 0))
+       (pop-to-buffer buffer '((display-buffer-reuse-window
+				display-buffer-pop-up-window)
+			       ((reusable-frames . nil)
+				(inhibit-same-window . t))))))
     (`split-window-right
-     (if (eq context 'exit)
-	 (delete-window)
-       (select-window (split-window-horizontally)))
-     (pop-to-buffer-same-window buffer))
+     (let ((split-width-threshold 0)
+	   (split-height-threshold))
+       (pop-to-buffer buffer '((display-buffer-reuse-window
+				display-buffer-pop-up-window)
+			       ((reusable-frames . nil)
+				(inhibit-same-window . t))))))
     (`other-frame
-     (pcase context
-       (`exit
-	(let ((frame (selected-frame)))
-	  (switch-to-buffer-other-frame buffer)
-	  (delete-frame frame)))
-       (`save
-	(kill-buffer (current-buffer))
-	(pop-to-buffer-same-window buffer))
-       (_ (switch-to-buffer-other-frame buffer))))
+     (pop-to-buffer buffer '((display-buffer-reuse-window
+			      display-buffer-pop-up-frame)
+			     ((reusable-frames . 0)
+			      (inhibit-same-window . t)))))
     (`reorganize-frame
      (when (eq context 'edit) (delete-other-windows))
-     (org-switch-to-buffer-other-window buffer)
-     (when (eq context 'exit) (delete-other-windows)))
-    (`switch-invisibly (set-buffer buffer))
+     (org-switch-to-buffer-other-window buffer))
+    (`switch-invisibly (pop-to-buffer buffer '(display-buffer-no-window)))
     (_
      (message "Invalid value %s for `org-src-window-setup'"
 	      org-src-window-setup)
@@ -1167,8 +1164,8 @@ Throw an error if there is no such buffer."
     (let ((edit-buffer (current-buffer))
 	  (source-buffer (marker-buffer beg)))
       (unless source-buffer (error "Source buffer disappeared.  Aborting"))
-      (org-src-switch-to-buffer source-buffer 'exit)
-      (kill-buffer edit-buffer))
+      (quit-restore-window nil 'kill)
+      (pop-to-buffer source-buffer))
     ;; Insert modified code.  Ensure it ends with a newline character.
     (org-with-wide-buffer
      (when (and write-back (not (equal (buffer-substring beg end) code)))
-- 
2.25.0