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.

Sign up for GitHub

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

deprecate XMonad.Util.Ungrab #843

Merged
merged 1 commit into from
Dec 9, 2023
Merged

deprecate XMonad.Util.Ungrab #843

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGES.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,9 @@
- The function `readKeySequence` now returns a non-empty list if it
succeeded.

* Deprecate `XMonad.Util.Ungrab`; it was moved to `XMonad.Operations`
in core.

### New Modules

* `XMonad.Layout.CenterMainFluid`
Expand Down
17 changes: 9 additions & 8 deletions XMonad/Config/Mate.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
{-# OPTIONS_GHC -fno-warn-missing-signatures #-}

-- TODO: Remove when we depend on a version of xmonad that has unGrab.
{-# OPTIONS_GHC -Wno-deprecations #-}
{-# OPTIONS_GHC -Wno-dodgy-imports #-}
-----------------------------------------------------------------------------
-- |
-- Module : XMonad.Config.Mate
Expand Down Expand Up @@ -28,15 +30,14 @@ module XMonad.Config.Mate (
desktopLayoutModifiers
) where

import XMonad
import XMonad.Config.Desktop
import XMonad.Util.Run (safeSpawn)
import XMonad.Util.Ungrab
Copy link
Member

Choose a reason for hiding this comment

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

This will result in a build failure related to this. I'm not sure it's worth bumping our bounds over, so we should probably be a bit more tame here as well

Copy link
Author

Choose a reason for hiding this comment

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

What I found is that if I don't remove this line, the compiler will complain about an ambiguous reference in the new version. Because unGrab will be defined both in XMonad.Operations (which is imported into this file from import XMonad) and XMonad.Util.Ungrab.

But thinking about it again. I could probably just do import XMonad hiding (unGrab). Would that be preferrable?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I was thinking about hiding the function from the XMonad import and perhaps adding a {-# OPTIONS_GHC -Wno-dodgy-imports #-} at the top of the file, to pacify the compiler when this is compiled against xmonad HEAD

Copy link
Member

@liskin liskin Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I'd be in favor of bumping xmonad and having #if MIN_VERSION_xmonad(0, 17, 9) in X.U.Ungrab so that users following the stable releases don't get the Ambiguous occurrence ‘unGrab’ error when compiling their configs, and just get a deprecaton warning.

Copy link
Member

Choose a reason for hiding this comment

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

(Note that for that to work, we can't have unGrab = Operations.unGrab but must reexport the import.)

Copy link
Member

Choose a reason for hiding this comment

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

I'd be in favor of bumping xmonad and having #if MIN_VERSION_xmonad(0, 17, 9) in X.U.Ungrab so that users following the stable releases don't get the Ambiguous occurrence ‘unGrab’ error when compiling their configs, and just get a deprecaton warning.

Right now users compiling against xmonad-0.17.2 should not get an error (as evidenced by the CI), since we're hiding (the possible non-existent) unGrab from the XMonad import. Still, bumping might not be such a bad idea

Copy link
Member

@liskin liskin Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I wasn't talking about compiling xmonad-contrib itself, but about compiling the xmonad.hs config. Anyone who imports both XMonad and XMonad.Util.Ungrab and uses unGrab will start getting errors. I mean, yeah, it is mentioned in "Breaking Changes", but I don't think it should actually be a Breaking Change when we can turn it into just a deprecation warning.

Copy link
Member

@liskin liskin Dec 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

(liskin/xmonad@5c6c3d4 & liskin@8526d93 are one way to solve this, if we want an ifdef)

Copy link
Member

@slotThe slotThe Dec 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Oh, right, that might indeed be an issue. The ifdef sounds fine in that case; thanks for spotting this!

Copy link
Member

Choose a reason for hiding this comment

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

Okay, I pushed the two commits then.

import XMonad.Prelude (toUpper)

import System.Environment (getEnvironment)
import qualified Data.Map as M

import System.Environment (getEnvironment)
import XMonad hiding (unGrab)
import XMonad.Config.Desktop
import XMonad.Prelude (toUpper)
import XMonad.Util.Run (safeSpawn)
import XMonad.Util.Ungrab (unGrab)

-- $usage
-- To use this module, start with the following @~\/.xmonad\/xmonad.hs@:
Expand Down
2 changes: 1 addition & 1 deletion XMonad/Util/Ungrab.hs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
--
-----------------------------------------------------------------------------

module XMonad.Util.Ungrab
module XMonad.Util.Ungrab {-# DEPRECATED "Use XMonad.Operations.unGrab instead" #-}
( -- * Usage:
-- $usage
unGrab
Expand Down