Skip to content
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

EWMH: Improve interface for custom sorting, filtering, renaming and window activation #626

Merged
merged 10 commits into from Oct 20, 2021

Conversation

liskin
Copy link
Member

@liskin liskin commented Oct 18, 2021

Description

Work in progress on #396. “#399 done right but quick.”

Yesterday's #625 is the glorious future, but it's too big a leap at this point in the release cycle. #399 is the ugly past with an ugly interface and no docs. This one is a pragmatic compromise: (mostly) backwards compatible interface extended with just enough ExtensibleConf bits to fix the problems at hand.

Supersedes #399.
Fixes #396.

TODO:

  • documentation updates for workspace filtering (scratchpads, etc.)
  • ExtensibleConf configuration for activateHook
  • documentation updates for window activation (examples)
  • documentation updates for X.H.Focus
  • (optional) mark individual hooks in EwmhDesktops deprecated but don't trigger deprecation warnings during build of xmonad-contrib itself
  • CHANGES.md
  • review
  • 🚀 merge

And that's it! Fullscreen hooks are not critical and will be done post-release in #625 or a followup thereof.

Checklist

  • I've read CONTRIBUTING.md

  • I've considered how to best test these changes (property, unit, manually, ...) and concluded: the plan is to try to migrate my xmonad config that makes use of most relevant features and hope any bugs are apparent; I'm not expecting any, this should primarily be a change in interface, not in behaviour; the only risk is in the X.H.Focus module, which I'm afraid will need to be accepted

  • I updated the CHANGES.md file: existing commits update CHANGES.md, so this part is ready for review

  • n/a I updated the XMonad.Doc.Extending file (if appropriate)

@liskin liskin added this to In progress in xmonad 0.17 via automation Oct 18, 2021
@liskin liskin added this to the v0.17 milestone Oct 18, 2021
@liskin liskin mentioned this pull request Oct 18, 2021
4 tasks
@liskin liskin mentioned this pull request Oct 19, 2021
4 tasks
@TheMC47
Copy link
Member

TheMC47 commented Oct 19, 2021

I like the pragmatic approach 👍 Good job!
I can't really comment on the implementation itself (it's outside of what I know), but I'll try to test it soon and comment on the docs (when they're ready)

This better matches the documentation.

It is still, however, considered bad practice to rely on the order of
these operations. `f` isn't meant to touch any extensible configuration.
If it happens to do so anyway, it no longer loops. :-)
@liskin
Copy link
Member Author

liskin commented Oct 20, 2021

Just one last missing TODO: X.H.Focus docs, so marking as ready to review and will hopefully finish that tomorrow morning.

@liskin liskin changed the title (wip) EWMH: Improve interface for custom sorting, filtering, renaming and window activation EWMH: Improve interface for custom sorting, filtering, renaming and window activation Oct 20, 2021
Copy link
Member

@slotThe slotThe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also not an expert on EWMH, but didn't notice anything wildly off; this approach looks pretty good to me 👍

XMonad/Hooks/EwmhDesktops.hs Outdated Show resolved Hide resolved
XMonad/Hooks/EwmhDesktops.hs Outdated Show resolved Hide resolved
XMonad/Hooks/EwmhDesktops.hs Outdated Show resolved Hide resolved
XMonad/Hooks/EwmhDesktops.hs Outdated Show resolved Hide resolved
XMonad/Hooks/EwmhDesktops.hs Outdated Show resolved Hide resolved
xmonad 0.17 automation moved this from In progress to To merge Oct 20, 2021
…ult types

For configuration values that don't compose well using a Semigroup
instance, provide a high-level API allowing arbitrary modification of
the value, taking its Default if absent. This API is only usable for
separate configuration data and cannot be used to guard addition of hook
using `once`.
These are useful when one blocks some _NET_ACTIVE_WINDOW requests but
still wants to somehow show that a window requested focus.

Related: xmonad#110
Related: xmonad#128
Related: xmonad#192
…tering and renaming

Now that we have `XMonad.Util.ExtensibleConf`, users can comfortably use
the `ewmh` combinator and still customize workspace ordering, filter out
scratchpads and expose altered workspace names.

To make this all work nicely, we introduce not one, but two
configuration options: a sort/filter function and a rename function.
This is because renaming and sorting in one go makes it hard (perhaps
even impossible) to decide which workspace to switch to upon receipt of
a _NET_CURRENT_DESKTOP request from a pager or wmctrl/xdotool. (The only
reason this wasn't a problem before is because one could pass the
renaming function to `ewmhDesktopsLogHookCustom` only, not
`ewmhDesktopsEventHookCustom`, which is a confusing hack as can be seen
in the related closed pull requests.)

Related: xmonad#238
Related: xmonad#105
Related: xmonad#122
Related: xmonad#105
Related: xmonad#122
Related: f271d59 ("X.A.WorkspaceNames: Provide workspaceListTransform for EwmhDesktops")
xmonad#192 introduced a breaking change:

  * `XMonad.Hooks.EwmhDesktops`

    `ewmh` function will use `logHook` for handling activated window. And now
    by default window activation will do nothing.

This breaking change can be avoided if we designed that a bit
differently. xmonad#192 changed `ewmhDesktopsEventHook` to invoke `logHook`
instead of focusing the window that requested activation and now
`logHook` is supposed to invoke a `ManageHook` through `activateLogHook`
which consults a global `NetActivated` extensible state to tell if it's
being invoked from `ewmhDesktopsEventHook`. This seems convoluted to me.

A better design, in my opinion, is to invoke the `ManageHook` directly
from `ewmhDesktopsEventHook`, and we just need a way to configure the
hook. Luckily, we now have `X.U.ExtensibleConf` which makes this
straightforward. So we now have a `setEwmhActivateHook`, and the
activation hook defaults to focusing the window, undoing the breaking
change.

Fixes: xmonad#396
Related: xmonad#110
Related: xmonad#192
Related: xmonad#128
We should get rid of this error-prone interface ASAP, so mark it as
deprecated to give people some time to adapt their configs.
@liskin liskin merged commit e5b5ce7 into xmonad:master Oct 20, 2021
xmonad 0.17 automation moved this from To merge to Done Oct 20, 2021
@liskin liskin deleted the ewmh-refactor-quick branch October 20, 2021 14:58
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Oct 23, 2022
Fairly straightforward, just add two hooks for (un)fullscreening. There
are multiple motivations for this:

* Users are calling for unfullscreened windows to revert back to their
  original location if they were floating.

* XMonad.Layout.Fullscreen uses some deprecated exports from
  XMonad.Hooks.EwmhDesktops and reimplements fullscreenEventHook.

This commit only adds the hooks. Neither of the motivations are dealt
with yet, so the docs are a bit terse still.

Related: xmonad#456
Related: xmonad#394
Related: xmonad#626
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Mar 25, 2023
Fairly straightforward, just add two hooks for (un)fullscreening. There
are multiple motivations for this:

* Users are calling for unfullscreened windows to revert back to their
  original location if they were floating.

* XMonad.Layout.Fullscreen uses some deprecated exports from
  XMonad.Hooks.EwmhDesktops and reimplements fullscreenEventHook.

This commit only adds the hooks. Neither of the motivations are dealt
with yet, so the docs are a bit terse still.

Related: xmonad#456
Related: xmonad#394
Related: xmonad#626
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Mar 25, 2023
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Mar 26, 2023
Fairly straightforward, just add two hooks for (un)fullscreening. There
are multiple motivations for this:

* Users are calling for unfullscreened windows to revert back to their
  original location if they were floating.

* XMonad.Layout.Fullscreen uses some deprecated exports from
  XMonad.Hooks.EwmhDesktops and reimplements fullscreenEventHook.

This commit only adds the hooks. Neither of the motivations are dealt
with yet, so the docs are a bit terse still.

Related: xmonad#456
Related: xmonad#394
Related: xmonad#626
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Mar 26, 2023
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Apr 1, 2023
Fairly straightforward, just add two hooks for (un)fullscreening. There
are multiple motivations for this:

* Users are calling for unfullscreened windows to revert back to their
  original location if they were floating.

* XMonad.Layout.Fullscreen uses some deprecated exports from
  XMonad.Hooks.EwmhDesktops and reimplements fullscreenEventHook.

This commit only adds the hooks. Neither of the motivations are dealt
with yet, so the docs are a bit terse still.

Related: xmonad#456
Related: xmonad#394
Related: xmonad#626
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Apr 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Breaking change in EWMH window activation
3 participants