psychon / x11rb

X11 bindings for the rust programming language, similar to xcb being the X11 C bindings
Apache License 2.0
372 stars 41 forks source link

Improve ergonomics of the AUX struct #10

Open psychon opened 5 years ago

psychon commented 5 years ago

Currently, an instance of the corresponding Aux struct has to be created and then passed to the request, like so:

https://github.com/psychon/x11rb/blob/bcfe334da2aada4a0142213d1773517597cbd4a3/examples/simple_window.rs#L23-L26 https://github.com/psychon/x11rb/blob/bcfe334da2aada4a0142213d1773517597cbd4a3/examples/simple_window.rs#L33

This API is similar to what Xlib does and what libxcb does (when one opts-in to use this extra support, which apparently no one does). Still, it "feels weird" to have to write "create window" twice. Can this be improved somehow?

psychon commented 4 years ago

Slightly related: AsRef or Borrow could be used for the argument in create_window. That would at least get rid of the & in the call.

dalcde commented 4 years ago

I can think of two ways to avoid this:

conn.create_window(24, win_id, screen.root, 0, 0, width, height, 0, WindowClass::InputOutput, 0)
    .event_mask(EventMask::Exposure | EventMask::StructureNotify | EventMask::NoEvent) 
    .background_pixel(screen.white_pixel) 
    .win_gravity(Gravity::NorthWest)
    .send();

This should produce a warning if the "builder object" is not sent (similar to unused Results). We can also provide a discard operation in case someone changes their mind after producing the builder.

The other way is to do something like

CreateWindowAux::new() 
     .event_mask(EventMask::Exposure | EventMask::StructureNotify | EventMask::NoEvent) 
     .background_pixel(screen.white_pixel) 
     .win_gravity(Gravity::NorthWest)
     .send(&conn, 24, win_id, screen.root, 0, 0, width, height, 0, WindowClass::InputOutput, 0)

I personally find the former more intuitive, but it might lead to inconsistencies between the actions that have an AUX and those that don't.