Update StatusBar library to use the X monad instead of IO.

Chobbes commented 8 months ago

Description

This change allows dynamic status bars to pull information out of the X monad, which can be really useful for status bars. For instance, you can now query the screen width in order to set the width of status bars appropriately.

Existing configurations may need to be updated in order to lift an IO StatusBarConfig to an X StatusBarConfig. This can be done using either the io function provided by XMonad.Core, or liftIO from base in Control.Monad.IO.Class

Checklist

[x] I've read CONTRIBUTING.md
[x] I've considered how to best test these changes (property, unit, manually, ...) and concluded: this is a minimal change to the types of some functions which does not break any existing tests. I have used these changes in my own XMonad configuration to great success.
[x] I updated the CHANGES.md file

Chobbes commented 8 months ago

Okay, I don't really use reddit, so hopefully I did this right:

https://old.reddit.com/r/xmonad/comments/1baovjf/proposed_improvements_to_statusbar_library/?

geekosaur commented 8 months ago

I don't use it a whole lot either, but it's more active than the IRC channel and certainly more active than expecting xmonad users to be monitoring github.

Chobbes commented 8 months ago

The breaking change is quite severe, but also trivial to fix, so I think I'm going with "yes" on this one. We just need to properly point this out to people in an eventual release announcement.

Yeah, that's been my opinion. The other alternative is to just add X versions of the functions in addition to the existing ones, but it doesn't really seem worth the duplication.

It seems like a good time to do it with DynamicBars being deprecated recently...

https://hackage.haskell.org/package/xmonad-contrib-0.18.0/docs/XMonad-Hooks-DynamicBars.html

Out of curiosity: do you have a usage example?

Here's an example of using this to build a status bar out of three components proportionally:

mainBarWidth :: Int -> Int
mainBarWidth fullWidth = floor $ 0.4 * fromIntegral fullWidth

conkyBarOffset = mainBarWidth

conkyBarWidth :: Int -> Int
conkyBarWidth fullWidth = (fullWidth - (conkyBarOffset fullWidth)) - (trayerBarWidth fullWidth)

trayerBarWidth :: Int -> Int
trayerBarWidth fullWidth = floor $ fromIntegral fullWidth * 0.05

-- Command to launch the bar.
mainBar :: ScreenId -> X StatusBarConfig
mainBar screen = do
  let S id' = screen
  let id = id' + 1
  (x, _) <- withWindowSet (getScreenDims screen)
  let barWidth = mainBarWidth x
  let dzencmd = "dzen2 -dock -e 'onstart=lower' -w " ++ show barWidth ++ " -h 15 -ta l -fn 'Helvetica:size=11' -xs " ++ show id
  liftIO $ statusBarPipe dzencmd (return myPP)

conkyBar :: ScreenId -> X StatusBarConfig
conkyBar screen = do
  let S id' = screen
  let id = id' + 1
  (x, _) <- withWindowSet (getScreenDims screen)
  let barOffset = conkyBarOffset x
  let barWidth = conkyBarWidth x
  let conkycmd = "conky -c ~/.xmonad-conky"
  let dzencmd = "dzen2 -dock -e 'onstart=lower' -x " ++ show barOffset ++ " -w " ++ show barWidth ++ " -h 15 -ta r -fn 'Helvetica:size=12' -xs " ++ show id
  return $ statusBarGeneric (conkycmd ++ " | " ++ dzencmd) mempty

trayerBar :: ScreenId -> X StatusBarConfig
trayerBar screen = do
  let S id' = screen
  let id = id' + 1
  (x, _) <- withWindowSet (getScreenDims screen)
  let barWidth = trayerBarWidth x
  let conkycmd = "conky -c ~/.xmonad-conky"
  let trayercmd = "trayer --edge top --align right --SetDockType true --SetPartialStrut true --expand true --widthtype pixel --heighttype pixel --tint 0x002b36 --height 15 --width " ++ show barWidth ++ " --alpha 0 --transparent true -l --monitor " ++ show id'
  return $ statusBarGeneric trayercmd mempty

myBar :: ScreenId -> X StatusBarConfig
myBar screen = mainBar screen <> conkyBar screen <> trayerBar screen

-- Defaults to 1920x1080 if it can't find it.
getScreenDims :: ScreenId -> WindowSet -> X (Int, Int)
getScreenDims sid ws =
  case listToMaybe (filter ((== sid) . screen) (screens ws)) of
    Just s  -> return $ rectDims . screenRect $ screenDetail s
    Nothing -> return (1920, 1080)

Note: I just started experimenting with trayer, and it seems like you can only have one copy of the system tray, so that might not work well on multiple monitors :sweat_smile:

TheMC47 commented 8 months ago

Looks neat. I think we just need to clearly outline the needed changes the release notes. For most configs, the signature of the function given to dynamicSBs must be changed and that's it. I think people just use pure with statusBarProp

geekosaur commented 8 months ago

Agreed, it needs to be clearly documented. I'm in favor.

Chobbes commented 8 months ago

Looks neat. I think we just need to clearly outline the needed changes the release notes. For most configs, the signature of the function given to dynamicSBs must be changed and that's it. I think people just use pure with statusBarProp

I think that's probably the main use case, in which case nothing would have to change because you would just get pure for X instead of IO.

There is one other thing which I wondered a bit about... statusBarPipe, currently returns an IO StatusBarConfig. I left that as is because it's easy to lift it into X, but it does mean that if you passed the result of statusBarPipe directly into dynamicSBs you would need to lift it from IO to X now. If it's only ever used in conjunction with these functions, though, maybe it's convenient to just have everything in X. I don't have strong opinions about it either way.

I'm also reasonably happy if we just duplicate everything... Or another potential solution might be to have a MonadX typeclass or something which provides a toX :: MonadX mx => mx a -> X a function. Then we could have toX = liftIO for MonadIO mio => mio a -> X a and toX = id for X a -> X a. Having MonadX would mean the usage of these functions wouldn't have to change, but it would mean adding a typeclass somewhere.

slotThe commented 8 months ago

There is one other thing which I wondered a bit about... statusBarPipe, currently returns an IO StatusBarConfig. I left that as is because it's easy to lift it into X, but it does mean that if you passed the result of statusBarPipe directly into dynamicSBs you would need to lift it from IO to X now. If it's only ever used in conjunction with these functions, though, maybe it's convenient to just have everything in X. I don't have strong opinions about it either way.

I think that a pattern like

main :: IO ()
main = do
  mySB <- statusBarPipe "xmobar" (pure myPP)
  xmonad $ withSB mySB myConf

is very common, so lifting this to X would probably cause some problems for users.

I'm also reasonably happy if we just duplicate everything...

I don't think that this is necessary, personally.

Or another potential solution might be to have a MonadX typeclass or something which provides a toX :: MonadX mx => mx a -> X a function. Then we could have toX = liftIO for MonadIO mio => mio a -> X a and toX = id for X a -> X a. Having MonadX would mean the usage of these functions wouldn't have to change, but it would mean adding a typeclass somewhere.

Now that you mention it… :) Again, no strong preference from my side.

TheMC47 commented 8 months ago

I think that a pattern like
main :: IO ()
main = do
  mySB <- statusBarPipe "xmobar" (pure myPP)
  xmonad $ withSB mySB myConf
is very common, so lifting this to X would probably cause some problems for users.

It was bugging me since I've seen the PR why I didn't just use X instead of IO, and now I remember. I think we should handle that case somehow. The typeclass-route seems fine.

I'm okay with the breaking change, I just don't want that pattern to break if we can help it

Chobbes commented 8 months ago

Now that you mention it… :) Again, no strong preference from my side.

Hmmm! I didn't see that when I went looking for something that already existed along these lines... This wouldn't work, though, would it?

class (MonadReader XConf m, MonadState XState m) => XLike m where

The constraints on the class would mean IO couldn't be XLike, no?

TheMC47 commented 8 months ago

Thank you for your contribution :tada: It's actually fine as-is. I created an issue so we don't forget documenting this change in the next release

xmonad / xmonad-contrib

Update StatusBar library to use the X monad instead of IO. #878

Description

Checklist