On Sun, Apr 5, 2009 at 8:46 PM, Adam Vogt wrote:

...

Could somebody review my attempt to write a layout and tell if it's worth including in contrib?

I found a couple of issues that should be corrected in the documentation: 1. Refers to XMonad.Utils.WindowProperties, should be XMonad.Util.WindowProperties 2. Class should be ClassName

Fixed. I'm attaching a darcs patch and a screenshot of my IM workspace. All the windows found their placement automatically.

Combo deals with tabbed's requirement that it be told whenever the layout changes (I think that it is due to the ReleaseResources that it sends).

Perhaps the same results would be possible as a LayoutModifier around Combo that places certain windows in the layout at specific spots in the stack, when they have not been seen before (ie. keep track of the windows laid out in the previous run). That way you can avoid dealing with the specifics of dealing with decorated sublayouts.

I don't know how to implement it this way, without any access to Combo's internal state. I could duplicate it's windows management logic and so maintain a mirror of it's state, but I really hate both high coupling and code duplication. To my mind a better idea would be to explicitly specify all the components invariants about applying combinators, processing messages and stack management, like requirement to send ReleaseResources to all the sublayouts etc. I had to spend quite a lot of time reading sources to get some bits of knowledge about it (esp. taking into account my modest Haskell experience).

...

- I see a lot of flickering when switching workspaces or tabs in Tabbed. Looks like root image is shown first, and then windows paint on top of it. It's very noticeable, especially when some windows are slow to redraw.

Tabbed is a relatively slow layout, but even then, I think that most of the redrawing delay can be blamed on the applications being slow to redraw. It seems to be less noticeable when using a black root window colour.

Exactly. I've switched from Ion which always has black background and immediately put some fancy picture in the root window, that's why it was so noticeable to me. I even thought that Ion does some kind of double buffering, but I've just checked - it doesn't, it's only a background colour which makes such a difference. By the way, is it possible to implement this in WM: save old window bitmaps and show them instead of background image while windows are redrawing?

...

- dragPane doesn't work when reflected

The idea of layout combinators looks nice, but overall impression is that different combinations haven't been tested thoroughly.

There many combinations, and its probably true that all combinations have not been tested. I'd say that these issues are due to the layout modifier breaking some invariant that the modified layout assumes to be true: for the dragPane, I'd expect that the mouse handling stuff still assumes that the divider is a vertical one, when it has in fact been changed. Such are the issues of impure layouts; they are harder to test and easier to break by changing the environment.

I meant reflection, I haven't even tried rotation. Reflecting a 1/3 division converts it to 2/3, but dragging the splitter doesn't work anymore.

In any case, if you run across an issue, file a bug and/or notify the maintainers: it could be that such a combination has not been tried before and maybe the solution isn't difficult at all.

OK, I'll file a couple of reports. Thank you.

On Tue, Apr 14, 2009 at 6:38 AM, Roman Cheplyaka wrote:

* Konstantin Sobolev [2009-04-03 17:00:36-0400] There are several wishes to your code: 1. All non-trivial top-level functions should be documented (such as   swap, forwardToFocused etc.)

done

2. If you use some function from a different module (like   'differentiate' from Combo) you shouldn't copy-paste its code;   instead prepare a separate patch which adds that function to export   list and explain where you use that function in the patch   description. Later that function may be also moved to some module in   the Util section.

I got rid of broadcastPrivate, now there's only differentiate left. It's too specific to be used by anything else I think.

  Again, those functions (differentiate and broadcastPrivate from   Combo) are not documented there. Could you please write documentation   for them if you use them?

I've added some short description to differentiate.

3. You shouldn't advise to import XMonad.Util.WindowProperties; instead   you should re-export needed things (or even the whole module) from   your module. See XMonad.Layout.IM for example.

done

Sorry for a late response and thanks for your work!

thank you