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.

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

XMonad.Actions.GridSelect: added gs_cancelOnEmptyClick field #894

Merged
merged 1 commit into from
Jun 10, 2024

Conversation

sylecn
Copy link
Contributor

@sylecn sylecn commented Jun 9, 2024

Description

In the original code, when a GridSelect is shown, user has to use a keyboard to cancel it (ESC key by default). With this field added, when it is set to True, mouse click on empty space can cancel the GridSelect.

When user trigger a GridSelect action using mouse, this change makes it easier to cancel the Grid, without reaching to the keyboard.

Checklist

  • I've read CONTRIBUTING.md

  • I've considered how to best test these changes (property, unit,
    manually, ...) and concluded:

    I have tested the code manually. The default setting preserves the original behavior. User has to opt-in to get the new feature, to reduce surprises.

    To test the new behavior,

import XMonad.Actions.GridSelect

myGoToSelected = goToSelected def { gs_cancelOnEmptyClick = True }

-- try it with a key binding
, ("M-<Space>", myGoToSelected)
  • I updated the CHANGES.md file

Copy link
Member

@slotThe slotThe left a comment

Choose a reason for hiding this comment

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

Looks fine to me, thanks :)

XMonad/Actions/GridSelect.hs Outdated Show resolved Hide resolved
XMonad/Actions/GridSelect.hs Outdated Show resolved Hide resolved
XMonad/Actions/GridSelect.hs Show resolved Hide resolved
@sylecn
Copy link
Contributor Author

sylecn commented Jun 9, 2024

Hi slotThe, thanks for the review.

I fixed some issues you mentioned and force pushed.
Remaining two issues:

  1. about whenMaybe. whenMaybe is in extra package, which is not currently in dependency list. Do you want to add it as a dependency, or just add a local definition?
  2. how to use whenMaybe here? A quick look on the doc gives the following code which doesn't type check.
Nothing -> whenMaybe (not $ gs_cancelOnEmptyClick gsconfig) contEventloop
xmonad-contrib>     • Couldn't match type ‘a’ with ‘Maybe a’
xmonad-contrib>       Expected: TwoD a a
xmonad-contrib>         Actual: TwoD a (Maybe a)
xmonad-contrib>       ‘a’ is a rigid type variable bound by
xmonad-contrib>         the type signature for:
xmonad-contrib>           stdHandle :: forall a.
xmonad-contrib>                        Event -> TwoD a (Maybe a) -> TwoD a (Maybe a)
  1. since the change is moved to "Breaking Changes" section, would you say it's okay to set gs_cancelOnEmptyClick to True by default? Which value is a better default?

@slotThe
Copy link
Member

slotThe commented Jun 9, 2024

1. about whenMaybe. whenMaybe is in extra package, which is not currently in dependency list. Do you want to add it as a dependency, or just add a local definition?

Oops, I thought we have this in ManageHook. Nevermind then, you don't have to create an extra definition for this.

3. since the change is moved to "Breaking Changes" section, would you say it's okay to set gs_cancelOnEmptyClick to True by default? Which value is a better default?

You don't have to necessarily move the whole thing to breaking changes, just the part that's actually a breaking change. I'd say this is can be a reasonable default

@sylecn
Copy link
Contributor Author

sylecn commented Jun 9, 2024

I set the default value to True and updated the doc accordingly.

XMonad/Actions/GridSelect.hs Outdated Show resolved Hide resolved
In the original code, when a GridSelect is shown, user has to use
keyboard to cancel it (ESC key by default). With this field added,
when it is set to True (the default), mouse click on empty space
can cancel the GridSelect.
@slotThe slotThe merged commit 5325ca8 into xmonad:master Jun 10, 2024
20 checks passed
@slotThe
Copy link
Member

slotThe commented Jun 10, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants