search

joakim-strandberg / xcb_library_thin_ada_binding

7 stars 1 forks source link

Use Prop_Mode_Type in Change_Property #2

Closed adammw closed 8 years ago

adammw commented 8 years ago

Mode can only be a Prop_Mode_Type anyway, may as well set it to be that type so it doesn't need to be casted.

Also changes Atom_Id_Type to Atom_type (which I believe was a typo)

joakim-strandberg commented 8 years ago

Thanks for the suggestion to use the Prop_Mode_Type in the ChangeProperty subprograms. You are right that the Mode parameter should be of the Prop_Mode_Type. There is a snag. The package XCB is auto-generated from xproto.xml: line 2000 something: The PropMode enum is correctly referenced there but the code that uses this information and generates the correct code is not yet implemented. It is on my TODO-list to make this adjustment.

adammw: Your previous commit to the repository needs to be added to proto parser application too (that is quickly done). I will do it as soon I have opportunity tonight unless you are quicker :)

joakim-strandberg commented 8 years ago

The xml that was included in previous comment was removed by github. Probably for security reasons. It was an excerpt from xproto.xml.

joakim-strandberg commented 8 years ago

adammw: Thanks for you contributions to the repository!

adammw commented 8 years ago

Yep, I thought it might be, hadn't looked into how it was auto-generated yet. Also the other commit I made was invalid, but I think some change is still needed. I believe Atom_Type and Atom_Id_Type should be the same thing, locally I've made the constants of Atom_Id_Type and seems to be working/making sense with the "close window" example.

joakim-strandberg commented 8 years ago

adammw: I think you are right that Atom_Type and Atom_Id_Type should be the same thing. It is not at all clear from the xproto.xml file that they should be the same. There is a type called "ATOM" that is a 32-bit X-identifier and Another type called "Atom" which is an enumeration type. In the request ChangeProperty it is the "ATOM" type that is referenced and that is why the auto-generated code chose the 32-bit X-identifier type Atom_Id_Type instead of Atom_Type. I will do some more research and then rewrite the parser to use "Atom_Id_Type" for the enumeration values of "Atom".