my proposal for a Xinerama safe PerWorkspace

Andrea Rossato

22 Feb 2008 22 Feb '08

1:12 p.m.

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 * 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 * 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 ps: I must confess that if the runLayout method could be accepted in the core my request of changing the 'description' type would be gone. While my sense of symmetry would require it for implementing a LayoutCombinator class as I have it mind, still I'm coming to thing that, from the practical point of view, the restrictions imposed by that signature are irrelevant for our purposes. And, probably, the fact of restricting the class to what I call pure combinators only, would be just fine. This is quite a common pattern of mine when it comes to type classes: I cannot see where I'll end up when I start implementing them. There must be some hidden weirdness I cannot grasp entirely. ;)

Attachments:

core_runLayout_method_proposal.dpatch (text/plain — 10.2 KB)
Perworkspace_runLayout_method_proposal.dpatch (text/plain — 44.0 KB)

Show replies by date

Andrea Rossato

22 Feb 22 Feb

1:27 p.m.

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

...

Hi,

this is my proposal for a Xinerama safe PerWorkspace.

obviously this proposal is not complete (I forgot to mention that!): I did not port the ModifiedLayout instance. Which means that if you use a layout modifier above PerWorkspace, it won't work. That is to say, this works: myLayoutHook = onWorkspace "9" (avoidStruts $ simpleTabBar Simplest) $ -- layout l1 will be used on workspace "foo". onWorkspaces ["var","mail","7"] simpleFloat $ -- layout l2 will be used on workspaces "bar" and "6". (avoidStruts simpleTabbed) but avoidStruts $ onWorkspace ... etc this will not work. I'm going to port the rest if runLayout will be accepted. Cheers, Andrea

Spencer Janssen

26 Feb 26 Feb

9:27 p.m.

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

ps: I must confess that if the runLayout method could be accepted in the core my request of changing the 'description' type would be gone. While my sense of symmetry would require it for implementing a LayoutCombinator class as I have it mind, still I'm coming to thing that, from the practical point of view, the restrictions imposed by that signature are irrelevant for our purposes. And, probably, the fact of restricting the class to what I call pure combinators only, would be just fine.

This is quite a common pattern of mine when it comes to type classes: I cannot see where I'll end up when I start implementing them. There must be some hidden weirdness I cannot grasp entirely. ;)

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. Cheers, Spencer Janssen

Brent Yorgey

27 Feb 27 Feb

noon

On Tue, Feb 26, 2008 at 9:27 PM, Spencer Janssen 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 * runLayout is now a LayoutClass method and takes the Workspace and

On Fri, Feb 22, 2008 at 07:12:31PM +0100, Andrea Rossato wrote: 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 * 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

David Roundy

2:54 p.m.

On Wed, Feb 27, 2008 at 12:00:33PM -0500, Brent Yorgey wrote:

...

On Tue, Feb 26, 2008 at 9:27 PM, Spencer Janssen 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 * runLayout is now a LayoutClass method and takes the Workspace and

On Fri, Feb 22, 2008 at 07:12:31PM +0100, Andrea Rossato wrote: 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 * 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

Brent Yorgey

5 Mar 5 Mar

12:50 p.m.

On Wed, Feb 27, 2008 at 2:54 PM, David Roundy 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 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

On Fri, Feb 22, 2008 at 07:12:31PM +0100, Andrea Rossato wrote: 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 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

Don Stewart

1:56 p.m.

byorgey:

...

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

Currently Spencer's out of action, and expressed some doubts, which is making it hard to reach a consensus here.

...

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.

I'm happy if Brent and David are on this one.

...

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.

As Spencer only expressed doubt here, and I'm persuaded by Brent's arguments (and his offering to champion them), let's proceed. -- Don

Brent Yorgey

8 Mar 8 Mar

2:37 p.m.

On Wed, Mar 5, 2008 at 1:56 PM, Don Stewart wrote:

...

byorgey:

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

Currently Spencer's out of action, and expressed some doubts, which is making it hard to reach a consensus here.

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

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

guide the

...
API. * Don has not expressed an opinion one way or the other.

I'm happy if Brent and David are on this one.

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

As Spencer only expressed doubt here, and I'm persuaded by Brent's arguments (and his offering to champion them), let's proceed.

OK, I am currently preparing a bundle of patches to update the core and contrib all at once. I'll send them out once I'm done (probably Monday). -Brent

6282

Age (days ago)

6297

Last active (days ago)

List overview

Download

7 comments

5 participants

participants (5)

Andrea Rossato
Brent Yorgey
David Roundy
Don Stewart
Spencer Janssen