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

[draft] workspace layouts #755

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

[draft] workspace layouts #755

wants to merge 8 commits into from

Conversation

Quelklef
Copy link

Description


This PR introduces "workspace layouts", which allow non-standard organization of the workspaces themselves, such as alignment into a grid, or combination of multiple workspaces.

The modules (at the time of PR creation) are as follows:

  • XMonad.Util.OneState and XMonad.WorkspaceLayout.Util -- Utility modules
  • XMonad.WorkspaceLayout.Core -- Defines functionality shared between workspace layouts
  • XMonad.WorkspaceLayout.Grid -- Defines a 2d-grid workspace layout. This is like XMonad.Actions.Plane, but not hacky and more featurful. The grid layout allows the user to organize their workspaces into a grid and for multiple coordinates on the grid to map to the same workspace.
  • XMonad.WorkspaceLayout.Cycle -- Defines a cyclic workspace layout. This module is intended to be more of a demonstration than anything. Its use for me during development was to help mitigate the risk of me over-coupling generic code with the grid layout.

Let me elaborate a bit on the deal with the grid layout by way of giving the example of my own current XMonad layout. I have a grid which is 10 cells across, and 4 cells tall. Altogether it looks like this:

y=1 | α 1 2 3 4 γ 6 7 8
y=2 | α 1 2 3 4 γ 6 7 8
y=3 | α 1 2 3 4 γ 6 7 8
y=4 | α 1 2 3 4 γ 6 7 8

This means that I have a grid of 32 cells total. Each row has the same names for its cells but these cells are distinct.

As I write this, I am positioned on cell α of row y=1:

image

Using keybindings MOD+n for a number n, I can move around the row y=1 to other workspaces on that row. If I press MOD+3, for instance, I move to workspace 3 on row y=1, which contains some terminals:

image

To move around the y-axis, I press MOD+s to increment y and MOD+w to decrement it. If I increment y twice to reach row y=3, my status bar looks like this:

image

This row contains a different set of workspaces than y=1, hence the difference in coloration for the labels.

It's also worth noting that I have the α and γ cells in each row to be linked together. This means that if I am on row y=2 and open a terminal in cell γ, and then I move to cell γ of row y=3, the same terminal will be there. Under the hood, the workspaces y=2 γ and y=3 γ are actually the same workspace.


My intent with the inclusion of the namespace XMonad.WorkspaceLayout and the module XMonad.WorkspaceLayout.Core is to encourage other XMonad contributors to possibly create and add their own workspace layouts besides just Grid and Cycle. (As it so happens, XMonad.WorkspaceLayout.Core turned out to be minimal in terms of code, but so be it.)


This PR is far from complete. The checklist below enumerates a number of outstanding tasks.

However, I wanted to open it earlier rather than later to start receiving feedback as soon as possible. Some questions include:

  • Is there interest for the grid and cycle layouts?
  • Is there interest for the generic "workspace layout" abstraction and the XMonad.WorkspaceLayout namespace?
  • Should the "workspace layout" abstraction be developed, so that the XMonad.WorkspaceLayout.Core affordance is not so ... lacking?

Thanks!

post-script. I recognize that I haven't include any example client code that uses this PR. As of right now, the best example I have is my own XMonad config; see specifically how I hook into XMonad, hook into my status bar, and deifne my keybindings.


Checklist

  • Read CONTRIBUTING.md
  • Documentation: comments + examples
  • Manual tests performed
  • xmonad-contrib tests still pass
  • CHANGES.md updated
  • hlint passes
  • PR generates no new warnings

@slotThe
Copy link
Member

slotThe commented Sep 19, 2022

Just chiming in to say that this looks really cool! I don't have the capacity to review this in full just now, but it's definitely on my radar!

{-# LANGUAGE TypeFamilies #-}
{-# OPTIONS_GHC -Wall -Werror #-}

module XMonad.Util.OneState
Copy link
Member

Choose a reason for hiding this comment

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

One quick thing: the implementation of this is pretty neat, the only question I have is whether we actually want to unify ExtensibleState and ExtensibleConf into one thing. Personally, I find the current situation quite convenient in terms of having a mental model of what a given piece of code can do, but maybe I'm alone in that (Cc. @liskin as the author of ExtensibleConf)

Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's much about the situation being convenient or not. It's just a very elaborate workaround for not being able to initialise/modify (put) ExtensibleState in config-time. Or, to be more precise, not having a nice interface for it, as one can indeed do it using startupHook, but that's ugly, and composes poorly.

(I have yet to take a look at the rest of the code so I don't really have an opinion whether this elaborate workaround is worth it or whether there are easier ways to solve the same problem.)

Copy link
Author

Choose a reason for hiding this comment

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

I believe OneState was created for two reasons. (1) I found combining state and config to be conceptually simpler and provide more flexibility for free (and at the time I wasn't too focused on thinking about merging into xmonad-contrib); and (2) I wanted to allow modifying the config at runtime, similar to what @liskin is saying.

Currently I don't think (2) is actually used. Originally I wanted to use it in order to allow adjustments to the grid layout at runtime, so you could, for instance, add and remove rows on-demand. I still think this would be worthwhile, although it's not on my TODO list at this moment.

Copy link
Member

Choose a reason for hiding this comment

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

Do we want the config to be modifiable at runtime though? I'm not sure

as one can indeed do it using startupHook, but that's ugly, and composes poorly.

@liskin could you elaborate? I mean, we can have a nice interface where we internally collect all defaults from the config, and compose then apply them in the startupHook. Might need some type magic, but I don't see why that's a problem.

Copy link
Member

@liskin liskin Oct 5, 2022

Choose a reason for hiding this comment

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

Yeah, we can have an interface, and OneState is one such interface. Although it doesn't use startupHook, but there's a good reason for it: it really does compose poorly. You can't guarantee that a specific part of startupHook runs before everything else, so if you want to make a general-purpose ExtensibleState-like interface that can be initialised in config-time, you need to make it work even if the hook hasn't run yet. So you need to do exactly what OneState does: look into ExtensibleState and fall back to ExtensibleConf, every single time you access it.

If you didn't need a general-purpose interface and you knew the order of startupHooks doesn't matter in your specific case, then it's okay to just do a single ExtensibleConf → ExtensibleState sync in startupHook. It's not safe to do this in a general-purpose interface though.

Anyway, looks like nobody really needs this functionality now… :-)

Copy link
Author

Choose a reason for hiding this comment

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

Anyway, looks like nobody really needs this functionality now… :-)

Well, I would like to export an all-powerful (State -> State) -> X () function from Grid so that users may modify their grid layout at runtime if they would like. (While one can certainly do without such a capability, I think it's also a reasonable desire)

XMonad/WorkspaceLayout/Core.hs Outdated Show resolved Hide resolved
Comment on lines +3 to +4
(!%) :: [a] -> Int -> a
xs !% n = xs !! (n `mod` length xs)
Copy link
Member

Choose a reason for hiding this comment

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

Is this a common operator? If so, let's add it to XMonad.Prelude

Copy link
Author

Choose a reason for hiding this comment

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

I wouldn't be surprised if it's pretty common, but as far as I know it doesn't have any standard name

Comment on lines +301 to +302
hook :: Init -> XConfig l -> XConfig l
hook Init { initMapping, initWrapping, initLabelf } =
Copy link
Member

Choose a reason for hiding this comment

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

This should be named differently

Copy link
Author

Choose a reason for hiding this comment

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

Fine by me. Suggestions?

Copy link
Member

Choose a reason for hiding this comment

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

People commonly import xmonad-contrib modules unqualified, so the user-facing functions should be named in a way that works without the module name. In this case, gridWorkspaceLayout is probably best.

You'd need to unexport or rename all the other functions too… Perhaps it'd be okay to include a Usage section (most if not all other modules do have such a section, so you should have one here too) that instructs users to import it qualified, and then the names could stay.

Copy link
Member

Choose a reason for hiding this comment

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

I'm against using qualified imports here, it's really not something that the modules do.

Copy link
Author

Choose a reason for hiding this comment

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

Is the consensus to switch to unqualified-import-friendly names? Exporting something like

gridGrid :: GridDims -> GridMapping 'GridFormatted

seems very clunky to me. And I don't think it's the responsibility of the library author to manage the user's imports.

That being said, I'll follow consensus

{-# LANGUAGE TypeFamilies #-}
{-# OPTIONS_GHC -Wall -Werror #-}

module XMonad.Util.OneState
Copy link
Member

Choose a reason for hiding this comment

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

Do we want the config to be modifiable at runtime though? I'm not sure

as one can indeed do it using startupHook, but that's ugly, and composes poorly.

@liskin could you elaborate? I mean, we can have a nice interface where we internally collect all defaults from the config, and compose then apply them in the startupHook. Might need some type magic, but I don't see why that's a problem.

Copy link
Member

@TheMC47 TheMC47 left a comment

Choose a reason for hiding this comment

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

I skimmed the files, and I think the functionality is cool! Good job on your first contribution 🎉 This is in no way a thorough review, and I would need some documentation to help me navigate the code. I left some small comments/conversation starters.

As a general comment: I'm not sure about XMonad.Util.OneState. And I think the WorkspaceLayout.* shouldn't be a top-level submodule of XMonad. XMonad.Util might be a good fit.

All in all, solid work 👏

hook :: Init -> XConfig l -> XConfig l
hook Init { initMapping, initWrapping, initLabelf } =
St.once @State
(\xc -> xc { XMonad.workspaces = workspaces })
Copy link
Member

Choose a reason for hiding this comment

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

I'm torn about this interface. On the one hand, it's cool because you have a single entry point to hook the module to your configuration, which makes it easy to add and remove. On the other hand, this behavior might be confusing since it just throws away the workspaces in the initial config. It might be okay though since I can't think of an ergonomic behavior

Copy link
Author

@Quelklef Quelklef Oct 3, 2022

Choose a reason for hiding this comment

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

Yeah, it's kind of a fundamental limitation of my design that the code needs to have control over your workspaces.

The good news is that by initializing with a Mapping 'Unformatted, the user can choose what the workspace names should be. So if they already have a list of workspaces, then with the right choice of Init, the call to hook will not change those workspaces.

That fact could use some documentation though : P

Copy link
Author

Choose a reason for hiding this comment

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

Also, it's worth noting that this "single entrypoint" is not as clean as it may seem. For effective use of XMonad.WorkspaceLayouts.Grid one has to (a) use hook; (b) modify their pretty-printer; and (c) setup keybindings

Copy link
Member

Choose a reason for hiding this comment

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

IMO it's fine to replace workspaces. Might be nice to mention it in the docs, but it's okay to do it.

Copy link
Contributor

Choose a reason for hiding this comment

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

X.L.IndependentScreens also rewrites workspaces, although it does so by having you call a function explicitly. Maybe there's something here in terms of design that could be used to simplify use of both? (IndependentScreens is currently pretty invasive and difficult to set up.)

@liskin
Copy link
Member

liskin commented Oct 5, 2022

And I think the WorkspaceLayout.* shouldn't be a top-level submodule of XMonad. XMonad.Util might be a good fit.

Don't want to bikeshed too much but can't resist here: this WorkspaceLayout stuff is closer to XMonad.Layout.IndependentScreens than to anything in XMonad.Util. Arguably IndependentScreens is not a window layout, it's more of a … workspace layout. If none of this already existed I'd probably put it in XMonad.Actions because all this workspace layout logic mostly happens in keybindings that switch workspaces, but that's just me trying to fit this into the existing namespace.

Anyway, that's just my thoughts. Not a suggestion, not a conclusion, just a thought experiment. :-)

@osv
Copy link

osv commented Nov 3, 2022

@Quelklef Hi, I tried few days ago KDE "activities" feature first time, and I want to share some ideas with you.

What activity-like features I would like to have:

  • different workspace list per activity. Similar to XMonad.Actions.TreeSelect, workspace name can be separated via ".", so no need 2 dimensions (name like "Work.Proj1.Term" is also allowed - activity name will be "Work.Proj1") e.g:
[ "Relax.WWW"        "Relax.DOC",     "Relax.Gimp"
 ,"Work.Term",       "Work.WWW"       "Work.Editor",       "Work.etc"
 ,"MyProjects.Term", "MyProjects.WWW" "MyProjects.Editor", "MyProjects.etc"
 ,"Study.WWW",       "Study.Doc",     "Study.Doc" ]
  • Hook when switching between activities (e.g changing from "Relax" to "Work").
    I can have different statusbars with different widgets. I think it possible to hide/show them depending on workspace activity prefix name (spawn xdotool, wmctl?).
    I can change background when workspace's activity is changed.
  • When switching "activity" (move Y) return to last active workspace.
  • Some label/indicator in statusbar when I have windows not in current activity, or/and count of windows not in current activity.

Again, this is just ideas and not request for change and might be done in another module but it is very close to your module and you might want to have smtg from above list for yourself needs

@Quelklef
Copy link
Author

Quelklef commented Nov 4, 2022

Hi @osv, thank you for the comment! Let me make sure I am understanding you correctly. You're thinking of using the y-axis in order to keep track of what "activity" you are engaging with. And you are thinking that:

  1. On different activities, workspaces can have different names. So in the Relax acitivity workspaces would be WWW, DOC, Gimp but in Work workspaces would Term, WWW, Editor, etc.
  2. There would be a hook for detecting a change of activity
  3. When switching activities the previously-active workspace would be automatically active
  4. There would be some kind of indication regarding how many active windows there are not shown in the current activity

These are some interesting suggestions. Here are my thoughts --

  1. You can already do this with the current WorkspaceLayout implementation. In my examples, workspaces are named after their x-coordinate, but in actuality you can name them whatever you want :P
  2. The WorkspaceLayout implementation on its own does not control when workspaces are changed; the user does. The user has to configure, for instance, that Super+W moves the y-coordinate down by one. If the user also wants something to happen on workspace change, they can add that code to the callsite. In other words, they can change the (hypothetical) callsite on "Super+W" (changeCoordinate addOneToY) to on "Super+W" (changeCoordinate addOneToY >> doSomethingElse)
  3. I think the user could technically add this themselves, but it seems like a reasonable thing to have built-in
  4. A user should be able to configure this themselves using neighborhood and label from WorkspaceLayoutView. That being said, it might also be a good addition to WorkspaceLayout proper, assuming it doesn't require too much code to implement

I probably won't implement these right now, since it's unclear to me whether or not this PR is headed towards merging or not anyway. If it is eventually merged (or it is known that it will eventually be merged), I'd be happy to look more into implementing these =)

@osv
Copy link

osv commented Nov 4, 2022

  1. Yes, different count of activities namespaces (all what is need for it, as I understand, just new ppRename function to hide namespaces not in current activity)
  2. I would like to have hook when activity is changed, it will be more integrated solution with other modules I think (new separate hook needed).
  3. This is more question where to go when you are moving in Y. First workspace in list? Probably in most cases user would like return back to the last one used so need to keep state of last visited activiries (one more hook)
  4. Yes, it possible to do with module in this PR. This draft have no example in doc (this is draft, so it is ok maybe) how to use, so I need to check your dotfiles, but better add synopsis/example of usage in modules directly)

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.

6 participants