Code review comment for lp:~tcaswell-gmail/python-mode/window_fix

Revision history for this message
Andreas Roehler (a-roehler) wrote :

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 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.

indeed

>
> This logic is not in the lisp yet.
>

correct

>
> 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 where *Python* is not visible at the end
> p_win = None
> do nothing
>
> if py-switch-buffers-on-execute-p or py-replace-code-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
>

Looks like py-shell-manage-windows needs to be re-written from scratch.

Nice result so far, cheers,

Andreas

« Back to merge proposal