[Xcb] Enumerations and typed valueparams

[Peter Harris]

Mark Seaborn wrote:
> Peter Harris <peter.harris at hummingbird.com> wrote:
>> My changes so far are at ("gitweb" web page): 
>> http://repo.or.cz/w/xcbproto/enum.git
> 
> Do you have any code that reads your extended definition with "enum"
> attributes?

Yes, but it's pretty ugly. Perl with XML::Twig generating Java classes 
to be loaded by a legacy pretty-printer. The legacy pretty-printer was 
written by hand. I'm getting tired of writing new extension code all the 
time, which is why I'm excited about XCB.

Note that since I have a stable xproto pretty-printer already, I've only 
tested with xv.xml so far.

Before my enum changes:
106-175: [15081.1] (1) <-- Reply to Request 138.1 QueryAdaptors (XVideo)

sequence #        0x21 (105)
num_adaptors      1
info 0:
  base_id          0xc9
  name_size        0x16
  num_ports        1
  num_formats      0x42
  type             17
... and so on ...

After my enum changes:
106-175: [15081.1] (1) <-- Reply to Request 138.1 QueryAdaptors (XVideo)

sequence #        0x21 (105)
num_adaptors      1
info 0:
  base_id          0xc9
  name_size        0x16
  num_ports        1
  num_formats      0x42
  type             InputMask|ImageMask
... and so on ...

(or, after the "reduced Mask" patches I sent to the list,
  type             Input|Image
)

> 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.

This matches my thoughts exactly. After a quick test, my changes so far 
do not break libxcb. Moving "bit" would break libxcb.

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

libxcb already translates CamelCase into snake_case (although for enums 
it uses SHOUTING_SNAKE_CASE). I don't mind writing a small bit of 
conversion code (or stealing libxcb's, once it has been ported to a sane 
language).

That said, the XCB maintainers have stated a goal of keeping the XML as 
close to the protocol documentation as possible. The protocol 
documentation uses hypen-ated, which we would have to translate into 
snake_case for most programming languages. Not a real problem, but 
someone else would have to fix libxcb's XSLT (or python, if it is ever 
released).

> 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).

See my reply to Barton elsethread.

> 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.)

Yes, I did. I probably missed one or two others along the way.

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

Agreed.

> 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 think that would add needless complexity to code generators. I'd like 
to keep the XSD as simple as possible while still getting the job done.

> 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.

I don't doubt that I've missed a couple of things. I went through 
xproto.xml while referencing Appendix B, but I did it quickly. I have 
only tested xv.xml so far; my changes to xproto.xml were more for 
demonstration and discussion purposes.

Peter Harris
-- 
      Hummingbird Connectivity - A Division of Open Text
Peter Harris                    http://connectivity.hummingbird.com
Research and Development        Phone: +1 905 762 6001
peter.harris at hummingbird.com    Toll Free: 1 877 359 4866