darcs patch: X.A.TopicSpace: new viewMethod field

I've attached a small patch which adds a 'viewMethod' field to TopicSpace configs, allowing the user to choose what method (usually view or greedyView) is used to view workspaces. This will not break any configs (except for those silly enough to use an explicit TopicConfig constructor instead of overriding fields in the default config record) since the default is the same as the old behavior. -Brent

* On Monday, December 28 2009, Brent Yorgey wrote:
I've attached a small patch which adds a 'viewMethod' field to TopicSpace configs, allowing the user to choose what method (usually view or greedyView) is used to view workspaces. This will not break any configs (except for those silly enough to use an explicit TopicConfig constructor instead of overriding fields in the default config record) since the default is the same as the old behavior.
-Brent
Unfortunately, the change to supply a default TopicConfig was added shortly after 0.9. While I can see cases where adding additional fields in the TopicConfig record are justified, your change would be just as easily accomplished by exporting a version of switchTopic parameterized by that function, while accomplishing less breakage:
switchTopic' :: (WorkspaceId -> WindowSet -> WindowSet) -> TopicConfig -> Topic -> X ()
Do you mind sending an updated patch? -- Adam

On Mon, Dec 28, 2009 at 09:13:44PM -0500, Adam Vogt wrote:
* On Monday, December 28 2009, Brent Yorgey wrote:
I've attached a small patch which adds a 'viewMethod' field to TopicSpace configs, allowing the user to choose what method (usually view or greedyView) is used to view workspaces. This will not break any configs (except for those silly enough to use an explicit TopicConfig constructor instead of overriding fields in the default config record) since the default is the same as the old behavior.
-Brent
Unfortunately, the change to supply a default TopicConfig was added shortly after 0.9.
While I can see cases where adding additional fields in the TopicConfig record are justified, your change would be just as easily accomplished by exporting a version of switchTopic parameterized by that function, while accomplishing less breakage:
switchTopic' :: (WorkspaceId -> WindowSet -> WindowSet) -> TopicConfig -> Topic -> X ()
Do you mind sending an updated patch?
Hmm, fair enough. This actually isn't QUITE the same, since there are a few other exported functions (switchNthLastFocused, topicActionWithPrompt) which also call switchTopic... so to be complete we'd have to make primed versions of those as well. But since I don't use those functions... meh. I've attached a new patch. -Brent

(2nd send: I have the impression that this mail wasn't well sent) Excerpts from Brent Yorgey's message of Tue Dec 29 17:13:37 +0100 2009:
On Mon, Dec 28, 2009 at 09:13:44PM -0500, Adam Vogt wrote:
* On Monday, December 28 2009, Brent Yorgey wrote:
I've attached a small patch which adds a 'viewMethod' field to TopicSpace configs, allowing the user to choose what method (usually view or greedyView) is used to view workspaces. This will not break any configs (except for those silly enough to use an explicit TopicConfig constructor instead of overriding fields in the default config record) since the default is the same as the old behavior.
-Brent
Unfortunately, the change to supply a default TopicConfig was added shortly after 0.9.
While I can see cases where adding additional fields in the TopicConfig record are justified, your change would be just as easily accomplished by exporting a version of switchTopic parameterized by that function, while accomplishing less breakage:
switchTopic' :: (WorkspaceId -> WindowSet -> WindowSet) -> TopicConfig -> Topic -> X ()
Do you mind sending an updated patch?
Hmm, fair enough. This actually isn't QUITE the same, since there are a few other exported functions (switchNthLastFocused, topicActionWithPrompt) which also call switchTopic... so to be complete we'd have to make primed versions of those as well. But since I don't use those functions... meh. I've attached a new patch.
Right, I lean toward really preferring your original change. What do you think Adam? -- Nicolas Pouillard http://nicolaspouillard.fr

* On Thursday, January 14 2010, Nicolas Pouillard wrote:
(2nd send: I have the impression that this mail wasn't well sent) ...
Right, I lean toward really preferring your original change. What do you think Adam?
I like the original change better, since lots of functions need to be duplicated for my alternative suggestion to be equivalent. While I'd rather not break 0.9 configs, I think my alterate suggestion is probably worse overall for users. -- Adam

(2nd send: I have the impression that this mail wasn't well sent) Excerpts from Adam Vogt's message of Tue Dec 29 03:13:44 +0100 2009:
* On Monday, December 28 2009, Brent Yorgey wrote:
I've attached a small patch which adds a 'viewMethod' field to TopicSpace configs, allowing the user to choose what method (usually view or greedyView) is used to view workspaces. This will not break any configs (except for those silly enough to use an explicit TopicConfig constructor instead of overriding fields in the default config record) since the default is the same as the old behavior.
-Brent
Unfortunately, the change to supply a default TopicConfig was added shortly after 0.9.
While I can see cases where adding additional fields in the TopicConfig record are justified, your change would be just as easily accomplished by exporting a version of switchTopic parameterized by that function, while accomplishing less breakage:
switchTopic' :: (WorkspaceId -> WindowSet -> WindowSet) -> TopicConfig -> Topic -> X ()
Do you mind sending an updated patch?
I'm a bit hesitating on this, I like both changes (I indeed consider the config extension as a minor issue). The switchTopic' one is more lightweight and modular but also more obscure (for the novice). While we're talking of TopicSpace I have a change to share that I would like to get included. Basically the maximal depth becomes visual only, internally all topics are kept, this is especially useful when closing lots of topics, older ones will reappear in the previous topics list. However I have a little concern about the space usage of TopicSpace or maybe this can be caused by other extensions as well. I think I have a space leak, I also think I have cured one (see the `seq` call). This may be related to the extensible state change as well. Best regards, -- Nicolas Pouillard http://nicolaspouillard.fr

This 1-patch bundle was just applied to http://code.haskell.org/XMonadContrib:
20091220212813 Nicolas Pouillard
participants (4)
-
Adam Vogt
-
Brent Yorgey
-
darcswatch@nomeata.de
-
Nicolas Pouillard