Add a window list to IPC workspace messages #506
Conversation
| @@ -237,3 +226,20 @@ async fn process(ctx: &ClientCtx, request: Request) -> Reply { | |||
|
|
|||
| Ok(response) | |||
| } | |||
|
|
|||
| pub fn smithay_window_to_ipc(window: &smithay::desktop::Window) -> niri_ipc::Window { | |||
There was a problem hiding this comment.
Ideally I'd have wanted this function to be a method on either niri_ipc::Window or on smithay::desktop::Window
However, doing that would either require adding smithay and some other stuff as deps of niri-ipc, or adding an extension trait which would be impl'd on smithay windows. That was extra annoying since methods are being called on the window which requires more work.
For now, this is the cleanest solution I came up with
|
|
||
| impl Layout<Mapped> { |
There was a problem hiding this comment.
Dragging ipc_workspaces out of the impl<W: LayoutElement> block isn't super pretty, but without this, I can't access any window information here as far as I can tell
| @@ -523,6 +523,8 @@ pub struct Workspace { | |||
| pub output: Option<String>, | |||
| /// Whether the workspace is currently active on its output. | |||
| pub is_active: bool, | |||
| /// The windows currently on this workspace | |||
| pub windows: Vec<Window>, | |||
There was a problem hiding this comment.
This is one of the things that I'm not sure what the best design is. We have a MappedId now which we can add to niri_ipc::Window (maybe not right away, need to think some more if it should be u32 or u64). So we can have this as essentially pub windows: Vec<MappedId>, thereby avoiding sending a lot of extra data (niri_ipc::Window will surely grow in size in the future) along with every workspace. Then there can be another IPC call to get window info. This will be similar to how output is Option<String> and not Option<Output>.
There was a problem hiding this comment.
It does mean there's a race between niri msg workspaces and niri msg windows (or something), but then there will be the event stream IPC to solve that cleanly
There was a problem hiding this comment.
Oh yeah, that sounds like a pretty good solution, I'm not sure what the overhead of IPC is but having too large things sent over doesn't seem ideal
|
I think the event stream IPC should cover this case, could you test #453 to see if you can get what you wanted to work?
|
|
I added |
|
Excellent, that should satisfy my use case so I'll close this |
I want workspaces to list the windows they contain so I can have icons for the programs active on each workspace. This should make that work for my usecase, but I'm not sure if this is the "correct" way to implement this.