xmonad / xmonad-contrib

Contributed modules for xmonad
https://xmonad.org
BSD 3-Clause "New" or "Revised" License
584 stars 274 forks source link

Deprecations in `XMonad.Hooks.EwmhDesktops` #721

Closed Javran closed 2 years ago

Javran commented 2 years ago

I see those deprecations in https://github.com/xmonad/xmonad-contrib/commit/f666cf4e4e0e433ecb067d2a7466eee1a39c7acf but don't find it convicing - could you elaborate what are error-prone about those APIs? If the plan is just to hide those from user, I can still easily workaround it by passing an empty config to ewmh and have access corresponding fields.

Consider in some use cases that I want to entirely prevent some programs from obtaining focus, I want to have a custom event hook that identifies and throws requests from those program before passing to ewmhDesktopsEventHook, with this function hidden, it's less convenient to do so.

TheMC47 commented 2 years ago

Maybe it's leas convenient for you, but I'm pretty sure hiding the old API would save a lot of users a lot of headache.

Could you provide a code snippet for your particular case? Maybe there's a workaround that we can document.

Javran commented 2 years ago

But what kind of headache specifically? User can migrate to use ewmh and for those want deeper customization we still prefer easy access to those functions.

For my specific example, I have this hook inserted instead of the usual ewmhDesktopsEventHook:

myEwmhDesktopsEventHook :: Event -> X All
myEwmhDesktopsEventHook e@ClientMessageEvent {..} = do
  a_aw <- getAtom "_NET_ACTIVE_WINDOW"
  curTime <- liftIO getCurrentTime
  StartupTime starupTime <- XS.get
  -- stop EWMH until T+5 sec to prevent focus grabbing war right after launching startup programs.
  if ev_message_type == a_aw && curTime `diffUTCTime` starupTime <= 5.0
    then pure (All False)
    else ewmhDesktopsEventHook e
myEwmhDesktopsEventHook e = ewmhDesktopsEventHook e

And I might also condition on ev_window and current XState and choose to drop an event before passing to ewmhDesktopsEventHook.

liskin commented 2 years ago

Consider in some use cases that I want to entirely prevent some programs from obtaining focus, I want to have a custom event hook that identifies and throws requests from those program before passing to ewmhDesktopsEventHook, with this function hidden, it's less convenient to do so.

No, it's not less convenient to do this. We have an easy to use interface for just that use case: https://xmonad.github.io/xmonad-docs/xmonad-contrib-0.17.0.9/XMonad-Hooks-EwmhDesktops.html#g:5

Even if we did not, you can make a XConfig a → XConfig a combinator that takes the whole handleEventHook and replaces it with one that ignores some specific events and invokes the original hook for the rest. So you'd end up with main = myIgnoreFocusStealingApp $ ewmh $ def{ … } which, IMO, is still way nicer than using the individual hooks.

I see those deprecations in f666cf4 but don't find it convicing - could you elaborate what are error-prone about those APIs? If the plan is just to hide those from user, I can still easily workaround it by passing an empty config to ewmh and have access corresponding fields.

We have users who barely know Haskell, so asking them to configure all hooks in the correct order and to read the changelog of every release and add/remove hooks that are suddenly required (the ManageDocks fuckup of xmonad 0.12 was haunting us for years until 0.17, and it probably still is as most distros haven't updated) is really too much.

Indeed, if you absolutely need to extract those individual hooks, passing an empty config to ewmh is a way to do it. (That is, until #625 moves forward, then things will get more complicated.) But if you do something like this then you might be on your own if you encounter issues :-)

Javran commented 2 years ago

I hacked together this solution probably 7 years ago so I'm totally unaware of setEwmhActivateHook and other new interfaces. But after taking at look at them, it sounds good to me. I'll give it a try and see how it goes.

slotThe commented 2 years ago

@Javran how did you get on?

Javran commented 2 years ago

Yup, it looks fine after migration, omitting unrelated stuff, new code looks like this:

myConfig :: ... -> XConfig _
myConfig _ _ =
    ...
    setEwmhActivateHook myActivateHook $
      ewmh $
         ...
  where
    myActivateHook =
      composeOne
        [ (do
             curTime <- liftIO getCurrentTime
             StartupTime starupTime <- liftX XS.get
             pure $ curTime `diffUTCTime` starupTime <= 5.0)
            -?> doAskUrgent
        , pure True -?> doFocus
        ]

Closing this issue as I'm happy with the result. Thanks for the help!