Use x11rb instead of rust-xcb #31
Conversation
| (attrs.map_state() & xcb::MAP_STATE_VIEWABLE as u8) != 0 | ||
| fn viewable<C: Connection>(conn: &C, win: xproto::Window) -> bool { | ||
| let attrs = xproto::get_window_attributes(conn, win).unwrap().reply().unwrap(); | ||
| attrs.map_state == xproto::MapState::Viewable |
There was a problem hiding this comment.
map_state is an enum variant so using == works instead of checking bits
There was a problem hiding this comment.
(this should probably go in a source code comment)
There was a problem hiding this comment.
It was an explanation for why the change is valid.
|
Looks good overall and I trust you, I'll take a deeper look in the morning. |
|
and yes do rustfmt |
| @@ -17,15 +20,21 @@ fn min_max(a: i16, b: i16) -> (i16, i16) { | |||
| } | |||
| } | |||
|
|
|||
| fn build_guides(screen: xcb::Rectangle, pt: xcb::Point, width: u16) -> [xcb::Rectangle; 2] { | |||
| fn build_guides(screen: xproto::Rectangle, pt: xproto::Point, width: u16) -> [xproto::Rectangle; 2] { | |||
| [ | |||
There was a problem hiding this comment.
I prefer splitting out rectangle into their own definition first since that would make them one line IIRC.
|
Mildly related to this PR, we should probably change a fair bit of the unwraps to expects so we can have a decent error message if it crashes |
| | xproto::EventMask::StructureNotify | ||
| | xproto::EventMask::SubstructureNotify, | ||
| ) | ||
| .override_redirect(1); |
There was a problem hiding this comment.
Comment was lost here? "Don't be window managed"
There was a problem hiding this comment.
The ?RGB comment was also lost. Keypress comment isn't needed anymore since we do use the event. What does it mean to be window managed?
There was a problem hiding this comment.
that the window manager doesn't manage this window
Yes, maybe it would be benefical for us to adopt an application-focused error library so we could easily propagate up with context. Not sure what the state of best practices is right now. |
|
I forgot about this PR |
|
Are there any plans to merge this? Is this PR incomplete? |
|
There just hasn't been much motivation to complete it. It'd be nice though. |
Agreed. We can have something along the lines of this: https://github.com/samyak-jain/hacksaw/tree/lib but using x11rb. I'd be happy to contribute. I think as a library it probably makes sense to propogate custom error types instead of using anyhow. |
If we have an idea on what's left to be completed. I'd be happy to help. I don't know if anyone would have the bandwidth to review it though. |
|
The maintainers can help to review but since not all of us still use it now there may be less reviewers. |
This, I've been using sway for two years now so I can't use hacksaw anymore, so it's both lack of interest and limited ability to test for me. Can't speak for the others but I think @JuanPotato and @expectocode still use it though. |
I managed to typo in the commit. nice. Anyway this switches our xcb library to x11rb because it's very nice.
I tried to leave as much of the code as similar to the original as possible to make the diff easier to review.