X.H.ManageDocks: Init strut cache on demand if necessary

This makes docksStartupHook unnecessary. That is desirable because we
didn't add a changelog entry about it being necessary, and 4 years after
its introduction we still have users grabbing old configs and reporting
(xmonad/xmonad#21 (comment)) that
ManageDocks doesn't work properly.

I would love to finally settle this.

Full story follows:

xmonad-contrib 0.12 introduced (00be056, merged in April 2013)
caching to ManageDocks to avoid queryTree calls on every runLayout,
which brought in several bugs. Attempts to fix these bugs in
xmonad-contrib 0.13 introduced (28e9f8b, merged in February 2016) a
breaking change in ManageDocks that required users to add
docksStartupHook (or docks, after e38fb3b got merged in November
2016) to their configs for avoidStruts to still work after xmonad
restart. Unfortunately this was never mentioned in the changelog nor in
any compilation warning (which get discarded by xmonad --recompile
anyway !!!), so as of March 2020 we still have users being bitten by
this.

Back in September 2016 in a discussion about other bugs introduced in
28e9f8b I suggested that we use Maybe to indicate whether the cache
had been initialized and initialize it on demand when it had not.
Unfortunately I wasn't sufficiently clear about what I meant and Brandon
was going through some health issues, so we just got into a heated
argument and ended up merging a suboptimal solution. :-(

And since we're _still_ getting complaints from users every now and
then, I believe it's worth dealing with even after all these years.
If nothing else, let this serve as a reminder that breaking users'
configs without any warning is wrong.

(Oh and we should probably stop hiding xmonad.hs compilation warnings,
otherwise we can't ever hope to provide smooth deprecation and upgrade
paths.)

Fixes: 00be056 ("Cache results from calcGap in ManageDocks")
Fixes: 28e9f8b ("add docksStartupHook for handling docks when restarted")
Fixes: e38fb3b ("Make usage of ManageDocks simpler and more robust")
Related: xmonad#118
Related: xmonad#30
Related: xmonad#80
Related: xmonad/xmonad#21

CHANGES.md

@@ -294,8 +294,12 @@
     - Export `popHiddenWindow` function restoring a specific window.
 
   * `XMonad.Hooks.ManageDocks`
+
     - Export `AvoidStruts` constructor
 
+    - Restored compatibility with pre-0.13 configs by making the startup hook
+      unnecessary for correct functioning.
+
   * `XMonad.Hooks.ManageHelpers`
     - Export `doSink`
 
@@ -899,6 +903,12 @@
   * `XMonad.Prompt` now stores its history file in the XMonad cache
     directory in a file named `prompt-history`.
 
+  * `XMonad.Hooks.ManageDocks` now requires an additional startup hook to be
+    added to configuration in addition to the other 3 hooks, otherwise docks
+    started before xmonad are covered by windows. It's recommended to use the
+    newly introduced `docks` function to add all necessary hooks to xmonad
+    config.
+
 ### New Modules
 
   * `XMonad.Layout.SortedLayout`
@@ -932,7 +942,7 @@
 
 ### Bug Fixes and Minor Changes
 
-  * `XMonad.Hooks.ManageDocks`,
+  * `XMonad.Hooks.ManageDocks`
 
     - Fix a very annoying bug where taskbars/docs would be
       covered by windows.

XMonad/Hooks/ManageDocks.hs

@@ -1,4 +1,5 @@
 {-# LANGUAGE DeriveDataTypeable, PatternGuards, FlexibleInstances, MultiParamTypeClasses, CPP #-}
+{-# LANGUAGE LambdaCase #-}
 -----------------------------------------------------------------------------
 -- |
 -- Module       : XMonad.Hooks.ManageDocks
@@ -44,7 +45,7 @@ import Data.Monoid (All(..))
 
 import qualified Data.Set as S
 import qualified Data.Map as M
-import Control.Monad (when, forM_, filterM)
+import Control.Monad (when, filterM, void)
 
 -- $usage
 -- To use this module, add the following import to @~\/.xmonad\/xmonad.hs@:
@@ -90,7 +91,10 @@ docks c = c { startupHook     = docksStartupHook <+> startupHook c
             , handleEventHook = docksEventHook <+> handleEventHook c
             , manageHook      = manageDocks <+> manageHook c }
 
-newtype StrutCache = StrutCache { fromStrutCache :: M.Map Window [Strut] }
+type WindowStruts = M.Map Window [Strut]
+
+-- Nothing means cache hasn't been initialized yet
+newtype StrutCache = StrutCache { fromStrutCache :: Maybe WindowStruts }
     deriving (Eq, Typeable)
 
 data UpdateDocks = UpdateDocks deriving Typeable
@@ -100,15 +104,35 @@ refreshDocks :: X ()
 refreshDocks = sendMessage UpdateDocks
 
 instance ExtensionClass StrutCache where
-  initialValue = StrutCache M.empty
+    initialValue = StrutCache Nothing
+
+initStrutCache :: X WindowStruts
+initStrutCache = withDisplay $ \dpy -> do
+    rootw <- asks theRoot
+    (_,_,wins) <- io $ queryTree dpy rootw
+    dockws <- filterM (runQuery checkDock) wins
+    M.fromList . zip dockws <$> mapM getStrut dockws
+
+getStrutCache :: X (Bool, WindowStruts)
+getStrutCache = XS.gets fromStrutCache >>= \case
+    Just cache ->
+        return (False, cache)
+    Nothing -> do
+        cache <- initStrutCache
+        XS.put $ StrutCache $ Just cache
+        return (True, cache)
 
 updateStrutCache :: Window -> [Strut] -> X Bool
-updateStrutCache w strut =
-  XS.modified $ StrutCache . M.insert w strut . fromStrutCache
+updateStrutCache w strut = do
+    ch1 <- fst <$> getStrutCache
+    ch2 <- XS.modified $ StrutCache . fmap (M.insert w strut) . fromStrutCache
+    return $ ch1 || ch2
 
 deleteFromStructCache :: Window -> X Bool
-deleteFromStructCache w =
-  XS.modified $ StrutCache . M.delete w . fromStrutCache
+deleteFromStructCache w = do
+    ch1 <- fst <$> getStrutCache
+    ch2 <- XS.modified $ StrutCache . fmap (M.delete w) . fromStrutCache
+    return $ ch1 || ch2
 
 -- | Detects if the given window is of type DOCK and if so, reveals
 --   it, but does not manage it.
@@ -151,14 +175,7 @@ docksEventHook (DestroyWindowEvent {ev_window = w}) = do
 docksEventHook _ = return (All True)
 
 docksStartupHook :: X ()
-docksStartupHook = withDisplay $ \dpy -> do
-    rootw <- asks theRoot
-    (_,_,wins) <- io $ queryTree dpy rootw
-    docks <- filterM (runQuery checkDock) wins
-    forM_ docks $ \win -> do
-        strut <- getStrut win
-        updateStrutCache win strut
-    refreshDocks
+docksStartupHook = void $ getStrutCache
 
 -- | Gets the STRUT config, if present, in xmonad gap order
 getStrut :: Window -> X [Strut]
@@ -181,7 +198,7 @@ getStrut w = do
 calcGap :: S.Set Direction2D -> X (Rectangle -> Rectangle)
 calcGap ss = withDisplay $ \dpy -> do
     rootw <- asks theRoot
-    struts <- (filter careAbout . concat) <$> XS.gets (M.elems . fromStrutCache)
+    struts <- filter careAbout . concat . M.elems . snd <$> getStrutCache
 
     -- we grab the window attributes of the root window rather than checking
     -- the width of the screen because xlib caches this info and it tends to