XMonadContrib patch review

It's that time of the month - time for all lonely chillun' patches to gather close to the dying fire and eke out what comfort they can from the modicum of attention I can spare; the night is long, and the huddled patches so lonesome. 'Love me, read me!' they say. (I am sorry, little ones, I can but look at your status and move on.)
Today we'll be reviewing XMC: http://darcswatch.nomeata.de/repo_http:__code.haskell.org_XMonadContrib.html which has 11 patches outstanding, including 2 veterans of this Missing Patches bulletin from 2008:
* "XMonad.Actions.DynamicKeys: utilities to update key bindings at runtime"
http://www.haskell.org/pipermail/xmonad/2008-July/006041.html
Applicable. No comments. Requires the patch to XMonad core discussed previously.
20081019140330: FocusNth improvements fixed by Aleksey Artamonov

20090618115854: XMonad.Actions.Volume for controlling and grokking amixer within xmonad by daniel@wagner-home.com 20090618125959: Add helper functions for displaying volume via osd_cat by Daniel Schoepe
http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7896 Applies cleanly. There were no comments.
There was some concern on the mailing list about the dependencies for these. Daniel and I have made these patches (and some others which also have bad dependency chains) available via darcs from http://code.haskell.org/xmonad-extras. Though, of course, I wouldn't object if these got applied to XMC proper. =) ~d

I rechecked that manageDocksConfig patch, it seems fine and the conflict was
a simple export list conflict. Attached is a patch that resolves the
conflict against current darcs.
If I remember there was some debate in #xmonad whether this patch was useful
and/or the Right Thing, so I didn't push it directly, but rather sent it to
the list for comments. It received none, so we're back to square one. Even
I'm not convinced it's useful, this is closer to a RFC than a request for
application of the patch.
Braden Shepherdson
shepheb
On Sat, Aug 1, 2009 at 11:40 AM,
It's that time of the month - time for all lonely chillun' patches to gather close to the dying fire and eke out what comfort they can from the modicum of attention I can spare; the night is long, and the huddled patches so lonesome. 'Love me, read me!' they say. (I am sorry, little ones, I can but look at your status and move on.)
Today we'll be reviewing XMC: http://darcswatch.nomeata.de/repo_http:__code.haskell.org_XMonadContrib.html... has 11 patches outstanding, including 2 veterans of this Missing Patches bulletin from 2008:
* "XMonad.Actions.DynamicKeys: utilities to update key bindings at runtime" http://www.haskell.org/pipermail/xmonad/2008-July/006041.html Applicable. No comments. Requires the patch to XMonad core discussed previously.
20081019140330: FocusNth improvements fixed by Aleksey Artamonov < aleksey.artamonov@gmail.com> http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/6320 Not currently applicable; but this seems to be because of unrelated changes to the FocusNth module. The one comment by Wirt Wolff on it was favorable, and the maintainer has not commented on it for the last 23 weeks (at least).
20090128004726: NoWrap export patch for use with X.L.MessageControl by quentin.moser@unifr.ch Adam Vogt remarked 14 April that "These apply cleanly, and seem useful enough." Since noone has objected, and the patch has been extant for quite some time, it applies cleanly/compiles/passes tests, doesn't seem obviously wrong to a layout dunce like myself, and does not affect other modules, I have taken it upon myself to push this into the main XMC repo. Consider this due notification!
20090315001809: X.H.ManageDocks: Added manageDocksConfig by Braden Shepherdson
Doesn't apply cleanly. No comments. 20090428192731: NS_Placement by konstantin.sobolev@gmail.com20090428200136: ScratchpadRewrite by konstantin.sobolev@gmail.com http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7592 Applies cleanly. Got bogged down in bikeshedding; I'm tempted to apply it and let Wogt or Wolff code up some patches if they really care about long identifiers and 2 modules rather than one.
20090503195026: ThreeColMid - Swap slave window positions by Anders Engstrom http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7629< ankaan@gmail.com> Applies cleanly. Adam Vogt didn't seem too keen on this patch, though, and I can't tell whether Anders is agreeing it's unnecessary or what.
20090618115854: XMonad.Actions.Volume for controlling and grokking amixer within xmonad by daniel@wagner-home.com 20090618125959: Add helper functions for displaying volume via osd_cat by Daniel Schoepe
http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7896 Applies cleanly. There were no comments. So that makes 4 or 5 patches which could be applied; and 2 which are broken. I've cc'd relevant people. Let's get some comments on the clean ones!
-- gwern

On Sat, Aug 01, 2009 at 07:40:01AM -0400, gwern0@gmail.com wrote:
20090618115854: XMonad.Actions.Volume for controlling and grokking amixer within xmonad by daniel@wagner-home.com 20090618125959: Add helper functions for displaying volume via osd_cat by Daniel Schoepe
http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7896 Applies cleanly. There were no comments.
I think the decision was not to apply these since they pull in a Parsec dependency. The Volume module (and a few other modules) now reside in xmonad-extras (code.haskell.org/xmonad-extras). -Brent

On Sat, Aug 01, 2009 at 11:57:25PM -0400, Brent Yorgey wrote:
On Sat, Aug 01, 2009 at 07:40:01AM -0400, gwern0@gmail.com wrote:
20090618115854: XMonad.Actions.Volume for controlling and grokking amixer within xmonad by daniel@wagner-home.com 20090618125959: Add helper functions for displaying volume via osd_cat by Daniel Schoepe
http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7896 Applies cleanly. There were no comments. I think the decision was not to apply these since they pull in a Parsec dependency. The Volume module (and a few other modules) now reside in xmonad-extras (code.haskell.org/xmonad-extras).
Ah, whoops, I see Mr. Wagner already responded. And on second thought it looks like there actually wasn't ever a decision, so my pronouncement above shouldn't be taken as one. If others think this should go in and the Parsec dependency isn't a big deal, then fine. I am ambivalent (not indifferent, mind you; I feel strongly both ways =). -Brent

* On Saturday, August 01 2009, gwern0@gmail.com wrote:
20090428192731: NS_Placement by konstantin.sobolev@gmail.com 20090428200136: ScratchpadRewrite by konstantin.sobolev@gmail.com http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7592 Applies cleanly. Got bogged down in bikeshedding; I'm tempted to apply it and let Wogt or Wolff code up some patches if they really care about long identifiers and 2 modules rather than one.
Those suggestions were pretty minor, and contrib is already written in a few different styles anyways, so I've pushed these as-is.
20090503195026: ThreeColMid - Swap slave window positions by Anders Engstrom http://thread.gmane.org/gmane.comp.lang.haskell.xmonad/7629
Applies cleanly. Adam Vogt didn't seem too keen on this patch, though, and I can't tell whether Anders is agreeing it's unnecessary or what.
I've gone ahead and pushed this one: the change is pretty subtle, and I doubt that the change is going disrupt many configs. (ThreeColMid is a new module this year) -- Adam Vogt
participants (5)
-
Adam Vogt
-
Braden Shepherdson
-
Brent Yorgey
-
gwern0@gmail.com
-
wagnerdm@seas.upenn.edu