Split out a x11rb-protocol crate #664
Conversation
This commit splits the code into two crates: "x11rb-protocol" contains the protocol definitions and allows parsing and serialising things. It does not do any actual I/O. "x11rb" uses x11rb-protocol and implements an X11 client based on this. I tried to do as few changes as possible here. Code moves around, but the structure of the code is not supposed to change. There is one place where this did not work out: Previously, e.g. CreateWindowRequest had an associated function send(). This function send the request that it is called on via a given X11 connection. Since this does I/O, this functionality does not belong into the x11rb-protocol crate. Since it is an associated function, it cannot easily be provided by x11rb. For now, I worked around this problem by turning send() into a "free" function. The resulting name collision was dealt by renaming it to e.g. send_create_window(). The result of all this is a huge commit. This is still a mess and things should be cleaned up further in the future, but I didn't want to make this commit even larger. This is the first version that I managed to get to compile. Signed-off-by: Uli Schlachter <[email protected]>
error[E0277]: `[&mut generator::output::Output; 2]` is not an iterator
--> generator/src/generator/mod.rs:26:16
|
26 | for out in [&mut main_proto_out, &mut main_x11rb_out] {
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ borrow the array with `&` or call `.iter()` on it to iterate over it
|
= help: the trait `std::iter::Iterator` is not implemented for `[&mut generator::output::Output; 2]`
= note: arrays are not iterators, but slices like the following are: `&[1, 2, 3]`
= note: required by `std::iter::IntoIterator::into_iter`
Signed-off-by: Uli Schlachter <[email protected]>
A wrong/old crate name was used in a doc test.
---- src/protocol/xproto.rs - protocol::xproto::GetPropertyReply::value16 (line 8909) stdout ----
error[E0433]: failed to resolve: use of undeclared crate or module `x11rb`
--> src/protocol/xproto.rs:8911:13
|
4 | let reply = x11rb::protocol::xproto::GetPropertyReply {
| ^^^^^ use of undeclared crate or module `x11rb`
Signed-off-by: Uli Schlachter <[email protected]>
I did receive the mails, thanks for keeping me in the loop although I did not participate in the thread. I think it is a good idea the "protocol". In the future it might be also useful to implement a server library or a protocol analyzer. I'll try to review this now. |
|
Nice! I'll work on building up a "sans-I/O" client inside of this. |
| @@ -23,7 +23,7 @@ pub(crate) fn generate(module: &xcbgen::defs::Module) -> Vec<Generated> { | |||
|
|
|||
| let mut main_proto_out = Output::new(); | |||
| let mut main_x11rb_out = Output::new(); | |||
| for out in [&mut main_proto_out, &mut main_x11rb_out] { | |||
| for out in vec![&mut main_proto_out, &mut main_x11rb_out].into_iter() { | |||
There was a problem hiding this comment.
I guess we should leave a comment about this in #538.
| let buf = bufs.iter().flat_map(|buf| buf.iter().copied()).collect(); | ||
| (buf, fds) | ||
| } | ||
| fn send_enable<'c, Conn>(req: EnableRequest, conn: &'c Conn) -> Result<Cookie<'c, Conn, EnableReply>, ConnectionError> |
There was a problem hiding this comment.
Why split this part into a send_* function?
There was a problem hiding this comment.
It was already a send function before, but it was an associated function.
impl EnableRequest {
pub fn send<Conn>(self, conn: &Conn) -> Result<Cookie<'_, Conn, EnableReply>, ConnectionError>
where
Conn: RequestConnection + ?Sized,
{
let (bytes, fds) = self.serialize_impl(major_opcode(conn)?);
let slices = bytes.iter().map(|b| IoSlice::new(&*b)).collect::<Vec<_>>();
conn.send_request_with_reply(&slices, fds)
}
}In "the new world", EnableRequest comes from another crate (x11rb-protocol) and thus x11rb cannot add a function to it. This is why I turned this into send_enable() (the rename is to avoid name conflicts with the other send functions).
I agree that this is weird and should be changed. Somehow. However, I tried to do the minimum number of changes to get things to compile.
Random quick thought for a next step would be: Get rid of these useless send_enable function. With this, we lose the feature to send requests from a struct (I had to make a change to this effect in src/image.rs). To get this feature back, some new API on connection would be needed, something like send_with_reply_from_trait<R>(&self, request: R) where R: ReplyRequest.
There was a problem hiding this comment.
I'm fine with leaving it as is for now.
This commit splits the code into two crates:
"x11rb-protocol" contains the protocol definitions and allows parsing
and serialising things. It does not do any actual I/O.
"x11rb" uses x11rb-protocol and implements an X11 client based on this.
I tried to do as few changes as possible here. Code moves around, but
the structure of the code is not supposed to change.
There is one place where this did not work out:
Previously, e.g. CreateWindowRequest had an associated function send().
This function send the request that it is called on via a given X11
connection. Since this does I/O, this functionality does not belong into
the x11rb-protocol crate. Since it is an associated function, it cannot
easily be provided by x11rb.
For now, I worked around this problem by turning send() into a "free"
function. The resulting name collision was dealt by renaming it to e.g.
send_create_window().
The result of all this is a huge commit. This is still a mess and things
should be cleaned up further in the future, but I didn't want to make
this commit even larger. This is the first version that I managed to get
to compile.
Signed-off-by: Uli Schlachter [email protected]
CC @notgull
@eduardosm Did you get the mails? Do you have some suggestions/doubts or is this change okay/sensible?