X.L.MultiToggle: Add function to query toggle state

[colonelpanic8]

I'm entirely aware that the approach I have taken here is pretty hideous, and as such will probably never be accepted into xmonad-contrib. The thing is that no matter how hard I tried I could not figure out another way to provide this functionality. I'm putting this here in the hopes that someone can come up with a better way, or perhaps to get confirmation that there is no real way around the gross hack that I've added here.

[mention-bot]

@IvanMalison, thanks for your PR! By analyzing the history of the files in this pull request, we identified @liskin, @mauke and @byorgey to be potential reviewers.

[liskin]

You're right that there's currently no good way to query a layout and I've done ugly hacks in my copy of xmonad-contrib to work around it too, e.g.: liskin@3b4ada8

But in this particular case you only need one bit of information and you could possibly get that by using handleMessage directly:

sendQueryMessage :: Message a => a -> X Bool
sendQueryMessage a = do
    w <- W.workspace . W.current <$> gets windowset
    ml' <- handleMessage (W.layout w) (SomeMessage a) `catchX` return Nothing
    return $ isJust ml'

Don't export this function, though, as it throws the modified layout away and is thus only safe if the message did nothing interesting in the X monad. Then, in X.L.MultiToggle handleMessage, return if i == currIndex mt then Just mt else Nothing.

[colonelpanic8]

But in this particular case you only need one bit of information and you could possibly get that by using handleMessage directly:

Clever, but as you point out, this sort of code is also an abuse of handleMessage, is it not? I'm not necessarily sure that I think its any less hacky.

Shouldn't there be a nice way to query layouts in general?

[liskin]

Yes it's a horrible hack, too. :-)
There should be a nice way to query layouts, I think. I'm not sure if we should add something to LayoutClass or do it entirely in xmonad-contrib by creating an additional type class for querying and defining instances for ModifiedLayout, Choose, NewSelect, etc.

[colonelpanic8]

@pjones Do you want me to close this? I think that something definitely SHOULD be done about this, but I'm not really sure what the best approach is.

[pjones]

@IvanMalison closing it is up to you. I haven't read enough about this issue to know what the right way of doing it is. Perhaps open an issue and reference this PR before you close it. That way it doesn't get forgotten.

[colonelpanic8]

I still think something ought to be done about this. I might file an issue, but am wondering if it should go against contrib or against xmonad itself. I think the cleanest solution would involve changes in xmonad.

[liskin]

I've done something in the general direction of this: xmonad/xmonad#242, #398
It's already usable for querying the outermost ModifiedLayout, in my case WorkspaceDir: liskin/dotfiles@6ea6c52
If we wanted to dig deeper inside the tree of ModifiedLayouts and Chooses, we'd probably need to add another function to LayoutClass. I don't need that right now, but I think it'd be okay to do it.

[liskin]

A month passed and I have some additional progress:

XMonad.Layout.Inspect, a class for inspection of layouts and layout modifiers. Example of usage in X.L.WorkspaceDir and X.L.SubLayouts.

The relevant bits in my config are:
https://github.com/liskin/dotfiles/blob/a4140660ab0d83c7dc24378485ed7d8bda90d742/.xmonad/xmonad.hs#L406
https://github.com/liskin/dotfiles/blob/a4140660ab0d83c7dc24378485ed7d8bda90d742/.xmonad/xmonad.hs#L417

Also, I think your ugly trick with ExtensibleState can be made way less ugly by using IORef wrapped in a message: the query function creates an IORef, wraps it in a message, calls sendMessageWithNoRefresh, the layout handles the message by writing to the IORef, and then the query function reads the result and returns it. No need for permanent extensible state. It's still a bit ugly and it may not work with hacky layouts like SubLayouts that queue messages and process them later, and there are other things the layouts can do to break it, but it's simple. :-)

[colonelpanic8]

@liskin What is the status of this? Could we merge Xmonad.Layout.Inspect any time soon?

[liskin]

@IvanMalison I haven't managed to get to this since December so it's still more of a proof-of-concept than anything. The plan was to get back to it once the 0.17 release is out. It doesn't even have MultiToggle inspection at this point, but that should be like 5 more lines or something, I guess I can try to hack on that bit next week so that you can at least test it.

[colonelpanic8]

@IvanMalison I haven't managed to get to this since December so it's still more of a proof-of-concept than anything. The plan was to get back to it once the 0.17 release is out. It doesn't even have MultiToggle inspection at this point, but that should be like 5 more lines or something, I guess I can try to hack on that bit next week so that you can at least test it.

I'm happy to work on it myself. Just wondering what the state is/how much work is needed.

[liskin]

I'm happy to work on it myself. Just wondering what the state is/how much work is needed.

Even better :-)

[liskin]

(if you want to discuss something quickly, I'm liskin @ irc.libera.chat or @liskin:matrix.org)

[colonelpanic8]

Also, I think your ugly trick with ExtensibleState can be made way less ugly by using IORef wrapped in a message: the query function creates an IORef, wraps it in a message, calls sendMessageWithNoRefresh, the layout handles the message by writing to the IORef, and then the query function reads the result and returns it.

Yeah, actually this seems like a pretty reasonable solution to me. Would you have a problem with merging something like that?

[liskin]

Would you have a problem with merging something like that?

Seems like a reasonable thing to do. The inspection stuff won't be ready for a while and you only export a function the implementation of which can be swapped any time. I'll take a closer look later today, hopefully.

[liskin]

Aligned the indent with the rest of MultiToggle and added docs/changelog. Will merge later in case you need to change something else.

[colonelpanic8]

Aligned the indent with the rest of MultiToggle and added docs/changelog. Will merge later in case you need to change something else.

Thanks!