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

(wip) Refactor EWMH support #625

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from
Draft

Conversation

liskin
Copy link
Member

@liskin liskin commented Oct 17, 2021

Description

Work in progress on #396 and on #109 (comment) (a blast from the past, but we do have all the necessary infrastructure for an extensible framework now!) and on #396 (comment).

Replaces/supersedes #399, which will be closed once all its functionality is implemented here which will be closed in favor of a conservative temporary implementation based on ExtensibleConf in #626, to be merged before the release, and work on a new modular EWMH support will continue here in #625 after the release.

Related: #396 (not really, just for GitHub metadata)

Checklist

  • I've read CONTRIBUTING.md

  • I've considered how to best test these changes (property, unit, manually, ...) and concluded:

    This will ideally be just a refactor, and we'll be more worried about user experience than code bugs anyway. That being said, I don't expect to even try running the code for a couple days. :-)

  • (TODO) I updated the CHANGES.md file

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

@liskin liskin added this to the v0.17 milestone Oct 17, 2021
@liskin liskin added this to In progress in xmonad 0.17 via automation Oct 17, 2021
@liskin liskin changed the title ### Description (wip) Refactor EWMH support Oct 17, 2021
modify :: forall a l. (Typeable a)
=> (a -> a) -- ^ modification of configuration
-> XConfig l -> XConfig l
modify f c = maybe (trace missing) (const (alter (f <$>))) (lookup @a c) c
Copy link
Member

Choose a reason for hiding this comment

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

I have a very queasy feeling about using trace anywhere outside of a pure debugging environment. How about just making this monadic?

Copy link
Member Author

Choose a reason for hiding this comment

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

There's a TODO to make it print (and show via xmessage) the message through startupHook, but I might drop half of the functions added here instead. I was experimenting with this yesterday, trying to come up with an interface that's good, and probably ended up adding more functions than I'll really need.

Copy link
Member Author

@liskin liskin Oct 18, 2021

Choose a reason for hiding this comment

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

It's also worth noting that if we actually wanted to keep this flavour of modify which requires once to have been used already, it'd be nice if the error/warning could be shown in compile-time. In other words, having the extensibility baked into the XConfig type itself, like the layout type parameter. That's not something we can do with xmonad 0.anything, but should anyone ever rewrite xmonad core from scratch (Wayland, tree-based data structure for layouts/windows, …), it's something to think about. Probably not worth it though.

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. :-)
Sometimes it may be better to provide an interface not based on
Semigroup. Users may need the option to completely override a value; to
choose between prepending/appending, etc.
This is almost functionally equivalent to X.H.EwmhDesktops except for
the logHook window activation (will be replaced by a configurable
activateHook) and full-screen handling (will go into its own module).
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
@liskin
Copy link
Member Author

liskin commented Oct 18, 2021

As mentioned elsewhere, during the implementation of this I realized that everything's in place for a less invasive solution, which is being worked on in #626, so this PR is now moved to the v0.18 milestone, to be finished post-release.

liskin added a commit to liskin/xmonad-contrib that referenced this pull request Oct 22, 2021
This will make it easier to transition to an implementation of EWMH that
doesn't expose the individual hooks: X.H.ManageDocks would become a
deprecated compatibility reexport of X.H.EWMH.Struts for a release or
two, but the individual hooks need to be removed before that.

Note that individual hooks in X.H.EwmhDesktops were deprecated earlier
and individual hooks in XMonad.Hooks.UrgencyHook aren't exported any
more (or perhaps never been), so this only leaves X.H.SetWMName, which
unfortunately does not have a combinator interface at this point.

Related: xmonad#625
@liskin liskin modified the milestones: v0.18.0, v0.19.0 Feb 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

None yet

2 participants