On Jun 2, 2016, at 4:26 AM, Auke Booij <a...@tulcod.com> wrote: > > On 1 June 2016 at 20:16, Yong Bakos <j...@humanoriented.com> wrote: >> On May 30, 2016, at 3:54 AM, Pekka Paalanen <ppaala...@gmail.com> wrote: >>> >>> On Sat, 28 May 2016 08:39:59 -0500 >>> Yong Bakos <j...@humanoriented.com> wrote: >>> >>>> Hi Mike, >>>> Regarding the combination of type="array" enum="foo"... >>>> >>>>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz >>>>> <michael.blumenkra...@gmail.com> wrote: >>>>> >>>>> I've inlined some replies below. >>>>> >>>>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos <j...@humanoriented.com >>>>> <mailto:j...@humanoriented.com>> wrote: >>>>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz <zm...@osg.samsung.com >>>>> <mailto:zm...@osg.samsung.com>> wrote: >>>>>> >>>>>> this adds a method for compositors to change various draw attributes >>>>>> for a surface >>>>>> >>>>>> Signed-off-by: Mike Blumenkrantz <zm...@osg.samsung.com >>>>>> <mailto:zm...@osg.samsung.com>> >>>>>> Signed-off-by: Jonas Ådahl <jad...@gmail.com <mailto:jad...@gmail.com>> >>>>> >>>>> Hi Mike & Jonas, >>>>> A question about communicating default state, and some >>>>> minor nits you can certainly ignore, inline below. >>>>> >>>>> >>>>>> --- >>>>>> unstable/xdg-shell/xdg-shell-unstable-v6.xml | 69 >>>>>> ++++++++++++++++++++++++++++ >>>>>> 1 file changed, 69 insertions(+) >>>>>> >>>>>> diff --git a/unstable/xdg-shell/xdg-shell-unstable-v6.xml >>>>>> b/unstable/xdg-shell/xdg-shell-unstable-v6.xml >>>>>> index dfd7e84..0fa76d4 100644 >>>>>> --- a/unstable/xdg-shell/xdg-shell-unstable-v6.xml >>>>>> +++ b/unstable/xdg-shell/xdg-shell-unstable-v6.xml >>> >>>>>> + >>>>>> + Calling this after an xdg_toplevel's first commit will raise a >>>>>> client error. >>>>>> + </description> >>>>>> + <arg name="states" type="array" enum="draw_state"/> >>>>> >>>>> Just a sanity check, since I haven't seen it in a protocol spec yet. Does >>>>> scanner handle >>>>> this combination of array and enum correctly? >>>>> >>>>> Good catch. This also affects the event above it. >>>> >>>> As we discussed via IRC (27 May), the scanner will choke on this. While we >>>> talked about >>>> making a change to the scanner to allow this, perhaps such a change >>>> doesn't make sense. >>>> >>>> Given a type="array", scanner will generate a parameter of type wl_array. >>>> >>>> Perhaps the short story here is to just remove the enum from this arg, and >>>> the similar >>>> arg in the configure_draw_states event above. What do you think? >>>> >>>> (I wonder if it's the DTD that can change, so the scanner's validation step >>>> will catch the unsupported combination of type="array" enum="foo". My gut >>>> tells me that >>>> DTDs don't support this logic, but I'll dig into this.) >>> >>> Hi, >>> >>> here is some background. >>> >>> A type="array" argument is really just a binary blob of data. The XML >>> description, human documentation aside, does not specify anything about >>> the blob contents. Therefore adding an XML attribute pointing to an >>> enum definition is half-useless. Generators could use it for creating >>> automatic links in documentation, but it cannot be used for code >>> generation, because you don't know the types contained in the blob. >>> >>> We also do not want to add blob content type definitions to the XML >>> language, because you might want to have everything C is able to >>> express, including nested structs. There is also no requirement that >>> the "array" is really an array - every "element" could be a different >>> thing. It could be bitstream and whatnot. Only the use of >>> wl_array_for_each() implies it is an array of similar elements, >>> wl_array_add() does not. >>> >>> The big point in adding enum annotations was that language bindings >>> generators (other than wayland-scanner) could use the annotation for >>> code generation. I don't think it is possible with the array type. >>> >>> If we allow enum annotation with the array type, it will only be usable >>> for doc links, unlike the other enum annotations. >>> >>> OTOH, we have lots and lots of places in the documentation texts that >>> refer to some request, event, interface, etc. that would be useful as a >>> hyperlink in the generated docs. Enums could fall into that as well, so >>> we would not need the attribute for only documentation. >>> >>> Auke, Nils, what's your take on this matter? >>> >>> We do have some documentation about enums in >>> https://wayland.freedesktop.org/docs/html/ch04.html#sect-Protocol-Basic-Principles >>> >>> Thanks, >>> pq >> >> Pekka, >> Thank you for the info. Just so I understand your points correctly, let >> me assert that /just/ making a minor change to scanner to not error on >> the presence of both array and enum together does not have any major >> drawbacks. >> >> Correct? > > Technically, it might be the case that nothing will necessarily break > *now*. But such a change would move us from very the currently > well-defined semantics of the enum attribute, into weird > documentation-only or half-defined specifications. This will be > confusing more often than it will be helpful. > > If you want to refer to a specific enum, because your binary data > *could* be interpreted as doing so, in my opinion this should be done > in the textual documentation. Any formal reference should have a > formal technical specification, which we don't have (and don't want).
Thanks Auke and Pekka, I definitely see the point in needing to be conservative here, despite the innocuous intent. Scanner should not accommodate the presence of array with enum right now. yong _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
