psychon / x11rb

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

Add "this creates/frees an XID" to the XML and use that for better generated API #589

Open psychon opened 3 years ago

psychon commented 3 years ago

Related: #369 and #516

We could do something like the following:

diff --git a/xcb-proto-1.14-1-g2b3559c/src/xproto.xml b/xcb-proto-1.14-1-g2b3559c/src/xproto.xml
index dea48df..7281c5a 100644
--- a/xcb-proto-1.14-1-g2b3559c/src/xproto.xml
+++ b/xcb-proto-1.14-1-g2b3559c/src/xproto.xml
@@ -3748,7 +3748,7 @@ The maximum number of fonts to be returned.

   <request name="CreatePixmap" opcode="53">
     <field type="CARD8" name="depth" />
-    <field type="PIXMAP" name="pid" />
+    <field type="PIXMAP" name="pid" xid="create" />
     <field type="DRAWABLE" name="drawable" />
     <field type="CARD16" name="width" />
     <field type="CARD16" name="height" />
@@ -3789,7 +3789,7 @@ The X server could not allocate the requested resources (no memory?).

   <request name="FreePixmap" opcode="54">
     <pad bytes="1" />
-    <field type="PIXMAP" name="pixmap" />
+    <field type="PIXMAP" name="pixmap" xid="free" />
     <doc>
       <brief>Destroys a pixmap</brief>
       <description><![CDATA[

No idea what to do with the free annotation, but the create would allow to provide a more Xlib-y API where the new XID is returned instead of provided by the caller. That would then remove the need for generate_id() and thus fix #516. (Another option would be to add new XML <free name="pid" /> and <create name="pid" /> that is only allowed inside of <request>. More bikeshedding options...)

My preferred approach for how to tackle this would be to first propose a WIP patch upstream to xcb-proto and get through the bikeshedding on how to call this and how to integrate that with XSD rules. After the "how" is done, one could go through all the extension and figure out where to add the annotations. Hopefully, the result would then be mergable upstream.

Open questions include:

CC @not-a-seagull just FYI. Feel free to just ignore this, but I thought you might be interested in this. Right now, it looks like you have e.g. a hand-written wrapper for create_pixmap that already returns the XID.

notgull commented 3 years ago

I probably wouldn’t use it (the manual design of breadx is an intentional decision), but I would support it. Scratch that I would 100% use it