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.

Sign up for GitHub

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

Jump to bottom

(wip) Refactor EWMH window activation #399

Closed
wants to merge 8 commits into from
Closed

(wip) Refactor EWMH window activation #399

wants to merge 8 commits into from

Conversation

liskin
Copy link
Member

@liskin liskin commented Nov 5, 2020

Description

Work in progress on #396.

Checklist

  • I've read CONTRIBUTING.md

  • I tested my changes with xmonad-testing

  • I updated the CHANGES.md file

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

@liskin liskin added this to the v0.17 milestone Nov 5, 2020
@liskin liskin requested review from geekosaur and sgf-dma November 5, 2020 00:47
This was referenced Nov 5, 2020
Closed
Closed
Closed
Closed
Closed
Merged
@liskin liskin force-pushed the ewmh-ignoreactivation-managehook branch 4 times, most recently from f861b5b to e28e141 Compare November 16, 2020 11:26
@liskin liskin mentioned this pull request Dec 31, 2020
Merged
3 tasks
@liskin liskin mentioned this pull request Jan 15, 2021
Closed
16 tasks
@liskin liskin force-pushed the ewmh-ignoreactivation-managehook branch from e28e141 to d4235fb Compare January 25, 2021 23:23
@liskin
Copy link
Member Author

liskin commented Jan 25, 2021

(test failures expected, we need to merge xmonad/X11#71 and release X11)

@liskin liskin closed this Jan 26, 2021
@liskin liskin deleted the ewmh-ignoreactivation-managehook branch January 26, 2021 12:04
@liskin liskin restored the ewmh-ignoreactivation-managehook branch January 26, 2021 12:08
@liskin liskin reopened this Jan 26, 2021
@liskin liskin mentioned this pull request Jan 30, 2021
Closed
2 tasks
@liskin liskin force-pushed the ewmh-ignoreactivation-managehook branch from d4235fb to 8f6c8d6 Compare January 30, 2021 12:03
@liskin liskin mentioned this pull request Feb 4, 2021
Closed
4 tasks
@liskin liskin force-pushed the ewmh-ignoreactivation-managehook branch from a0555f8 to dfd44cd Compare February 7, 2021 13:48
@TheMC47
Copy link
Member

TheMC47 commented Mar 1, 2021

I played around with this a bit, and it's definitely an improvement to the UX in my opinion. Awesome work 🎉 I'm not familiar with X.H.Focus or X.A.WorkspaceNames so I didn't get to try them fully (my config is still pretty basic), but I have some suggestions for the documentation of XMonad.Hooks.EwmhDesktops module:

  • The examples in your personal dotfiles are nice, especially the one with activateHook. I'm only a bit confused on why one would need workspaceListSort
  • Regarding fullscreen support:
    • It might be nice to mention why you wouldn't want fullscreen support. I personally don't use it, because I want picture-in-picture support sometimes. So it's maybe a good idea to mention that and refer to windowedFullscreenFixEventHook
    • Since we're making the window float, if the user focuses another window in the same workspace by mistake, they'll have no indication that they're focused on the wrong window. Maybe it makes sense to have a combinator that we can use on windows W.focusUp for example, so it only triggers if no window in the current workspace is in fullscreen mode. It doesn't have to be here, I'm just thinking out loud

liskin added a commit to liskin/xmonad-contrib that referenced this pull request May 17, 2021
@liskin
X.U.ExtensibleConf: New helper module for extensible config
4193231 
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
TheMC47 pushed a commit to TheMC47/xmonad-contrib that referenced this pull request May 21, 2021
@liskin @TheMC47
X.H.{EwmhDesktops,ManageDocks}: Improve the usage doc a bit
bc5c12e 
Don't assume ewmh/docks are the only xmonad config combinator out there.

Related: xmonad#396
Related: xmonad#399
TheMC47 pushed a commit to TheMC47/xmonad-contrib that referenced this pull request May 21, 2021
@liskin @TheMC47
X.{H.EwmhDesktops,L.Fullscreen}: Kill a few warnings
99e8b05 
Related: xmonad#396
Related: xmonad#399
TheMC47 pushed a commit to TheMC47/xmonad-contrib that referenced this pull request May 21, 2021
@liskin @TheMC47
X.H.EwmhDesktops: Clean up "handle"
5f14db5 
Related: xmonad#396
Related: xmonad#399
Related: xmonad#192
liskin added a commit that referenced this pull request May 21, 2021
@liskin
X.H.EwmhDesktops: Avoid some unnecessary refreshes (border flicker)
ebe2a62 
Current version of Steam sends _NET_ACTIVE_WINDOW ClientMessage for
every mouse click which results in a lot of border blinking.

Ignore requests that would result in no change to get rid of the
annoying border flicker that is inevitable with the current
implementation of XMonad.Operations.windows.

(Note that Steam also sends ConfigureRequest events, and these cause
an additional refresh due to the call to `float` when handling the event
in xmonad core. Not sure if worth fixing.)

Related: #371
Related: #399
@liskin
Copy link
Member Author

liskin commented May 21, 2021

I cherry-picked the non-controversial bits into master over the last few days. I intend to reimplement the rest on top of the extensible config stuff (#547) later.

@sgf-dma
Copy link
Contributor

sgf-dma commented May 21, 2021

In fact, i don't really understand what review you want from me. I have nothing to say about code, because i already forget (again) how all this work. Or you just want, that i try my config on current master?

@liskin
Copy link
Member Author

liskin commented May 21, 2021

@sgf-dma Once this is in a usable state (right now it's a bit behind current developments), I'd welcome if you tried using it in your config. I remember you had some concerns about this being unwieldy for certain use cases. If you don't remember or don't care, though, I'm fine with that, and I'll just try my best not to break legitimate use cases.

tl;dr: I'll let you know once it's ready.

@sgf-dma
Copy link
Contributor

sgf-dma commented May 21, 2021 •
edited
Loading

@liskin Ok, thanks. Well, use cases, which i thought essential (when wrote X.H.Focus), are provided in examples in comments. So, it'd be good, if all examples will work.

liskin added a commit to liskin/xmonad-contrib that referenced this pull request May 21, 2021
@liskin
X.U.ExtensibleConf: New helper module for extensible config
664b694 
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
TheMC47 pushed a commit to TheMC47/xmonad-contrib that referenced this pull request May 31, 2021
@liskin @TheMC47
X.H.EwmhDesktops: Avoid some unnecessary refreshes (border flicker)
ee279c2 
Current version of Steam sends _NET_ACTIVE_WINDOW ClientMessage for
every mouse click which results in a lot of border blinking.

Ignore requests that would result in no change to get rid of the
annoying border flicker that is inevitable with the current
implementation of XMonad.Operations.windows.

(Note that Steam also sends ConfigureRequest events, and these cause
an additional refresh due to the call to `float` when handling the event
in xmonad core. Not sure if worth fixing.)

Related: xmonad#371
Related: xmonad#399
liskin added a commit to liskin/xmonad that referenced this pull request Jun 1, 2021
@liskin
Bump X11 dependency
77710a1 
We have a bunch of open PRs that need features added in X11-1.10.

Related: xmonad#274
Related: xmonad#273
Related: xmonad/xmonad-contrib#545
Related: xmonad/xmonad-contrib#546
Related: xmonad/xmonad-contrib#399
liskin added a commit to xmonad/xmonad that referenced this pull request Jun 1, 2021
@liskin
Bump X11 dependency
1805666 
We have a bunch of open PRs that need features added in X11-1.10.

Related: #274
Related: #273
Related: xmonad/xmonad-contrib#545
Related: xmonad/xmonad-contrib#546
Related: xmonad/xmonad-contrib#399
liskin added a commit to liskin/xmonad-contrib that referenced this pull request Jun 1, 2021
@liskin
X.U.ExtensibleConf: New helper module for extensible config
f6b1e5d 
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
liskin added 8 commits June 4, 2021 17:19
@liskin
(wip) X.H.EwmhDesktops: Introduce EwmhConfig and replace ewmh*HookCus…
034e338 
…tom with it

We need to make EwmhDesktops configurable: not just
workspaceListTransform, but users also need a way to customize the
handling of _NET_ACTIVE_WINDOW, enable/disable fullscreen handling, etc.
Instead of having them manually piece together chains of hooks in their
XConfigs, let's introduce a EwmhConfig record and a `ewmh'` function
that takes care of everything.

Related: xmonad#396
Related: xmonad#109
@liskin
(wip) X.H.EwmhDesktops: activation via activateHook
3c5f31f 
TODO: documentation in X.H.EwmhDesktops
TODO: changelog
TODO: adapt X.H.Focus

Related: xmonad#396
Related: xmonad#110
@liskin
(wip) X.H.EwmhDesktops: Migrate fullscreen handling to EwmhConfig
b645b0e
@liskin
(wip) X.H.EwmhDesktops: Turn workspaceListTransform into X action
1b6c480 
This makes it easier to use transforms that need some state, e.g.
XMonad.Actions.WorkspaceNames could provide this.

Related: xmonad#105
Related: xmonad#122
Related: f271d59 ("X.A.WorkspaceNames: Provide workspaceListTransform for EwmhDesktops")
@liskin
(wip) X.H.UrgencyHook: Add askUrgent and doAskUrgent
11c1aed 
These are useful when one blocks some _NET_ACTIVE_WINDOW requests but
still wants to somehow show that a window requested focus.

TODO: changelog
@liskin
(wip) X.H.EwmhDesktops: Split workspaceListTransform into two (fix wm…
898fe90 
…ctrl -s)

Turns out that renaming workspaces in the transform is a bad idea in the
`ewmhDesktopsEventHook'` as W.view is then unable to find the workspace.
This was somewhat usable before we introduced the unified `ewmh'` config
combinator as one would only rename in the transform passed to
`ewmhDesktopsLogHookCustom`, but with the unified config, we actually
need to separate renames from sorting/reordering, otherwise switching
workspaces by pagers or wmctrl doesn't work.

Related: xmonad#105
Related: xmonad#122
Related: f271d59 ("X.A.WorkspaceNames: Provide workspaceListTransform for EwmhDesktops")
@liskin
(wip)
1589f87
@liskin
(wip) TODOs for dropping workspaceNamesRenameWS
b176837
@liskin liskin force-pushed the ewmh-ignoreactivation-managehook branch from 8f331d4 to b176837 Compare June 4, 2021 16:47
@liskin
Copy link
Member Author

liskin commented Jun 4, 2021 •
edited
Loading

(just a rebase so that I can update my daily driver, nothing exciting yet)

colonelpanic8 pushed a commit to colonelpanic8/xmonad that referenced this pull request Jul 11, 2021
@liskin @colonelpanic8
Bump X11 dependency
6457fe8 
We have a bunch of open PRs that need features added in X11-1.10.

Related: xmonad#274
Related: xmonad#273
Related: xmonad/xmonad-contrib#545
Related: xmonad/xmonad-contrib#546
Related: xmonad/xmonad-contrib#399
This was referenced Oct 17, 2021
Draft
Merged
@liskin
Copy link
Member Author

liskin commented Oct 19, 2021

#626 implements everything that was here, better, with more (but not all yet!) documentation, so this can finally be closed.

@liskin liskin closed this Oct 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geekosaur geekosaur Awaiting requested review from geekosaur

@sgf-dma sgf-dma Awaiting requested review from sgf-dma

Assignees
No one assigned
Labels
needs: documentation
Projects
None yet
Milestone
v0.17.0
Development

Successfully merging this pull request may close these issues.
3 participants
@liskin @TheMC47 @sgf-dma