[Xcb] XKEYBOARD protocol definition

[Eamon Walsh]

Peter,

Some review below.  I haven't seen the earlier work so this will be
zero-based.

There are some changes upstream that aren't in your repo.  I needed to
pull from xcb/proto to get recent changes.

Obligatory whitespace comments: the indentation of the XML files is a
little erratic.  There is some flagged whitespace I can see with git
diff --color, some of which I mention below but not all.

--Eamon W.


diff --git a/doc/xml-xcb.txt b/doc/xml-xcb.txt
index feb9984..db86159 100644
--- a/doc/xml-xcb.txt
+++ b/doc/xml-xcb.txt
@@ -227,6 +227,17 @@ enum; the value is restricted to one of the constants named in the enum.
   CARD32.  value-mask-name gives the field name of the bitmask, and
   value-list-name gives the field name of the list of values.
 
+<switch name="identifier"> switch expression <bitcase> bitcase expression, fields </bitcase>
+...</switch>
+
+  This element represents conditional inclusion of fields. It can be viewed as
+  sequence of multiple if's: if ( switch expression & bitcase
+  expression ) is equal to bitcase expression, bitcase fields are included in structure.
+  It can be used only as the last field of structure.
+


I don't see any new xcbgen Python code to handle this new element.  The
only Python changes I see are to expr.py, adding new list length
calculators.  Are there additional Python changes forthcoming, perhaps
in the libxcb changes?


+<replyof request="identifier" name="identifier" />
+
+  This element makes reply of request a field of structure.


This does not appear anywhere else and should be removed.


 
 Expressions
 -----------
@@ -256,3 +267,21 @@ Expressions
 
   The bit element represents a literal bitmask value in an expression.
   The integer must be in the range 0..31, expanding to (1<<n) in C.
+
+<enumref ref="identifier">enum item identifier</enumref>
+
+  This elements represents reference to item of enum.

s/elements/element/


+
+<unop op="operator">expression</unop>
+
+   This element represents unary operator, with the op attribute specifying
+   which operator. 

Whitespace at the end of this line.


+
+<sumof ref="identifier" />
+
+   This element represents sumation of elements of referenced list.
+
+<popcount>expression</popcount>
+
+   This element represents number of bits set.
+


Going off on a tangent here: if popcount goes in, then we don't need the
<valueparam> element in the schema anymore.  The reason is that the
following two snippets are identical:

<valueparam value-mask-type="CARD32"
            value-mask-name="value-mask"
            value-list-name="value-list" />

<field type="CARD32" name="value-mask" />
<list name="value-list" type="CARD32">
  <popcount><fieldref>value-mask</fieldref></popcount>
</list>


diff --git a/src/xcb.xsd b/src/xcb.xsd
index f3fcb6f..d94ba6e 100644
--- a/src/xcb.xsd
+++ b/src/xcb.xsd
@@ -46,6 +46,9 @@ authorization from the authors.
     </xsd:complexType>
   </xsd:element>
 
+  <!-- Alignment = pad(expr) -->
+  <xsd:element name="align" />
+


This appears to be unused in the XML and should be removed.


diff --git a/src/xinput2.xml b/src/xinput2.xml
new file mode 100644
index 0000000..b962620
--- /dev/null
+++ b/src/xinput2.xml
@@ -0,0 +1,700 @@

[snip]

+    <!-- Types -->
+
+    <!-- BUTTONMASK -->
+    <typedef oldname="CARD32" newname="BUTTONMASK" />
+
+    <!-- DEVICEID -->
+    <typedef oldname="CARD16" newname="DEVICEID" />
+
+    <enum name="Devices">
+    	<item name="All">       <value>0</value> </item>

Spaces before tabs here.


+	<item name="AllMaster"> <value>1</value> </item>
+    </enum>
+
+    <!-- DEVICE -->
+    <typedef oldname="DEVICEID" newname="DEVICE" />

Do we really need both DEVICE and DEVICEID aliases for CARD16?  Why
can't we just use one or the other?


+
+    <!-- DEVICEUSE -->
+    <enum name="DeviceUse">
+        <item name="MasterPointer">  <value>1</value> </item>
+	<item name="MasterKeyboard"> <value>2</value> </item>
+	<item name="SlavePointer">   <value>3</value> </item>
+	<item name="SlaveKeyboard">  <value>4</value> </item>
+	<item name="FloatingSlave">  <value>5</value> </item>
+    </enum>
+
+    <!-- FP1616 -->
+    <typedef oldname="INT32" newname="FP1616" />
+
+    <!-- FP3232 -->
+    <struct name="FP3232">
+    	<field name="integral" type="INT32" />
+	<field name="frac" type="CARD32" />
+    </struct>

Spaces before tab here.

Tangent: we will probably need some utility functions to convert these
to floats and vice versa.

+
+    <!-- CHAR8 -->
+    <typedef oldname="CARD8" newname="CHAR8" />

This is only used in two places, both of which are string names.  The
convention in the other files is to use the predefined type "char" for this.

[snip]


+
+    <!-- WhatProperty -->
+    <enum name="WhatProperty">
+        <item name="Created">  <value>1</value> </item>
+	<item name="Deleted">  <value>0</value> </item>
+	<item name="Modified"> <value>2</value> </item>
+    </enum>

This is kind of an odd name.  Maybe  PropertyWhat, PropertyChange, or
PropertyEventType?

/me looks at the size of the xkb.xml file and decides to skip the rest
of the XML for now.  Will do some more review when the libxcb stuff
comes out and I can compile it.

[snip]

diff --git a/xcbgen/expr.py b/xcbgen/expr.py
index 522e17d..f479041 100644
--- a/xcbgen/expr.py
+++ b/xcbgen/expr.py
@@ -57,6 +57,22 @@ class Expression(object):
             # Standard list with a fieldref
             self.lenfield_name = elt.text
 
+	elif elt.tag == 'enumref':
+	    self.lenfield_name = elt.text
+	    self.lenfield_type = elt.get('ref') 
+	
+	elif elt.tag == 'bitcount':
+	    self.op = 'bitcount'
+	    self.lhs = Expression(list(elt)[0], parent)
+	    self.rhs = int(elt.get('value'))
+	    self.lenfield_name = self.lhs.lenfield_name
+	    self.lenfield_type = 'CARD32'
+	    
+	elif elt.tag == 'sumof':
+	    self.op = 'sumof'
+	    self.lenfield_name = elt.get('ref')
+	    self.lenfield_type = elt.get('type')
+


There is extra whitespace on the two blank lines here, and some
whitespace at the end of the third added line.

Also I mentioned this above but there are a bunch of new schema elements
that aren't added to the Python parser in this patchset.


         elif elt.tag == 'valueparam':
             # Value-mask.  The length bitmask is described by attributes.
             self.lenfield_name = elt.get('value-mask-name')
@@ -81,7 +97,7 @@ class Expression(object):
 
         else:
             # Notreached
-            raise Exception('XXX')
+            raise Exception('Unknown element: %s' % elt.tag)


What, you don't like getting Python tracebacks ending with
"Exception:XXX"?  :-)