-
-
Notifications
You must be signed in to change notification settings - Fork 278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
X.H.RescreenHook: New module (custom rescreen and randr event hooks) #460
Conversation
Ping @fis. Could you perhaps test your Autorandr with these hooks? I'm especially interested if my "debouncing" logic works, as it's possibly less robust (but also less hacky) than yours and doesn't use sleep. :-) |
Somewhat related: liskin/xmonad@839d0d9 That xmonad commit also needs xmonad/X11#72, which is otherwise not required by this PR, but should be merged sooner or later anyway. |
Heh. It may take me a day or three to actually get around to it, but I can definitely try to test my AutoRandr code on top of these hooks. Regarding the debouncing logic, some context: my setup involves two monitors connected over a single (USB-C) cable with DisplayPort MST. If memory serves, to X plugging in the cable looks like two consecutive (but distinct) configuration changes: first a single monitor appears at the "DP-0" output, and then that changes into a DP "hub", with two monitors connected as "DP-0-1" and "DP-0-8". I think I added the sleep-based debouncing as an initial attempt to work around that. That said, I don't think it really was stable enough (the timing is unpredictable), and I ended up instead just not configuring any action for the "single external monitor" configuration. So it's quite possible just clearing the actually immediate X events would work well enough (for my purposes). |
Absolutely no need to rush it! :-) I also have a DP MST hub here (Lenovo ThinkPad Ultra Dock) and I had all sorts of problems with it in the past, but right now it seems to work rather well. Your setup is probably different, though, as I guess one of your monitors acts as the MST hub, but it really could look like a single monitor for a brief moment. Not sure if this is something we can reliably handle in xmonad. We could probably add some configurable sleep to the event clearing or something to make sure the hook really fires only once it's settled. Or we can leave it up to users to add sleep and some actual debouncing to the hook if they need that. In my own config, I added flock to my bash layout-auto: liskin/dotfiles@706db34#diff-c57c136cf207a3cd44f4fac23bde2146a5f8259c4391333981aec1a8aba66898 and now I can just freely add sleeps if they ever become needed. And my configuration also won't reconfigure xrandr for unknown monitor configuration, I still need to manually press a key for that. |
-- | Remove all X events of a given window and type from the event queue, | ||
-- return whether there were any. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really cool; not only in the context of this module but in general even. Do we have a place where these kinds of "XUtils" (sadly XMonad.Util.XUtils
does not seem to be this place) can go?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not currently. Sadly evicting and reclaiming XMonad.Util.XUtils
would be a breaking change; but it's frankly misnamed.
That said, maybe it deserves to be in XMonad.Util.EvUtils
(this doesn't exist yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with XMonad.Util.XUtils
?
On the other hand: should I really try to move a function that nobody needed for a decade somewhere where it's only slightly easier to find than where it is? After all, if anyone opens a PR that adds a similar function for their module, surely I'll notice and suggest factoring it out then. :-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Anyway, if we ever make the function available, it should come with some warnings as it may change the order of events. It does put them back in the correct order relative to one another, but the order is changed relative to events that weren't picked up by XCheckTypedWindowEvent
. It is extremely unlikely to matter in this usecase, but with something else than ConfigureEvent
, it may matter.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's wrong with
XMonad.Util.XUtils
?
The module description says "A module for painting on the screen" and that's pretty much what all of the functions do in some capacity; not really a place for general X utility functions. Then again this is only there as a doc, so we may well repurpose it and give all the drawing stuff a heading of its own.
On the other hand: should I really try to move a function that nobody needed for a decade somewhere where it's only slightly easier to find than where it is? After all, if anyone opens a PR that adds a similar function for their module, surely I'll notice and suggest factoring it out then. :-)
I'll give you that one; probably a bit of a premature optimization, especially since no good module already exists where we could put it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The module description says "A module for painting on the screen" and that's pretty much what all of the functions do in some capacity; not really a place for general X utility functions.
There are also a couple of X wrappers for IO functions with Display parameter. We can move those under a separate haddock heading and then add another heading for utilities (which I define as more than a wrapper) like my clearTypedWindowEvents. None of that needs to be done now, but I think this is the most sensible thing to do if we want a module for X utils.
It's often difficult to make contrib modules work together. When one depends on a functionality of another, it is often necessary to expose lots of low-level functions and hooks and have the user combine these into a complex configuration that works. This is error-prone, and arguably a bad UX in general. This commit presents a simple solution to that problem inspired by "extensible state": extensible config. It allows contrib modules to store custom configuration values inside XConfig. This lets them create custom hooks, ensure they hook into xmonad core only once, and possibly other use cases I haven't thought of yet. This requires changes to xmonad core: xmonad/xmonad#294 A couple examples of what this gives us: * [X.H.RescreenHook](xmonad#460) can be made safe to apply multiple times, making it composable and usable in other contrib modules like X.H.StatusBar * `withSB` from X.H.StatusBar can also be made safe to apply multiple times, and we can even provide an API [similar to what we had before](https://hackage.haskell.org/package/xmonad-contrib-0.16/docs/XMonad-Hooks-DynamicLog.html#v:statusBar) if we want (probably not, consistency with the new dynamic status bars of xmonad#463 is more important) * The [X.H.EwmhDesktops refactor](xmonad#399) can possibly be made without breaking the `ewmh`/`ewmhFullscreen` API. And we will finally be able to have composable EWMH hooks. Related: xmonad/xmonad#294
It's often difficult to make contrib modules work together. When one depends on a functionality of another, it is often necessary to expose lots of low-level functions and hooks and have the user combine these into a complex configuration that works. This is error-prone, and arguably a bad UX in general. This commit presents a simple solution to that problem inspired by "extensible state": extensible config. It allows contrib modules to store custom configuration values inside XConfig. This lets them create custom hooks, ensure they hook into xmonad core only once, and possibly other use cases I haven't thought of yet. This requires changes to xmonad core: xmonad/xmonad#294 A couple examples of what this gives us: * [X.H.RescreenHook](xmonad#460) can be made safe to apply multiple times, making it composable and usable in other contrib modules like X.H.StatusBar * `withSB` from X.H.StatusBar can also be made safe to apply multiple times, and we can even provide an API [similar to what we had before](https://hackage.haskell.org/package/xmonad-contrib-0.16/docs/XMonad-Hooks-DynamicLog.html#v:statusBar) if we want (probably not, consistency with the new dynamic status bars of xmonad#463 is more important) * The [X.H.EwmhDesktops refactor](xmonad#399) can possibly be made without breaking the `ewmh`/`ewmhFullscreen` API. And we will finally be able to have composable EWMH hooks. Related: xmonad/xmonad#294
It's often difficult to make contrib modules work together. When one depends on a functionality of another, it is often necessary to expose lots of low-level functions and hooks and have the user combine these into a complex configuration that works. This is error-prone, and arguably a bad UX in general. This commit presents a simple solution to that problem inspired by "extensible state": extensible config. It allows contrib modules to store custom configuration values inside XConfig. This lets them create custom hooks, ensure they hook into xmonad core only once, and possibly other use cases I haven't thought of yet. This requires changes to xmonad core: xmonad/xmonad#294 A couple examples of what this gives us: * [X.H.RescreenHook](xmonad#460) can be made safe to apply multiple times, making it composable and usable in other contrib modules like X.H.StatusBar * `withSB` from X.H.StatusBar can also be made safe to apply multiple times, and we can even provide an API [similar to what we had before](https://hackage.haskell.org/package/xmonad-contrib-0.16/docs/XMonad-Hooks-DynamicLog.html#v:statusBar) if we want (probably not, consistency with the new dynamic status bars of xmonad#463 is more important) * The [X.H.EwmhDesktops refactor](xmonad#399) can possibly be made without breaking the `ewmh`/`ewmhFullscreen` API. And we will finally be able to have composable EWMH hooks. Related: xmonad/xmonad#294
It's often difficult to make contrib modules work together. When one depends on a functionality of another, it is often necessary to expose lots of low-level functions and hooks and have the user combine these into a complex configuration that works. This is error-prone, and arguably a bad UX in general. This commit presents a simple solution to that problem inspired by "extensible state": extensible config. It allows contrib modules to store custom configuration values inside XConfig. This lets them create custom hooks, ensure they hook into xmonad core only once, and possibly other use cases I haven't thought of yet. This requires changes to xmonad core: xmonad/xmonad#294 A couple examples of what this gives us: * [X.H.RescreenHook](xmonad#460) can be made safe to apply multiple times, making it composable and usable in other contrib modules like X.H.StatusBar * `withSB` from X.H.StatusBar can also be made safe to apply multiple times, and we can even provide an API [similar to what we had before](https://hackage.haskell.org/package/xmonad-contrib-0.16/docs/XMonad-Hooks-DynamicLog.html#v:statusBar) if we want (probably not, consistency with the new dynamic status bars of xmonad#463 is more important) * The [X.H.EwmhDesktops refactor](xmonad#399) can possibly be made without breaking the `ewmh`/`ewmhFullscreen` API. And we will finally be able to have composable EWMH hooks. Related: xmonad/xmonad#294
Now that randrChangeHook is only invoked for changes that don't result in rescreen, it can actually be used for autorandr.
Two things are still missing here:
Even without these, it should be useful for many people and should be good enough to be used by default in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working beautifully with X.H.StatusBar! Good job
I really like the safety that the ExtensibleConfig gives us. LGTM
This appears to be more natural. The function will most often be fixed by the module using `XC.once` and the configuration will often be supplied by users of those modules, so it's better to partially apply the function first.
…reenChangeNotifyEvent The new X.H.Rescreen provides a simpler composable API for hooking into rescreen. Unfortunately it also breaks other code that listens for RRScreenChangeNotifyEvent, so this conversion isn't really optional. Not that there's any dispute this is nicer, is there? :-)
Description
Inspired by
https://github.com/fis/dot-xmonad/blob/a8c8e804873f8657fc1f1a0d93ee3f58f7a56e22/zem/src/Zem/AutoRandr.hs
and my own
https://github.com/liskin/dotfiles/blob/43999573d006311292a9fadde02f3b234140d999/.xmonad/xmonad.hs#L308-L339,
this adds a generic hook module that allows attaching a custom hook to
rescreen and randr screen change notify event, hiding the complexity of
suppressing duplicate events (debouncing) and distinguishing actual
xrandr configuration changes from output connects/disconnects.
Commits
Add X.H.RescreenHook module (custom rescreen hooks)
X.H.{EwmhDesktops,ManageDocks}: Improve the usage doc a bit
Don't assume ewmh/docks are the only xmonad config combinator out there.
X.H.RescreenHook: Add randrHook
X.H.RescreenHook: Merge the two hooks together and improve their behaviour
Now that randrChangeHook is only invoked for changes that don't result
in rescreen, it can actually be used for autorandr.
Update CHANGES, X.D.Extending: add X.H.Rescreen
Checklist
I've read CONTRIBUTING.md
I updated the
CHANGES.md
fileI updated the
XMonad.Doc.Extending
file (if appropriate)