-
-
Notifications
You must be signed in to change notification settings - Fork 275
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
base: master
Are you sure you want to change the base?
Conversation
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 |
There was a problem hiding this comment.
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)
There was a problem hiding this comment.
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.)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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… :-)
There was a problem hiding this comment.
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)
(!%) :: [a] -> Int -> a | ||
xs !% n = xs !! (n `mod` length xs) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
hook :: Init -> XConfig l -> XConfig l | ||
hook Init { initMapping, initWrapping, initLabelf } = |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine by me. Suggestions?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this 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 }) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.)
Don't want to bikeshed too much but can't resist here: this Anyway, that's just my thoughts. Not a suggestion, not a conclusion, just a thought experiment. :-) |
@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:
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 |
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:
These are some interesting suggestions. Here are my thoughts --
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 =) |
|
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
andXMonad.WorkspaceLayout.Util
-- Utility modulesXMonad.WorkspaceLayout.Core
-- Defines functionality shared between workspace layoutsXMonad.WorkspaceLayout.Grid
-- Defines a 2d-grid workspace layout. This is likeXMonad.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:
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 rowy=1
:Using keybindings
MOD+n
for a numbern
, I can move around the rowy=1
to other workspaces on that row. If I pressMOD+3
, for instance, I move to workspace3
on rowy=1
, which contains some terminals:To move around the y-axis, I press
MOD+s
to incrementy
andMOD+w
to decrement it. If I incrementy
twice to reach rowy=3
, my status bar looks like this: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 rowy=2
and open a terminal in cellγ
, and then I move to cellγ
of rowy=3
, the same terminal will be there. Under the hood, the workspacesy=2 γ
andy=3 γ
are actually the same workspace.My intent with the inclusion of the namespace
XMonad.WorkspaceLayout
and the moduleXMonad.WorkspaceLayout.Core
is to encourage other XMonad contributors to possibly create and add their own workspace layouts besides justGrid
andCycle
. (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:
XMonad.WorkspaceLayout
namespace?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
xmonad-contrib
tests still passCHANGES.md
updatedhlint
passes