Merge lp:~tcaswell-gmail/python-mode/window_fix into lp:python-mode

[Thomas Caswell]

related to bug #1018996

[Andreas Roehler (a-roehler)]

Looks like you spotted the bug with changes of `py-execute-buffer-file' and `py-execute-buffer-finally', congratulations!

As for the remaining stuff, IMO we need some distance to go, it's not at the bottom yet.
Please do me the favor and restrict your patch at the changes mentioned above, so I may merge this branch.

If interested, we may dig a little big deeper afterward into the architecture of this section.
Would welcome if may take it in charge somehow. As a starting-point we may take the pseudo-code mailed by Yaroslav lately. Addressing this and some more rationales might help to choose solutions if issues rise up.

[Thomas Caswell (tcaswell-gmail)]

On Mon, Dec 24, 2012 at 7:15 AM, Andreas Roehler
<email address hidden> wrote:
> with changes of `py-execute-buffer-file' and `py-execute-buffer-finally'

Here is a patch of just those two lines.

--
Thomas Caswell
<email address hidden>

[Andreas Roehler (a-roehler)]

Am 24.12.2012 19:04, schrieb Thomas Caswell:
> On Mon, Dec 24, 2012 at 7:15 AM, Andreas Roehler
> <email address hidden> wrote:
>> with changes of `py-execute-buffer-file' and `py-execute-buffer-finally'
>
>
> Here is a patch of just those two lines.
>
>
> --
> Thomas Caswell
> <email address hidden>
>

Thanks. May you still upload it? Otherwise your branch remains with a merge-proposal without merge...

Cheers,

Andreas

[Andreas Roehler (a-roehler)]

ah, okay, you did it already. So I'm going to merge.

[Andreas Roehler (a-roehler)]

hmmn, was just your patch displayed.

[Thomas Caswell (tcaswell-gmail)]

I would re-arrange the psudo-code of Yaroslav a bit to include the
limit on the number of windows that can be made. It also deals with
the case where switching and splitting are both off, and there is more
than one window, the user may want the execution buffer to be
displayed in one of the other windows.

Consider the case where switching is off, splitting is on, there are 2
windows neither of which are showing *Python*, and
py-max-split-windows = 2 (which I think is equivalent to splitting
being turned off). What we want is this case is to put *Python* in the
window that our code is not in. This generalizes to more than 2 windows
directly by selecting the least recently used window.

(I wrote the psudo-code like python because I assume everyone
interested in this mode can read python. )

if py-manage-buffers-on-execute-p:
    # we want to make sure the user can see the output
    if *Python* buffer is visible:
        # we can already see *Python*, thus we are done
        p_win = window *Python* is in
        pass
    else:
        # we need to try and make *Python* visible
        if py-split-windows-on-execute-p and count-windows < py-max-split-windows:
            # we can make a new window
            do the split and put *Python* in the new window
            p_win = new window
        else:
            if count-windows > 1:
                seleect some other window and put *Python* in it
                p_win = window *Python* was put in
            else:
                # this is the only branch where *Python* is not visible at the end
                p_win = None
                do nothing

    if py-switch-buffers-on-execute-p:
        if p_win:
            switch focus to p_win
        else:
            # this is the only branch that replaces the code window with the output
            switch the buffer displayed in the current window, replacing the code
else:
    # DO NOTHING
    pass

To deal with the use case where the user wants to always replace
the code window with the output (with what I think is a reasonable
exception of *Python* being already visible, in which case it behaves
like switching is turned on and splitting is turned off), we need to
add another flag. If this flag (py-replace-code-on-execute-p) is set,
the values of the split and switch flag are irrelevant.

This logic is not in the lisp yet.

if py-manage-buffers-on-execute-p:
    # we want to make sure the user can see the output
    if *Python* buffer is visible:
        # we can already see *Python*, thus we are done
        p_win = window *Python* is in
        pass
    else:
        if py-replace-code-on-execute-p:
            p_win = None
        # we need to try and make *Python* visible
        elif py-split-windows-on-execute-p and count-windows < py-max-split-windows:
            # we can make a new window
            do the split and put *Python* in the new window
            p_win = new window
        else:
            if count-windows > 1:
                seleect some other window and put *Python* in it
                p_win = window *Python* was put in
            else:
                # this is the only branch w...

[Andreas Roehler (a-roehler)]

Am 30.12.2012 21:36, schrieb Thomas Caswell:
> I would re-arrange the psudo-code of Yaroslav a bit

Great, was meant just as a starting point.

  to include the
> limit on the number of windows that can be made. It also deals with
> the case where switching and splitting are both off, and there is more
> than one window, the user may want the execution buffer to be
> displayed in one of the other windows.
>
> Consider the case where switching is off, splitting is on, there are 2
> windows neither of which are showing *Python*, and
> py-max-split-windows = 2 (which I think is equivalent to splitting
> being turned off).

IMO that remark is misleading, as the way Emacs will create the final window configuration might be complex.
Temporary buffers and files might be created and so on. Think the final window state somehow independent from the
start configuration.

   What we want is this case is to put *Python* in the
> window that our code is not in. This generalizes to more than 2 windows
> directly by selecting the least recently used window.
>

See remark above. Such like "recently used" probably is not enough.

> (I wrote the psudo-code like python because I assume everyone
> interested in this mode can read python. )
>
>
> if py-manage-buffers-on-execute-p:

We don't need this boolean, because some management is needed in any case -
unless we want unpredicted results.

> # we want to make sure the user can see the output
> if *Python* buffer is visible:
> # we can already see *Python*, thus we are done

That's not sure user may want to see the output.
For example when running serialised stuff from a programm.

Basically the logic is:

Start window config
Unpredictable changes from point of the first
Desired final window config

Remains the question if 'switch and 'split are sufficient to describe the desired final config.

Probably not, also when thinking at use in org-babel.

> p_win = window *Python* is in
> pass
> else:
> # we need to try and make *Python* visible
> if py-split-windows-on-execute-p and count-windows < py-max-split-windows:
> # we can make a new window
> do the split and put *Python* in the new window
> p_win = new window
> else:
> if count-windows > 1:
> seleect some other window and put *Python* in it
> p_win = window *Python* was put in
> else:
> # this is the only branch where *Python* is not visible at the end
> p_win = None
> do nothing
>
> if py-switch-buffers-on-execute-p:
> if p_win:
> switch focus to p_win
> else:
> # this is the only branch that replaces the code window with the output
> switch the buffer displayed in the current window, replacing the code

the last is another possible option not implemented yet.

> else:
> # DO NOTHING
> pass
>
> To deal with the use case where the user wants to always replace
> the code window with the output (with what I think is a reasonable
> exception of *Python* being already visi...

=== modified file 'python-mode.el'
--- python-mode.el	2012-12-21 18:27:07 +0000
+++ python-mode.el	2012-12-22 21:11:20 +0000
@@ -710,6 +710,12 @@
   :type 'boolean
   :group 'python-mode)
 
+(defcustom py-on-execute-buffer-management-p t
+  "If emacs should attempt to manage your buffers on executing python code."
+  :type 'boolean
+  :group 'python-mode
+)
+
 (defcustom py-switch-buffers-on-execute-p nil
   "When non-nil switch to the Python output buffer. "
 
@@ -8757,7 +8763,6 @@
     (kill-buffer localname)))
 
 (defun py-execute-buffer-file (start end pyshellname dedicated switch nostars sepchar split file)
-  (delete-other-windows)
   (let* ((oldbuf (current-buffer))
          (py-exception-buffer oldbuf)
          (pyshellname (or pyshellname (py-choose-shell)))
@@ -8804,7 +8809,6 @@
     erg))
 
 (defun py-execute-buffer-finally (start end &optional pyshellname dedicated switch nostars sepchar split)
-  (delete-other-windows)
   (let* ((oldbuf (current-buffer))
          (pyshellname (or pyshellname (py-choose-shell)))
          (execute-directory
@@ -10492,41 +10496,78 @@
     "\*" ""
     string)))
 
+(defun py-try-make-output-buffer-visible (split py-buffer-name)
+  "A helper function that tries to make the output buffer is visible.
+
+If it is already visible it return the window it is in
+
+If it is not visible and splitting is allowed and there are less
+that the maximum number of windows, then it will split the code window,
+put the output buffer in the new window and return the new window
+
+If there is more than one  window and splitting is not allowed, then
+put the output buffer in the least recently used window and return that window
+
+if there is only one window and splitting in not allowed return nil
+"
+  (let ((cur-py-window (get-buffer-window py-buffer-name t)))
+    (if cur-py-window
+	;; There is a currently visible window with the output buffer in it use that one
+	cur-py-window
+      ;; if the output buffer isn't already visible, try to make it so
+      	  
+      ;;we have not explicitly said not to split, we are allowed to split, and
+      ;; there are few enough windows
+      (if (and (not (eq split 'nosplit))
+	       py-split-windows-on-execute-p
+	       (< (count-windows) py-max-split-windows)
+	       )
+	  (let ((new-py-wind (funcall py-split-windows-on-execute-function)))
+	    (set-window-buffer new-py-wind py-buffer-name)
+	    new-py-wind
+	    )
+	
+	(if (eq 1 (count-windows))
+	    nil
+	  ;; else make the lru window show the output buffer and return it
+	  (let ((new-py-wind (get-lru-window)))
+	    (set-window-buffer new-py-wind py-buffer-name)
+	    new-py-wind
+	    )
+	  )
+	)
+      )
+    )
+  )
+
 (defun py-shell-manage-windows (switch split oldbuf py-buffer-name)
-  (cond (;; split and switch
-         (and (not (eq split 'nosplit))
-              py-split-windows-on-execute-p
-              (not (eq switch 'noswitch))
-              (or (eq switch 'switch)
-                  py-switch-buffers-on-execute-p))
-         (when (< (count-windows) py-max-split-windows)
-           (funcall py-split-windows-on-execute-function))
-         (pop-to-buffer py-buffer-name)
-         (display-buffer oldbuf))
-        ;; split, not switch
-        ((and
-          (not (eq split 'nosplit))
-          py-split-windows-on-execute-p
-          (or (eq switch 'noswitch)
-              (not (eq switch 'switch))))
-         (if (< (count-windows) py-max-split-windows)
-             (progn
-               (funcall py-split-windows-on-execute-function)
-               (display-buffer py-buffer-name 'display-buffer-reuse-window))
-           (display-buffer py-buffer-name 'display-buffer-reuse-window))
-         (pop-to-buffer oldbuf))
-        ;; no split, switch
-        ((or (eq switch 'switch)
-             (and (not (eq switch 'noswitch))
-                  py-switch-buffers-on-execute-p))
-         (let (pop-up-windows)
-           (pop-to-buffer py-buffer-name)))
-        ;; no split, no switch
-        ((or (eq switch 'noswitch)
-             (not py-switch-buffers-on-execute-p))
-         (let (pop-up-windows)
-           (set-buffer oldbuf)
-           (switch-to-buffer (current-buffer))))))
+  (if py-on-execute-buffer-management-p 
+      ;;if we are going to let python mode touch our buffers under any conditions
+      (let ((cur-py-window (py-try-make-output-buffer-visible split py-buffer-name)))
+	;; if we are switching
+	(when 	 
+	    ;;this logic seems overly twisted    
+	    (and 		 
+		 (not (eq switch 'noswitch))
+		 (or (eq switch 'switch)
+		     py-switch-buffers-on-execute-p))
+	  (if cur-py-window
+	      ;; if we have a visible window with the output buffer, switch to it
+	      (select-window cur-py-window)
+	    ;; if we don't then make the current window display the output (case of 1 window, switch, nosplit)
+	    (switch-to-buffer py-buffer-name)
+	    )
+	  
+	  )
+       )
+    ;;else, return it to the state it was in
+    (let (pop-up-windows)
+      (set-buffer oldbuf)
+      (switch-to-buffer (current-buffer))
+      )
+    )
+  )
+
 
 (defun py-report-executable (py-buffer-name)
   (let ((erg (downcase (replace-regexp-in-string