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

Allow pattern-matching on 'XMonad.Layout.Choose' by exporting constructors #219

Merged
merged 2 commits into from
Dec 8, 2020

Conversation

jumper149
Copy link
Contributor

@jumper149 jumper149 commented Mar 31, 2020

Export the constructor Choose to be able to construct and pattern-match Choose l r a. To do that it is also necessary to export LR with constructors.

Description

I just added the constructor Choose and LR with constructors to the exports of the module XMonad.Layout

Checklist

  • I've read CONTRIBUTING.md

  • I've confirmed these changes don't belong in xmonad-contrib instead

  • I tested my changes with xmonad-testing (that seems to be a dead repository)

  • I updated the CHANGES.md file

jumper149 added a commit to jumper149/dotfiles that referenced this pull request Mar 31, 2020
@jumper149
Copy link
Contributor Author

Here in my dotfiles you can see an example of what can be done with this change:

I use everything that Choose provides but wrap it in a newtype and define a different instance of LayoutClass, so that I'm able to handle specific Messages differently.
I send the messages ModifyWindowBorderEnabled a and ModifyScreenBorderEnabled a to both sub-layouts instead of just the current one.

This way I can set the spacing for all sub-layouts on my workspaces with just using toggleGaps.

@liskin
Copy link
Member

liskin commented May 13, 2020

I have no opinion about whether exposing these constructors should be done or not, but I think there is an easier way to implement this.

If I understand your setup correctly, you have a global toggle for gaps that broadcasts a message to all workspaces and all sublayouts and this message toggles the inner state of all those layouts (or layout modifiers). So what you really want is a single global boolean toggle instead of a hundred local ones that you toggle, hoping they'll never come out of sync. How about this:

Store this global toggle in ExtensibleState and whenever the layout is switched, sync this state into it using something like

setGaps enabled :: X ()
setGaps enabled = do traverse_ broadcastMessage messages
                    refresh
   where messages = [ ModifyWindowBorderEnabled (const enabled)
                    , ModifyScreenBorderEnabled (const enabled)
                    ]

It's not the cleanest solution, I guess, but it's simpler than adding a LayoutClass that delivers messages to both active and inactive layouts (which is probably dangerous in general).

@liskin
Copy link
Member

liskin commented Dec 8, 2020

I am now in favor of merging this. In #242 and xmonad/xmonad-contrib#398 I started working on being able to query the current layout state. The next step is to add a InspectLayoutModifier type class to xmonad-contrib to allow inspecting the current state of some modifier. A wip version of that is sitting here: liskin/xmonad-contrib@77c1821. This class will need instances for many layout combinators, including Choose, so I also need the constructors to be exported.

Can you please rebase and fix the conflicts?
@psibi, @byorgey can you folks then merge it please?
Thanks!

@psibi psibi merged commit 96d4f1f into xmonad:master Dec 8, 2020
@psibi
Copy link
Member

psibi commented Dec 8, 2020

@jumper149 Thanks for for the MR!

@liskin Done.

@liskin
Copy link
Member

liskin commented Dec 9, 2020

This ended up breaking compilation of xmonad-contrib as modules that define their own L/R data constructors (X.U.Types, X.L.GridVariants, X.L.ResizeScreen, X.L.TallMastersCombo, X.C.Types) cause ambiguity now.
As LR was never exported before, the best course of action is probably to rename it and its constructors so it doesn't conflict. Exporting L and R from XMonad is really nasty. :-/

And then maybe also reduce the duplication in xmonad-contrib: use the renamed/exported LR in TallMastersCombo.

Reported here: https://old.reddit.com/r/xmonad/comments/ka1qxg/fixing_fullscreen/

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