[Xcb] Enumerations and typed valueparams (was: XKB once again ...)

[Mark Seaborn]

Peter Harris <peter.harris at hummingbird.com> wrote:

> Mark Seaborn wrote:
> > I actually started playing around
> > with xproto.xml in mid-2005 with some Perl code.  I picked up the idea
> > two years later and started again with Python but kept the modified
> > xproto.xml.  So the definition has diverged a bit with padding fixes,
> > type definitions for <valueparam>s, etc.
> 
> It sounds like you're doing something similar to what I'm doing. I just 
> recently started annotating fields with the enum that they reference, 
> and have plans to make even more changes. I would prefer to avoid 
> duplication of effort, though.
> 
> My changes so far are at ("gitweb" web page): 
> http://repo.or.cz/w/xcbproto/enum.git
> 
> Assuming I've found your xproto.xml correctly[1], it looks like you've 
> branched from an older version of xproto.xml that does not contain as 
> many enums. My current plan involves modifying some of the existing 
> enums to look a lot like your 'bitfield'. I think I prefer <bit 
> type="CARD32" enum="Gravity"> over <bit type="CARD32 : 0=Forget, ...">, 
> but I'm not terribly picky. Perhaps we can collaborate?

Sure.  I agree that it's better to encode the information in XML
structure rather than the mangled strings I added.

Do you have any code that reads your extended definition with "enum"
attributes?

On typing valueparams: I see you have added comments to the
valueparams' enums, e.g.

  <enum name="CW">
    <item name="BackPixmap"><bit>0</bit></item> <!-- enum="BackPixmap" or PIXMAP -->
    ...
  </enum>

I suppose this could become:

  <enum name="CW">
    <item name="BackPixmap" type="PIXMAP" enum="BackPixmap"><bit>0</bit></item>
    ...
  </enum>

The type and enum fields may as well go in XML attributes rather than
elements.  It would be more readable if "bit" could go in an attribute
but I suppose that would not be compatible with other code that reads
the definitions.

I would prefer to avoid having both an enum and an enum item called
"BackPixmap" though.

What I did was add a <bitfield> element alongside to the <enum>:

  <bitfield name="window_attributes">
    <bit number="0" type="PIXMAP / 0=None 1=ParentRelative" name="back_pixmap" />
    ...
  </bitfield>

I prefer having the field called "back_pixmap" rather than
"BackPixmap", because all the existing fields are in the
lowercase-with-underscores style.

On enums in general: Some fields may only contain a value from the
enumeration (closed enum), whereas others may contain an enum value or
another value of the original type, usually a resource ID (open enum).
In my definitions this is expressed with ":" versus "/", e.g.

    <bit number="0" type="PIXMAP / 0=None 1=ParentRelative"
       name="back_pixmap" />
    <bit number="6" type="CARD32 : 0=NotUseful 1=WhenMapped 2=Always"
       name="backing_store" />

Maybe this could be inferred from the type, i.e. if the type is a
resource ID it's an open enum, otherwise it's a closed enum.  But it
wouldn't hurt to make this explicit.  Some types are borderline:
TIMESTAMP, BUTTON and ATOM are all used with enums, and none are
resource ID types.  (ATOM does not follow the same rules as normal
resource IDs.)

Some enums are subsets of others.  There is a good example in
CreateWindow:

    <field type="CARD16 : 0=CopyFromParent 1=InputOutput 2=InputOnly" name="class" />
    <field type="VISUALID / 0=CopyFromParent" name="visual" />

(I think you missed the "visual" field, by the way.)

I think these two enumerations should be defined separately from each
other.

I suppose instead of referencing an enumeration, the <field> elements
could inline the enumeration directly, e.g.

    <field type="CARD16" name="class">
      <enum-item number="0" name="CopyFromParent" />
      <enum-item number="1" name="InputOutput" />
      <enum-item number="2" name="InputOnly" />
    </field>

For a one-off enumeration this could be slightly more readable and
easier to review.

I added the enumeration definitions in my version of xproto.xml by
going through "Appendix B: Protocol Encoding" of the X spec and just
typing up what I saw.  I'm fairly sure that I got them all.

Regards,
Mark