On Tue, Feb 26, 2008 at 9:27 PM, Spencer Janssen <sjanssen@cse.unl.edu> wrote:

On Fri, Feb 22, 2008 at 07:12:31PM +0100, Andrea Rossato wrote:

> Hi,
>
> this is my proposal for a Xinerama safe PerWorkspace.
>
> It includes a new LayoutClass method:
>
> runLayout :: Workspace WorkspaceId (layout a) a -> Rectangle -> X ([(a, Rectangle)], Maybe (layout a))
>
> This is just some code moved around:
>
> Fri Feb 22 18:58:15 CET 2008  Andrea Rossato <andrea.rossato@unibz.it>
>   * runLayout is now a LayoutClass method and takes the Workspace and the screen Rectangle
>     M ./XMonad/Core.hs -7 +7
>     M ./XMonad/Operations.hs -5 +5
>
>
> This is the patch to PerWorkspace:
>
> Fri Feb 22 18:59:54 CET 2008  Andrea Rossato <andrea.rossato@unibz.it>
>   * PerWorkspace: reimplemented using runLayout
>   This way we have a Xinerama safe PerWorkspace and the emptyLayout
>   method for free.
>     M ./XMonad/Layout/PerWorkspace.hs -73 +27
>
> Some lines were left there just because I'm lazy....;)
>
> Cheers,
> Andrea
>

I'm not sure that I'm comfortable with this style of extension.  Consider the
existing pureLayout/doLayout functions in the layout class.   Outside consumers
of the API can really only use doLayout, because pureLayout often has the
garbage default implementation.  Here we're just adding another layer, meaning
that layout combinators and such can't safely use doLayout anymore, they must
be updated to use runLayout.  It just seems like this style of API just isn't
very nice.

Don, what do you think? Anyone else?

To summarize: Andrea's patch moves the 'runLayout' function into the LayoutClass, changing it to take an entire workspace on which to run the layout as input, rather than a layout and a stack.  The default implementation simply extracts the layout and stack from the workspace, and calls 'doLayout' or 'emptyLayout' as appropriate, but other instances of LayoutClass (for example, PerWorkspace) are free to override this behavior.  In the case of PerWorkspace, of course, the workspace tag is used.

This provides an elegant solution for PerWorkspace; however, as Spencer points out, it does mean that most instances of LayoutClass (especially those that implement layout combinators of some sort) must be changed to use runLayout instead of doLayout/emptyLayout (note Andrea has already done this I think), and it will become even more unclear to people writing a LayoutClass instance which methods they can/should override.

If the consensus is not to apply this, then I will go ahead and push my other PerWorkspace solution (using the startupHook and special "you are here" messages).

-Brent