rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
161 stars 63 forks source link

Unclear requirements for XTest fake input #238

Closed jjholt closed 10 months ago

jjholt commented 1 year ago

I'm quoting the signature for XTest's FakeInput below.

From reading examples, I gather that type is XCB_KEY_PRESS, XCB_BUTTON_PRESS, etc. Xproto has definitions for those constants, which I assume is what is expected.

Issue #219 is an alternative, but I assume either creating constants or passing events directly would be a far better solution. Thoughts?

xcb::xtest::FakeInput {
    r#type: u8,
    detail: u8,
    time: u32,
    root: Window,
    root_x: i16,
    root_y: i16,
    deviceid: u8,
}
RealKC commented 1 year ago

Maybe xcb could export an API that looks like this?

impl FakeInput {
    pub fn for_event<E: base::Event>(detail: u8, time: u32, root: Window, root_x: i16, root_y: i16, deviceid: u8) -> Self {
        Self {
            r#type: E::NUMBER,
            detail,
            time,
            root,
            root_x,
            root_y,
            deviceid,
       }
    }
}
jjholt commented 1 year ago

Alternatively it's just a trait autoimplemented for most base events. The only issue is that KeyReleaseEvent is a type alias of KeyPressEvent. Something like:

trait EventNumber {
    fn event_number(&self) -> u8 where Self: xcb::BaseEvent {
        Self::NUMBER as u8
    }
}
rtbo commented 1 year ago

The only issue is that KeyReleaseEvent is a type alias of KeyPressEvent

Yes this will be a problem. Adding event number definitions such as xcb::x::KEY_PRESS or xcb::x::BUTTON_PRESS is not a big headache and would probably help.

jjholt commented 1 year ago

I'm trying to understand the xml-to-rust pipeline so I can create these constants, and I figured why not test making KeyPressEvent and KeyReleaseEvent two different structs? Copy and pasted the xml definition, changed the name and number, and as far as I can tell everything works as expected.

What was the reasoning for making Release events always a type alias of Press? I'm completely out of my depth here, but is it not simpler to separate the two events? then the existent event::NUMBER is already the solution

Edit: It hadn't clicked that NUMBER was what I had been looking for. Making it consistent and then adding an example seems like the right thing to do

rtbo commented 1 year ago

What was the reasoning for making Release events always a type alias of Press?

In Xlib, the two events are the same type:

typedef struct {
    int type;       /* KeyPress or KeyRelease */
        /* other fields */
} XKeyEvent;
typedef XKeyEvent XKeyPressedEvent;
typedef XKeyEvent XKeyReleasedEvent;

In the XCB xml definition, we have something similar:

  <event name="KeyPress" number="2">
    <!-- fields -->
  </event>

  <eventcopy name="KeyRelease" number="3" ref="KeyPress" />

So it is quite natural that in Rust, the eventcopy element generates a typedef. But there is nothing preventing the generation of a new type (exactly the same as KeyPressEvent). Apart maybe Semver, because I don't really want to have version 2. I'm not quite sure where is the boundary between minor and major version.