Ewmh support for workspace names

[colonelpanic8]

This PR also fixes the issue that #105 fixed in a cleaner way.

[mention-bot]

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

[pjones]

I don't understand why there is EWMH code in WorkspaceNames.hs. Anything with ewmh in the name should be moved out of this file and into EwmhDesktops.hs.

Also, your ewmhWorkspaceNames function is almost an exact copy of the ewmh in EwmhDesktops.hs.

Please fix these issues then update the CHANGES.md file.

[geekosaur]

The right way to do this is probably to provide an appropriate function for use with ewmhDesktopsLogHookCustom.

[colonelpanic8]

The right way to do this is probably to provide an appropriate function for use with ewmhDesktopsLogHookCustom.

This behavior can't be accomplished with a pure function because access to X is needed.

I don't understand why there is EWMH code in WorkspaceNames.hs. Anything with ewmh in the name should be moved out of this file and into EwmhDesktops.hs.

The behavior of these functions is specific to WorkspaceNames which is functionality that I imagine most people don't use. It seemed more appropriate to me to group this with WorkspaceNames than with ewmh.

I could write something of type

X ([WindowSpace] -> [WindowSpace])

Which you could then combine with ewmhDesktopsLogHookCustom like

ewmhWorkspaceNamesLogHook >>= ewmhDesktopsLogHookCustom

Also, your ewmhWorkspaceNames function is almost an exact copy of the ewmh in EwmhDesktops.hs.

Except that it uses the workspace names log hook instead of the one that just uses workspace tags. If I don't add this function, then users would have to manually set the startup, handleEvent and log hooks.

Is your argument that you can do something like

(ewmh def) {
   logHook = (logHook def) +++ ewmhWorkspaceNamesLogHook
}

to acheive the same effect?

This just ends up seeming a bit more complicated for the end user. Would it be okay if I wrote ewmhWorkspaceNames in terms of ewmh like:

ewmhWorkspaceNames :: XConfig a -> XConfig a
ewmhWorkspaceNames c = (ewmh c) { logHook = logHook c +++ ewmhWorkspaceNamesLogHook }
  where x +++ y = mappend y x

[colonelpanic8]

@pjones @geekosaur
Any thoughts on what I've written above? I'm happy to proceed in whatever way you think is best.

[liskin]

I think it'd be better to turn the WindowSet -> WindowSet in EwmhDesktops into X (WindowSet -> WindowSet) so that WorkspaceNames can just provide such a function and users would then be able to plug that into their ewmhWhateverHookCustom. This should simplify the code significantly.

See also #399. I'll be happy to make that change there. That is, if @IvanMalison still wants to pursue this pull request, or if there are other users who'd like to see it merged. I'm not using any pager that shows desktop names, personally, but I'd be very happy to see this supported in WorkspaceNames and EwmhDesktops.

Also, I'd like to remark that I'm mentioned as a maintainer of WorkspaceNames and that I would have reacted a few years earlier had I been pinged by either one of you. :-))

[liskin]

I drafted this at liskin#1. Got to admit it's significantly simpler :-)

[liskin]

Oh, I just realized that you already had the simple implementation in the cover letter of #105, but it wasn't possible back then. And two years later in #238 you fixed ewmhDesktopsLogHookCustom so that the simple implementation is possible, and this PR (#122) was never updated accordingly. Anyway, now that the simple implementation is possible, I'll just include it in #399.

[liskin]

And a few months later I realized it's not as simple as I thought it was. With two separate hooks, it is, as one can pass the renaming function to the log hook only, but in #399 I deprecated the individual hooks and now I need two different remaps as in #105 again. 🤦
2e982f1
Not really a big deal I guess, but I do feel like an idiot. :-)

[pjones]

I apologize for being a bit dense, but I don't understand the background for this change. I've read the comments on the referenced PR and yet I still don't understand the motivation behind this.

Please help me understand a use case where these changes are useful. Thank you.

[geekosaur]

XMonad.Actions.WorkspaceNames provides dynamic workspace names that are separate from the workspace tags. This is mostly useful because, if you have windows on a tag and remove that tag, xmonad recreates it from the propagated StackSet and leaves the windows on it; also, and relatedly, because tags do more than just user visible workspace names (consider all the hacks out there to put icons etc. into the tag for use by various bars). An argument could be made for adding a user visible name to go with tags, but (a) this will complicate a bunch of things including backward compatibility (b) I'm not sure it addresses the topic-space workflow cited by the documentation for X.A.WorkspaceNames. (OTOH that may itself be a workaround for TopicSpaces being one of the things that often causes argument overflow on mod-q.)

…

On Wed, Dec 14, 2016 at 5:52 PM, Peter J. Jones ***@***.***> wrote: I apologize for being a bit dense, but I don't understand the background for this change. I've read the comments on the referenced PR and yet I still don't understand the motivation behind this. Please help me understand a use case where these changes are useful. Thank you. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#122 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AB8SoPz_zK2UEGE6CgKyEq6x2_i7Uyfpks5rIHM9gaJpZM4K9AMJ> .

-- brandon s allbery kf8nh sine nomine associates allbery.b@gmail.com ballbery@sinenomine.net unix, openafs, kerberos, infrastructure, xmonad http://sinenomine.net

[colonelpanic8]

I moved the functions supporting ewmh to the ewmh module (frankly, I still think it would be more appropriate to have them in WorkspaceNames, but I don't feel very strongly about it).

[pjones]

@IvanMalison Sorry for the delay. After reading @geekosaur's comment and thinking about this some more I'm wondering if it makes sense to move everything from this PR into its own module. Like you said, it doesn't really belong in EWMH, but WorkspaceNames might not be the right place either. Really, this is a way to glue those two together (if I understand correctly).

How does that sound?

[colonelpanic8]

@pjones Yeah, sure that works!

[pjones]

@IvanMalison Are you still going to move this code into its own module?

[colonelpanic8]

yep I can do that!

[liskin]

Sorry for the noise and treating you all as rubber ducks, so here's a last status on this: I figured we don't need to wait for #399 so I just pushed f271d59 to master.