Refactor IPC to better facilitate scripting(?) use-cases of the niri-ipc crate #294
+598
−306
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Based on work in #278.
This basically reimplements the way niri-ipc currently works, in a way that is not necessarily objectively better. I've marked this as a draft for now, given that i still need to figure out a couple things.
Some of the current issues that should be ironed out:
Naming. One of the hard problems in computer science. What the heck are the names of
HandleRequest
andMsgRequest
meant to be (insrc/ipc/server.rs
,src/ipc/client.rs
, respectively)? Does theRequest
suffix for all request types make sense? Should it be a namespace?In the interest of not outright requiring all potential consumers of niri ipc to rewrite their logic, i've tried to keep an
AnyRequest
enum still functioning. This, complicates things, given that the response type is of course dependent on the request type, so i have an associated functiondecoder()
whose sole purpose is capturing the state necessary to determine what kind of response we're going to read (and for the actual request types, that is a ZST). I would love to remove this, but i didn't yet because i'm not sure if we do want to support such a use case anyways (if a client wants, they could make their own request enum, after all)Make
RequestMessage
private? TheRequestType
tag is kind of an implementation detail. It's only necessary for serialization. Currently, it's exposed, though. A side effect of separating the types of each request and response is that you can no longer enumerate all requests on the client side, which i don't think is actually problematic. The motivation for doing this is to make a somewhat simpler IPC protocol, where we aren't relying on a representation of Rust sum types (because, the representation differs by whether it's a unit variant or similar). I'm not sure this is actually the best way to do it.Heavy reliance on
serde_json::Value
. Is this problematic? it feels dirty, but i'm also not exactly serializing them as real json strings, so it could be worse.Re-implement long-lived sockets (i.e. handle many requests per socket)
With these changes, i've taken care not to alter the behaviour of the
niri msg
utility. The actual logic behind it has only two changes:current_mode
will result inCurrent mode: (invalid index)
instead of exiting the entire operation and printing no useful information about outputsOutputRequest
now returns aBTreeMap
instead of aHashMap
. This is becauseniri msg
only ever sorted it before iterating, so it might as well come pre-sorted to save some overhead at that point.Error handling may or may not be identical, but i did try to keep the behaviour as similar to before as possible.
And one great functionality of the code as is: All I/O logic happens in a distinct phase before any printing. In between them, i disable
SIGPIPE
, which is similar to what niri-ipc did before i ever touched it