Here are some minor comments on the code in StackSet.hs, which I hope will be helpful to the authors. Please excuse my extreme anal-retentiveness ;-) In general, the code is very nice and very well-commented. I have some gripes with the comments, though: -- As is pointed out in the comments, what "master" means needs to be spelled out. I use full/tabbed tiling only and I really don't understand the master concept very well, or how it relates to the data structures (especially since it's not explicitly represented in them). -- Polymorphic data types (StackSet, Screen, Workspace) could do with comments specifying what the type parameters refer to. Sometimes it's obvious, sometimes less so. -- The comments are sometimes confusing wrt the complexity metrics. O(n), O(s), O(w) etc. are not always very intuitive in terms of what n, s, or w are supposed to be. Seemingly "s" should be "screen" and "w" should be "workspace" or "window" but it isn't always that clear. For instance, "new" says it's O(n) and then it turns out that "n" is really "m". "index" is O(s) even though it calls "integrate" which is O(n), and the focus/swap functions are O(w) (worst-case) where "w" means "window". -- For "differentiate" the comment "texture a list" is not very illuminating. Other comments: -- I find the way that Screen wraps a workspace somewhat icky, though I can't think of a better alternative. It feels like it compromises the symmetry of the StackSet data structure. Maybe I'm just being too picky here. -- Why do we need the StackOrNot type alias? What's wrong with Maybe (Stack a)? IMO the latter is more readable, or more readable when you use Maybe monad functions. -- I think the "member" function would be simpler as
member :: Eq a => a -> StackSet i a s sd -> Bool member a s = isJust $ findIndex a s
instead of
member a s = maybe False (const True) (findIndex a s)
-- I think the "insertUp" function would read better as
insertUp :: Eq a => a -> StackSet i a s sd -> StackSet i a s sd insertUp a s = if member a s then s else insert s where insert = modify (Just $ Stack a [] []) (\(Stack t l r) -> Just $ Stack a l (t:r))
instead of
insertUp :: Eq a => a -> StackSet i a s sd -> StackSet i a s sd insertUp a s = if member a s then s else insert where insert = modify (Just $ Stack a [] []) (\(Stack t l r) -> Just $ Stack a l (t:r)) s
since in the latter case "insert" looks like a function but isn't. Again, these are tiny, tiny issues. Overall the code is really nice. Mike