[Xcb] splitting xcb/util

[Ian Osgood]

On Jul 16, 2010, at 9:13 PM, Jamey Sharp wrote:

> On Fri, Jul 16, 2010 at 8:43 PM, Michael Stapelberg
> <michael+xcb at stapelberg.de> wrote:
>> Excerpts from Jamey Sharp's message of 2010-07-17 02:45:50 +0200:
>>> If you can explain what's you're actually finding hard about the code
>>> you're writing, perhaps then we can discuss what to do about it? I'm
>>> always happy to explore better APIs.
>> Well, I need to handle the response_type of an event myself which involves
>> several assertions and a bitwise operation which is unclear to me yet (why
>> is it necessary?). Also, it’s about 20 lines of boilerplate code which does
>> not actually handle any event. In my opinion, it could well be hidden in a
>> library.
> 
> You only really need to do two things: check whether this is an error or
> event, and if it's an event, mask off the SendEvent bit. That
> most-significant bit of the response type indicates whether the event
> was generated by the server or is being forwarded from another client.
> It's just part of the X protocol, and like most of the important details
> of XCB programming, it's documented in the X protocol specification.
> 
> You don't need to assert that an 8-bit value is less than 256, and I
> wouldn't bother with the other assertions either.
> 
> That leaves you with a pretty simple structure in your event loop. If
> you want to use if statements instead of switch, which certainly has the
> advantage of handling extension events more smoothly, you might do it
> this way:
> 
> if(event.response_type == 0)

You mean event.response_type == XCB_ERROR?  If users need to know this constant in order to perform proper event handling, perhaps it should be moved out of libxcb/src/xcb_in.c into xcb.h.  (But do they even need to handle this case? I thought xcb_in.c shuffled errors into their own queue.)

> {
>    /* handle errors; you're doing this with: */
>    exit(1);
> }
> uint8_t type = event.response_type & 0x7F;

You mean type = XCB_EVENT_RESPONSE_TYPE(event)?  Should this macro move out of util/event/xcb_event.h to libxcb/xcb.h?  There isn't much left in the util/event library after that, just a bunch of string constants which should be generated from the XML protocol descriptions anyway.

> if(type == XCB_EXPOSE)
>    handle_expose_event();
> else if(type == XCB_KEY_PRESS)
>    ...
> else
>    /* this case would fire harmlessly if any of your asserts would */
>    printf("WARNING: unhandled event of type %d\n", event.response_type);
> 
> To me, this seems clear, handling each type of response with minimal
> boilerplate.
> 
> Jamey


I also prefer a switch statement for event handling.

Ian