[xmonad] my proposal for a Xinerama safe PerWorkspace

[David Roundy]

On Wed, Feb 27, 2008 at 12:00:33PM -0500, Brent Yorgey wrote:
> On Tue, Feb 26, 2008 at 9:27 PM, Spencer Janssen <sjanssen at 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 at 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 at 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 clarify: "this style of API" is one in which a class contains a chain of
several methods, any one of which may be defined, and only one of which may
be used.  It would be nice if we could enforce this restriction, but alas,
we can't.

I find this a useful approach:  it eases the creation of new instances, at
the expense of introducing an easily-summarized restriction on the use of
the class.  In fact, it wouldn't be too hard to write a checker that
enforces this (by reading the code and seeing if doLayout/pureLayout are
ever used apart from an instance definition).

But it's also true that there are other approaches one could use that would
be similarly easy.  We could define only runLayout in the class and then
define helper functions such as runLayoutFromPureLayout or
runLayoutFromDoLayout that would allow folks creating new instances or
porting old ones to use the simpler API if they like.

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

No, most instances of LayoutClass will be unchanged.  It's *users* of
LayoutClass that have to be changed, and these are far more rare.  The
advantage of this approach is that code that merely defines an instance of
LayoutClass can be guaranteed not to require updating, nor to have a bug
introduced.  It's only code that has "LayoutClass l a =>" in it that will
need reworking.

I like the runLayout approach, because it unifies and completes the
information provided to the layout: it provides all the information we've
got about the workspace being displayed, and it won't need to be changed if
we start including more information about workspaces (provided we store it
in Workspace).  I think that's nice.  The emptyLayout addition was a much
less elegant hack--and had precisely the same downside with regard to
breaking layout combinators.  In fact, I would assert the rules that must
be obeyed by layout combinators are simplified by the introduction of
runLayout.
-- 
David Roundy
Department of Physics
Oregon State University