darcs patch: refactor layout interface. (and 1 more)

Hi all,
Here's another shot at an interface that will allow prettier passing of
messages to the layout, so we can send interesting types. This patch adds
one warning in Main, since I removed the type of handle, so it could be
inferred from the type of keys. We could hardcode it to give an
X LayoutMsg (), but that seems uglier, to me. We could also consider
importing Main into Config (and renaming main), in which case handle (and
Main) would be functions of keys, and the name "keys" itself wouldn't be
hard-coded. It does seem prettier to make Config import all the other
modules, rather than having Config itself imported by another module.
I'm still not sure that this is a good plan. As long as our layout is a
pure function, we won't be able to do things that ion does, such as being
able to drag a tab from one location to another to move a window. That's
not a big deal, but if possible it'd be nice (if it can be done cleanly) to
define an API that doesn't limit the capabilities of the window manager.
David
Wed Apr 18 08:02:05 PDT 2007 David Roundy

On Wed, Apr 18, 2007 at 09:05:56AM -0700, David Roundy wrote:
hunk ./Operations.hs 49 - case layoutType fl of - Full -> fmap (flip (,) sc) $ maybeToList $ W.peekStack n ws - Tall -> tile (tileFraction fl) sc $ W.index n ws - Wide -> vtile (tileFraction fl) sc $ W.index n ws + (doLayout l) sc $ W.index n ws whenJust (W.peekStack n ws) (io . raiseWindow d) whenJust (W.peek ws) setFocus clearEnterEvents hunk ./Operations.hs 54
+full :: Layout +full = Layout { doLayout = \sc -> map (\w -> (w,sc)), modifyLayout = const Nothing }
If I'm reading this correctly, I don't think that will work. "full" doesn't display the first window in the tiling order, it displays the focused window. So Rectangle -> [Window] -> [(Window, Rectangle)] is not sufficient to support all the cases we have now: The layout function has to somehow know which window is focus. To that end, I think the layout function type is going to have to be something like Rectangle -> Workspace -> [(Window, Rectangle)] ...where "Workspace" is a magical, as-yet-non-existent datatype that contains a list of windows, which one is focused and the layout description. (Possibly other stuff; that's all I can think of ATM.) And of course StackSet would have to be modified to use "Workspace". In that scenario, I'm not quite sure where you'd want to parameterize your user-defined message type. Indeed, I'm not quite sure what to do about that in the first place. If the whole X monad takes this type, doesn't that mean that you made up a totally new type, you couldn't mix and match your layout functions with the builtin ones? Jason Creighton

On Wed, Apr 18, 2007 at 07:06:11PM -0600, Jason Creighton wrote:
On Wed, Apr 18, 2007 at 09:05:56AM -0700, David Roundy wrote:
hunk ./Operations.hs 49 - case layoutType fl of - Full -> fmap (flip (,) sc) $ maybeToList $ W.peekStack n ws - Tall -> tile (tileFraction fl) sc $ W.index n ws - Wide -> vtile (tileFraction fl) sc $ W.index n ws + (doLayout l) sc $ W.index n ws whenJust (W.peekStack n ws) (io . raiseWindow d) whenJust (W.peek ws) setFocus clearEnterEvents hunk ./Operations.hs 54
+full :: Layout +full = Layout { doLayout = \sc -> map (\w -> (w,sc)), modifyLayout = const Nothing }
If I'm reading this correctly, I don't think that will work. "full" doesn't display the first window in the tiling order, it displays the focused window. So Rectangle -> [Window] -> [(Window, Rectangle)] is not sufficient to support all the cases we have now: The layout function has to somehow know which window is focus. To that end, I think the layout function type is going to have to be something like
I can assure you that full does work (I just double-checked), although I couldn't really explain why--except experimentally. (By which I mean that it displays the focussed window, and no other.) Perhaps the focussed window is always displayed on top of the other windows when they overlap?
Rectangle -> Workspace -> [(Window, Rectangle)]
...where "Workspace" is a magical, as-yet-non-existent datatype that contains a list of windows, which one is focused and the layout description. (Possibly other stuff; that's all I can think of ATM.) And of course StackSet would have to be modified to use "Workspace".
We certainly could generalize, but I don't think we need a new datatype for this. As far as I can see, all we would need to add is an index to the focussed window, since that's all the information that is stored. I'm not sure what you mean by the "layout description". My idea (as exemplified in this patch) is that you don't need a layout description, and I don't want one--assuming you mean something like a data type.
In that scenario, I'm not quite sure where you'd want to parameterize your user-defined message type. Indeed, I'm not quite sure what to do about that in the first place. If the whole X monad takes this type, doesn't that mean that you made up a totally new type, you couldn't mix and match your layout functions with the builtin ones?
In the scenario I'm imagining, layout functions would define a class describing the messages they accept, and the Config file would define a data type that is an instance of all those classes. Thus we can have any combination of layouts, which can each accept any number of messages. The Config file needs to "know" the details of the messages in order to set up key bindings anyhow. In practice, there probably won't be very many such classes, since most layouts will accept similar commands. -- David Roundy http://www.darcs.net

On Wed, Apr 18, 2007 at 09:27:03PM -0700, David Roundy wrote:
On Wed, Apr 18, 2007 at 07:06:11PM -0600, Jason Creighton wrote:
On Wed, Apr 18, 2007 at 09:05:56AM -0700, David Roundy wrote:
hunk ./Operations.hs 49 - case layoutType fl of - Full -> fmap (flip (,) sc) $ maybeToList $ W.peekStack n ws - Tall -> tile (tileFraction fl) sc $ W.index n ws - Wide -> vtile (tileFraction fl) sc $ W.index n ws + (doLayout l) sc $ W.index n ws whenJust (W.peekStack n ws) (io . raiseWindow d) whenJust (W.peek ws) setFocus clearEnterEvents hunk ./Operations.hs 54
+full :: Layout +full = Layout { doLayout = \sc -> map (\w -> (w,sc)), modifyLayout = const Nothing }
If I'm reading this correctly, I don't think that will work. "full" doesn't display the first window in the tiling order, it displays the focused window. So Rectangle -> [Window] -> [(Window, Rectangle)] is not sufficient to support all the cases we have now: The layout function has to somehow know which window is focus. To that end, I think the layout function type is going to have to be something like
I can assure you that full does work (I just double-checked), although I couldn't really explain why--except experimentally. (By which I mean that it displays the focussed window, and no other.) Perhaps the focussed window is always displayed on top of the other windows when they overlap?
Oh, right. "refresh" raises the focused window. So it works, but by accident. Maybe that's not a fair way of putting it. As xmonad is currently written, the focused window has to be raised because we don't move the other windows out of the way. So while it seems kludgey to me, it will work, as you have noted. But it also seems distasteful to not be able to, for example, check all the normal properties (non-overlapping windows among them, I believe) with QC. So my opinion is that it would be better to pass in the focused window, as you propose below.
Rectangle -> Workspace -> [(Window, Rectangle)]
...where "Workspace" is a magical, as-yet-non-existent datatype that contains a list of windows, which one is focused and the layout description. (Possibly other stuff; that's all I can think of ATM.) And of course StackSet would have to be modified to use "Workspace".
We certainly could generalize, but I don't think we need a new datatype for this. As far as I can see, all we would need to add is an index to the focussed window, since that's all the information that is stored.
*nods*
I'm not sure what you mean by the "layout description". My idea (as exemplified in this patch) is that you don't need a layout description, and I don't want one--assuming you mean something like a data type.
Well, I was thinking of the LayoutDesc type. (Or, in your patch, the Layout type.) Specifically, I was thinking that all the properties of a workspace (The window list, the focused index, and whatever it needs to know about layout) would be in one datatype, and then we wouldn't need three seperate maps (layoutDescs in XMonad, stacks in StackSet and focus in StackSet) to keep track of workspaces. Now I'm not so sure; maybe the layout stuff should be separate from StackSet anyway. But in any case, any sort of refactor like that is unrelated to your change and isn't something we need to think about right this second.
In that scenario, I'm not quite sure where you'd want to parameterize your user-defined message type. Indeed, I'm not quite sure what to do about that in the first place. If the whole X monad takes this type, doesn't that mean that you made up a totally new type, you couldn't mix and match your layout functions with the builtin ones?
In the scenario I'm imagining, layout functions would define a class describing the messages they accept, and the Config file would define a data type that is an instance of all those classes. Thus we can have any combination of layouts, which can each accept any number of messages. The Config file needs to "know" the details of the messages in order to set up key bindings anyhow.
In practice, there probably won't be very many such classes, since most layouts will accept similar commands.
Hmm. What advantage does using classes have over just having a data type with multiple constructors, and have layout functions ignore messages (ie, constructors) they don't know about? Jason Creighton

On Thu, Apr 19, 2007 at 07:10:36PM -0600, Jason Creighton wrote:
On Wed, Apr 18, 2007 at 09:27:03PM -0700, David Roundy wrote:
On Wed, Apr 18, 2007 at 07:06:11PM -0600, Jason Creighton wrote:
If I'm reading this correctly, I don't think that will work. "full" doesn't display the first window in the tiling order, it displays the focused window. So Rectangle -> [Window] -> [(Window, Rectangle)] is not sufficient to support all the cases we have now: The layout function has to somehow know which window is focus. To that end, I think the layout function type is going to have to be something like
I can assure you that full does work (I just double-checked), although I couldn't really explain why--except experimentally. (By which I mean that it displays the focussed window, and no other.) Perhaps the focussed window is always displayed on top of the other windows when they overlap?
Oh, right. "refresh" raises the focused window. So it works, but by accident.
Maybe that's not a fair way of putting it. As xmonad is currently written, the focused window has to be raised because we don't move the other windows out of the way. So while it seems kludgey to me, it will work, as you have noted. But it also seems distasteful to not be able to, for example, check all the normal properties (non-overlapping windows among them, I believe) with QC. So my opinion is that it would be better to pass in the focused window, as you propose below.
I agree. It'll be cleaner to pass in the focussed window. It's also helpful for certain sorts of layouts I can imagine, where the focussed window is treated specially (e.g. if I had a keybinding to make the focussed window a bit wider).
In that scenario, I'm not quite sure where you'd want to parameterize your user-defined message type. Indeed, I'm not quite sure what to do about that in the first place. If the whole X monad takes this type, doesn't that mean that you made up a totally new type, you couldn't mix and match your layout functions with the builtin ones?
In the scenario I'm imagining, layout functions would define a class describing the messages they accept, and the Config file would define a data type that is an instance of all those classes. Thus we can have any combination of layouts, which can each accept any number of messages. The Config file needs to "know" the details of the messages in order to set up key bindings anyhow.
In practice, there probably won't be very many such classes, since most layouts will accept similar commands.
Hmm. What advantage does using classes have over just having a data type with multiple constructors, and have layout functions ignore messages (ie, constructors) they don't know about?
The advantage is that new layouts can be developed that desire new sorts of messages without modifying all the other layouts. You could also acheive this by having a standardized name for the message datatype and adding an additional config file that is imported into Operations, but that seems kludgey. Or if ghc supported mutually recursive modules, that could also make it work. But the class approach seems cleanest to me--I don't like the mutually recursive approach, as it means that a change in Config could cause Operations to fail to compile, which doesn't seem like a friendly interface for people who just want to change their key bindings. -- David Roundy http://www.darcs.net
participants (2)
-
David Roundy
-
Jason Creighton