-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
Update StatusBar library to use the X monad instead of IO. #878
Conversation
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` - https://hackage.haskell.org/package/xmonad-0.18.0/docs/XMonad-Core.html#v:io - https://hackage.haskell.org/package/base-4.19.1.0/docs/Control-Monad-IO-Class.html#v:liftIO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution! Looks good, but I want to wait for a second opinion because this is a breaking change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but I think we need to hear from users who would be affected by it. You might want to post to /r/xmonad.
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/? |
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. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Out of curiosity: do you have a usage example?
Yeah, that's been my opinion. The other alternative is to just add 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
Here's an example of using this to build a status bar out of three components proportionally:
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 😅 |
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 |
Agreed, it needs to be clearly documented. I'm in favor. |
I think that's probably the main use case, in which case nothing would have to change because you would just get There is one other thing which I wondered a bit about... I'm also reasonably happy if we just duplicate everything... Or another potential solution might be to have a |
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
I don't think that this is necessary, personally.
Now that you mention it… :) Again, no strong preference from my side. |
It was bugging me since I've seen the PR why I didn't just use I'm okay with the breaking change, I just don't want that pattern to break if we can help it |
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?
The constraints on the |
Thank you for your contribution 🎉 It's actually fine as-is. I created an issue so we don't forget documenting this change in the next release |
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 anX StatusBarConfig
. This can be done using either theio
function provided byXMonad.Core
, orliftIO
frombase
inControl.Monad.IO.Class
Checklist
I've read CONTRIBUTING.md
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.
I updated the
CHANGES.md
file