-
-
Notifications
You must be signed in to change notification settings - Fork 179
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
Initial implementation of named workspaces #314
Initial implementation of named workspaces #314
Conversation
I have been daily driving this for most of the day, and it is working surprisingly well. For my own needs, this is fine as it is, but I'll try to add a few tests in the coming days. |
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 looks very reasonable!
Maybe consider making it possible to declare named workspaces on a per-output basis (while also keeping the global declaration, which will create them on the primary display).
Maybe something like:
workspace "name" {
open-on-output "DP-2"
}
Yeah, I suppose that'd work better than |
Pushed a WIP update that changes the option name and syntax, it's now:
...and it actually does as advertised! It does open the "name" workspace on the primary output if However, this whole thing falls apart in a multi-monitor setup, because workspaces are searched per-monitor. So I had an attempt at that last week, but it got a wee bit messy. I don't remember anymore where, or how, though. I'll try again tomorrow, and see where it leads. |
Updated the PR with plenty of changes - also reflected in the updated initial commit. I am not sure how much further I can push this, the remaining todo list items feel like they require a deeper knowledge of Rust and Niri than I have. Conceptually, none of them feel hard, but I've been fighting the borrow checker and the compiler for quite a bit to even get here. I'm not sure I have it in me to go much further. Time permitting, I will have another go at the remaining stuff next weekend. For now, I'll go and test drive the changes I made in the past 24 hours =) |
Just pushed an update that changes the syntax from |
Following the discussion on matrix, I changed the syntax back to Technically it would still be possible to report it at parse time, using |
I had another go at it. It's still a bit clunky and verbose, but the parse-time error reporting is nice. And we can stick to the current syntax, too! |
I think that all functionality is in place now. The code is a tad on the ugly and inefficient side here and there, but things generally work. I'm going to test drive it today, and hopefully write some tests & clean up the code and commit history tomorrow. |
I've been daily driving this for the past three weeks, the latest iteration for the past ~6 hours or so, and am quite happy with where it is at. It's not perfect, some of the code I'm not really proud of, especially around the I have added a few invariants to There's also the question of being able to add/remove named workspaces at runtime, without adding them to the configuration file. I think this would be great for temporary named workspaces, and for experimentation. I did not implement those yet, because this PR is already big and complex, and run-time named workspaces would, I fear, open another can of worms. I do plan to implement those, but in a followup pull request instead. I wanted to write tests too, but... those parts of the code are way beyond my Rust skills. I'm afraid I will not be able to write tests. Best I can do is describe what I would test, but someone else will need to turn that into code. With all that said, I think this is as far as I can take it, so I'll be marking it as ready for review. |
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 is looking great! I don't have any big architectural changes; only smaller fixes and refactors.
Barring dogfooding, this should now be ready. I put all my changes into separate commits for now. |
This is an implementation of named, pre-declared workspaces. With this implementation, workspaces can be declared in the configuration file by name: ``` workspace "name" { open-on-output "winit" } ``` The `open-on-output` property is optional, and can be skipped, in which case the workspace will open on the primary output. All actions that were able to target a workspace by index can now target them by either an index, or a name. In case of the command line, where we do not have types available, this means that workspace names that also pass as `u8` cannot be switched to by name, only by index. Unlike dynamic workspaces, named workspaces do not close when they are empty, they remain static. Like dynamic workspaces, named workspaces are bound to a particular output. Switching to a named workspace, or moving a window or column to one will also switch to, or move the thing in question to the output of the workspace. When reloading the configuration, newly added named workspaces will be created, and removed ones will lose their name. If any such orphaned workspace was empty, they will be removed. If they weren't, they'll remain as a dynamic workspace, without a name. Re-declaring a workspace with the same name later will create a new one. Additionally, this also implements a `open-on-workspace "<name>"` window rule. Matching windows will open on the given workspace (or the current one, if the named workspace does not exist). Signed-off-by: Gergely Nagy <niri@gergo.csillger.hu>
Added docs. |
This is an implementation of named, pre-declared workspaces. With this implementation, workspaces can be declared in the configuration file by name:
The
open-on-output
property is optional, and can be skipped, in which case the workspace will open on the primary output.All actions that were able to target a workspace by index can now target them by either an index, or a name. In case of the command line, where we do not have types available, this means that workspace names that also pass as
u8
cannot be switched to by name, only by index.Unlike dynamic workspaces, named workspaces do not close when they are empty, they remain static. Like dynamic workspaces, named workspaces are bound to a particular output. Switching to a named workspace, or moving a window or column to one will also switch to, or move the thing in question to the output of the workspace.
When reloading the configuration, newly added named workspaces will be created, and removed ones will lose their name. If any such orphaned workspace was empty, they will be removed. If they weren't, they'll remain as a dynamic workspace, without a name. Re-declaring a workspace with the same name later will create a new one.
Additionally, this also implements a
open-on-workspace "<name>"
window rule. Matching windows will open on the given workspace (or the current one, if the named workspace does not exist).