X.A.Repeatable: Auto-detect modifier keys from currentEvent

[liskin]

Description

This makes all the cycleSomething key bindings easier to use: users don't need to manually list the KeySyms of modifier keys in addition to specifying the mask for the keybinding itself. We can easily get those KeySyms from the KeyEvent currently being handled with the help of XGetModifierMapping.

This only implements the logic in X.A.Repeatable. We need to decide
whether to change the API of all the user-facing functions (drop the mods
argument), just document that empty list means "auto-detect", or some
combination thereof.

Checklist

• I've read CONTRIBUTING.md

• I've considered how to best test these changes (property, unit,
  manually, ...) and concluded:
  seems to work fine for me…
  automated testing not feasible

• I updated the CHANGES.md file

[liskin]

Anyone happens to have an opinion about the API?

[slotThe]

This sounds much better than the current situation, but I guess breaking user configs by dropping an argument is always a bit "eh" :/

[geekosaur]

New API and mark the old one as deprecated? Or just go with "empty list means get it from the event"?
Also, do we want to think about this for https://hackage.haskell.org/package/xmonad-contrib-0.18.0/docs/XMonad-Actions-CycleWindows.html#v:cycleRecentWindows?

[liskin]

Also, do we want to think about this for https://hackage.haskell.org/package/xmonad-contrib-0.18.0/docs/XMonad-Actions-CycleWindows.html#v:cycleRecentWindows?

X.A.Repeatable is used in X.A.CycleWindows, and also in X.A.{CycleWorkspaceByScreen,CycleRecentWS,MostRecentlyUsed} so changing the API would result in quite a lot of breaking changes, or at least quite a lot of new function names and lot of deprecations.

I guess going with the "empty list …" option is better then. Or something.

[LSLeary]

Re API, the functions in X.A.Repeatable are mostly not used directly in config, but indirectly via client contrib modules.
As such, the mods argument here should just be removed entirely; let the clients handle the change.

[liskin]

Re API, the functions in X.A.Repeatable are mostly not used directly in config, but indirectly via client contrib modules. As such, the mods argument here should just be removed entirely; let the clients handle the change.

Yeah, agreed that changing the API of the repeatable* (and newly also concludable*) family of functions is an acceptable breaking change as few users are expected to touch that API directly.

That still leaves the question of what to do with all the client contrib modules, all of which have the same mods argument, which is passed without modification to repeatable*. That was my main concern earlier, and that's why the PR is draft — potentially all of those client modules (X.A.{CycleWindows,CycleWorkspaceByScreen,CycleRecentWS,MostRecentlyUsed}) will need changing. Either just the documentation (if we agree that empty list means auto-detect, as implemented by the PR currently), or something more complicated.

But I guess there's little point of further bikeshedding and I should just finish the doc updates and add a CHANGES entry… :-)

[LSLeary]

@liskin Those modules / their maintainers should ultimately decide how to handle the change on a case by case basis, but in the meantime I would just have them ignore the mods argument and document that it's unused.

It's not clear to me that there's any reason to make auto-detection optional.