Move ppTitleUnfocused to X.U.Loggers

[slotThe]

Description

This replaces ppTitleUnfocused from commit
a3e0668 with a more extensible version,
containing more information about the window and being able to operate
on the focused window as well.

The functionality provided is a strict superset of ppTitle, though
that function is kept for backwards compatibility.

This plays nicely with #463 for lazy people like me who want their bars to read from the same property, as showing windows only if they're on the current screen is now easily possible (see the example).

Checklist

• I've read CONTRIBUTING.md

• I tested my changes with xmonad-testing

• I updated the CHANGES.md file

- [ ] I updated the XMonad.Doc.Extending file (if appropriate)

[liskin]

I must confess that I'm not entirely happy that we're making dynamicLogString more complex. This could likely be implemented as a separate thing plugged into PP via ppExtras, and then people who don't want to show all windows wouldn't have to pay the price of all those useless getName calls (those weren't introduced in this PR, I know, this just reminded me of that problem), and also the code wouldn't turn dynamicLogString into spaghetti.

[liskin]

I also wonder if the WindowPP abstraction is general enough to be useful outside of your usecase. My own per-screen bar "PP" needs the Window id as well to make windows clickable, for example: https://github.com/liskin/dotfiles/blob/60fa4dace1678267d3cda6e0443a5fd0644f67fa/.xmonad/xmonad.hs#L417 (it also does some SubLayouts integration stuff that makes any attempts at general WindowPP unusable in my context, but let's ignore that now).

So my gut feeling is that it might be better to provide some helpers to make it easier for users to piece something like this together and put that into ppExtras rather than hardwiring this into PP.

[slotThe]

I also wonder if the WindowPP abstraction is general enough to be useful outside of your usecase. My own per-screen bar "PP" needs the Window id as well to make windows clickable, for example: https://github.com/liskin/dotfiles/blob/60fa4dace1678267d3cda6e0443a5fd0644f67fa/.xmonad/xmonad.hs#L417

Well, if it's just adding the Window id then it's quite trivial to add a field to WindowPP with that; but your point is well taken.

So my gut feeling is that it might be better to provide some helpers to make it easier for users to piece something like this together and put that into ppExtras rather than hardwiring this into PP.

Just plugging it into ppExtras would work as well yes. I you think that that that's the best way forward I wouldn't be disinclined to implement it like this. Though I would personally like to keep the WindowPP abstraction alive and supply something like getWindows :: X [WindowPP], so that users are as free as possible to do whatever they want with that information.

[liskin]

Well, if it's just adding the Window id then it's quite trivial to add a field to WindowPP with that; but your point is well taken.

For clickable windows, adding the Window is definitely enough, yeah.

Just plugging it into ppExtras would work as well yes. I you think that that that's the best way forward I wouldn't be disinclined to implement it like this.

I'd say start by factoring the code out of dynamicLogString anyway and try to make it work standalone and see where that leads you.

Though I would personally like to keep the WindowPP abstraction alive and supply something like getWindows :: X [WindowPP], so that users are as free as possible to do whatever they want with that information.

I'm okay with that. Do note, however, that what you have now is not really a PP (pretty-printer) record, but more like a WindowInfo record. I'd expect WindowPP to have fields like wppTitle :: String -> String and wppUrgent :: String -> String etc, which then makes it really easy to customize colors, but possibly harder to customize stuff that depends on screen id or window id, so then we'd end up with needing to add wppRename :: String -> Window -> X String and pprWindows :: WindowPP -> ScreenId -> X [String] (or something that fits better into ppExtras). Have you considered having such an API? I'm not entirely sure it's better, it's just more consistent with what we already have. Obviously we can't hope to cover all use cases, so we want a tradeoff between making the easy stuff (e.g. custom colors) easy, and we always get the "hard stuff possible" part for free as our configuration language is Haskell.

[TheMC47]

Regarding ppExtras, I already implemented something similar to this (e.g. only showing information that is bound to a particular screen) back in #448. Or does this provide more functionality than that?

[slotThe]

Regarding ppExtras, I already implemented something similar to this (e.g. only showing information that is bound to a particular screen) back in #448. Or does this provide more functionality than that?

Oh. Stupid brain, I totally forgot that pr :)

[slotThe]

Okay as discussed on IRC I will use this to remove ppTitleUnfocused instead and move it to X.U.Loggers. This is a sample implementation, but there are several ways forward:

• Give back an X [String] and let the user handle formatting however they like
• Take some String -> String formatting functions (the current way)
• Use the WindowInfo abstraction somewhere in here
• Probably a few more

Some of these may or may not break composition with the rest of the module—opinions?

[TheMC47]

This makes the Loggers module more relevant, and it's quite a good addition. We could experiment with (WindowPP -> String) -> Logger - but I'm not sure if that's useful. This is good in its current state
LGTM.

[liskin]

I'd maybe consider adding a formatter for urgent windows, but I'm not sure about it really. Anyone who needs something more complex like numbered windows, per-screen window lists, custom separators, etc. will roll their own anyway. Having an X [String] won't help much in at least the per-screen usecase.

This fully replaces ppTitleUnfocused, so if that was enough for some, this is good enough too. So yeah, it's good to go as it is I think.

[slotThe]

Yeah, if you get into more complicated formatters it's difficult to tell when to stop. The aim was indeed just to replace ppTitleUnfocused; overall I don't think it's worth making it more complicated for something that perhaps relatively few people would use (and, as you said, these people may well just roll their own logger)