rust-x-bindings / xkbcommon-rs

bindings and safe wrappers for libxkbcommon
MIT License
20 stars 24 forks source link

Create xkb_x11_setup_xkb_extension helper function #8

Open insipx opened 7 years ago

insipx commented 7 years ago

Hey, working on those examples (University got in the way, though, haha). However, I noticed that xkb_x11_setup_xkb_extension is only a helper function for setting up XKB. Currently in this library, this function can be a bit of mess to use (where Rust is concerned). Essentially, its a nice wrapper over the use_extension() function.

Since it's only a helper function, the underlying library is already nicely created (use_extension() functions, etc). I thought it would be beneficial to create this helper function in a more Rust-like manner so that it is more accessible through docs/easier to use/find.

the function i am talking about is here in the C library: https://github.com/xkbcommon/libxkbcommon/blob/5eeba0fe0da70b7cb1ac17ef0b606058e07c64ea/src/x11/util.c#L27

if approved, since im already steeped in the codebase, i could totally just put this in a place that makes sense using functions of the rust xkb library.

in fact, if this is implemented it would remove some of the boilerplate in your toy_xcb examples (talking about the Keyboard impl)

rtbo commented 7 years ago

Not sure I get you. This function is already wrapped in rust: https://github.com/rtbo/xkbcommon-rs/blob/aa97597bb737c3bb323a8972938ae7ae1c2b7e31/src/xkb/x11/mod.rs#L19 You are proposing to change the signature to something more rusty?

insipx commented 7 years ago

Yes,

Looking at the C-Code, it seems the function does nothing other than act as a wrapper around use_extension call and query_extension_reply.

I propose that instead of wrapping the C-function, we just re-write the function in Rust. Since the use_extension and query_extension_replies are already wrapped in this lib. IMO this would have a few advantages, namely making the function more 'rusty' (don't have to check for 0/1 on return like in C, better function signature, etc).

rtbo commented 7 years ago

Do you have an idea of the rust API you want? I would have seen returning an Option<(u16, u16, u8, u8)>. That clearly does not need a rewrite.