[xmonad] my proposal for a Xinerama safe PerWorkspace

[Brent Yorgey]

On Wed, Feb 27, 2008 at 2:54 PM, David Roundy <droundy at darcs.net> wrote:

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


What's the current status on this?  Don? Spencer?

Summary so far:

  * Andrea's patch moves runLayout into the LayoutClass, changing the
signature slightly so that layouts receive all possible information.
  * This would necessitate updating all *users* of LayoutClass to use
runLayout instead of doLayout/emptyLayout.  The patches for these changes
are already available.
  * New instances of LayoutClass can choose whether to implement runLayout,
doLayout, or pureLayout, and so on.  Existing instances of LayoutClass will
not be affected (except those that are also users of it, e.g.
LayoutModifier).
  * This provides a nice solution (IMO the best) for PerWorkspace.

  Also:

  * If this patch is applied to the core, I am willing to take
responsibility for making sure the contrib library is updated appropriately,
and that thorough documentation is made available to guide those wishing to
understand the architecture of the LayoutClass.
  * David and I think this is elegant and should be applied.  (See David's
prior e-mail for an excellent discussion of some of the issues involved.)
  * Spencer has voiced discomfort with the style of this change to the API.
  * Don has not expressed an opinion one way or the other.

It would be nice if a decision could be reached on this one way or the
other, so we can move ahead with PerWorkspace and other things.  I'm not
trying to be pushy, just trying to spur things forward in a friendly way.

cheers,
-Brent
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://www.haskell.org/pipermail/xmonad/attachments/20080305/7c6f3d06/attachment-0001.htm