Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-07-29 Thread Nils Chr. Brause 
Hi!

On Fri, Jul 29, 2016 at 10:52 AM, Pekka Paalanen  wrote:
> On Fri, 22 Jul 2016 21:39:20 +0200
> "Nils Chr. Brause"  wrote:
>
>> HI,
>>
>> On Thu, Jun 2, 2016 at 10:30 AM, Pekka Paalanen  wrote:
>> > On Thu, 2 Jun 2016 15:39:47 +0800
>> > Jonas Ådahl  wrote:
>> >
>> >> On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:
>> >> > On Wed, 1 Jun 2016 13:16:31 -0500
>> >> > Yong Bakos  wrote:
>> >> >
>> >> > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  
>> >> > > wrote:
>> >> > > >
>> >> > > > On Sat, 28 May 2016 08:39:59 -0500
>> >> > > > Yong Bakos  wrote:
>> >> > > >
>> >> > > >> Hi Mike,
>> >> > > >> Regarding the combination of type="array" enum="foo"...
>> >> > > >>
>> >> > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
>> >> > > >>>  wrote:
>> >> > > >>>
>> >> > > >>> I've inlined some replies below.
>> >> > > >>>
>> >> > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos 
>> >> > > >>> > wrote:
>> >> > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
>> >> > > >>> > wrote:
>> >> > > 
>> >> > >  this adds a method for compositors to change various draw 
>> >> > >  attributes
>> >> > >  for a surface
>> >> > > 
>> >> > >  Signed-off-by: Mike Blumenkrantz > >> > >  >
>> >> > >  Signed-off-by: Jonas Ådahl > >> > >  >
>> >> > > >>>
>> >> > > >>> 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.
>> >> > >  +  
>> >> > >  +  
>> >> > > >>>
>> >> > > >>> 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.
>>
>> Then that's not an 'array' than but more like a struct.
>> Therefore the structure definition should be in the XML.
>>

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-07-29 Thread Pekka Paalanen 
On Fri, 22 Jul 2016 21:39:20 +0200
"Nils Chr. Brause"  wrote:

> HI,
> 
> On Thu, Jun 2, 2016 at 10:30 AM, Pekka Paalanen  wrote:
> > On Thu, 2 Jun 2016 15:39:47 +0800
> > Jonas Ådahl  wrote:
> >  
> >> On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:  
> >> > On Wed, 1 Jun 2016 13:16:31 -0500
> >> > Yong Bakos  wrote:
> >> >  
> >> > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  
> >> > > wrote:  
> >> > > >
> >> > > > On Sat, 28 May 2016 08:39:59 -0500
> >> > > > Yong Bakos  wrote:
> >> > > >  
> >> > > >> Hi Mike,
> >> > > >> Regarding the combination of type="array" enum="foo"...
> >> > > >>  
> >> > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
> >> > > >>>  wrote:
> >> > > >>>
> >> > > >>> I've inlined some replies below.
> >> > > >>>
> >> > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  >> > > >>> > wrote:
> >> > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
> >> > > >>> > wrote:  
> >> > > 
> >> > >  this adds a method for compositors to change various draw 
> >> > >  attributes
> >> > >  for a surface
> >> > > 
> >> > >  Signed-off-by: Mike Blumenkrantz  >> > >  >
> >> > >  Signed-off-by: Jonas Ådahl  >> > >  >  
> >> > > >>>
> >> > > >>> 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.
> >> > >  +  
> >> > >  +
> >> > > >>>
> >> > > >>> 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.  
> 
> Then that's not an 'array' than but more like a struct.
> Therefore the structure definition should be in the XML.
> Leaving it undocumented is a terrible option.

Hi Nils,

yes, things should definitely be documented. I mean human-readable
documentation, not necessarily machine-parseable description.

> >> > > > The

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-07-22 Thread Nils Chr. Brause 
HI,

On Thu, Jun 2, 2016 at 10:30 AM, Pekka Paalanen  wrote:
> On Thu, 2 Jun 2016 15:39:47 +0800
> Jonas Ådahl  wrote:
>
>> On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:
>> > On Wed, 1 Jun 2016 13:16:31 -0500
>> > Yong Bakos  wrote:
>> >
>> > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
>> > > >
>> > > > On Sat, 28 May 2016 08:39:59 -0500
>> > > > Yong Bakos  wrote:
>> > > >
>> > > >> Hi Mike,
>> > > >> Regarding the combination of type="array" enum="foo"...
>> > > >>
>> > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
>> > > >>>  wrote:
>> > > >>>
>> > > >>> I've inlined some replies below.
>> > > >>>
>> > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos > > > >>> > wrote:
>> > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
>> > > >>> > wrote:
>> > > 
>> > >  this adds a method for compositors to change various draw attributes
>> > >  for a surface
>> > > 
>> > >  Signed-off-by: Mike Blumenkrantz > > >  >
>> > >  Signed-off-by: Jonas Ådahl > > >  >
>> > > >>>
>> > > >>> 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.
>> > >  +  
>> > >  +  
>> > > >>>
>> > > >>> 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.

Then that's not an 'array' than but more like a struct.
Therefore the structure definition should be in the XML.
Leaving it undocumented is a terrible option.

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

Of course it is. If we properly define its contents, language bindings
could put the
proper structure into the argument list of the particular requests/events.

>> > > >
>> > > > If we allow enum annotation with the array type, it will only be usable
>> > > > for doc links, unlike the

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-02 Thread Yong Bakos 
On Jun 2, 2016, at 4:26 AM, Auke Booij  wrote:
> 
> On 1 June 2016 at 20:16, Yong Bakos  wrote:
>> On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
>>> 
>>> On Sat, 28 May 2016 08:39:59 -0500
>>> Yong Bakos  wrote:
>>> 
 Hi Mike,
 Regarding the combination of type="array" enum="foo"...
 
> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
>  wrote:
> 
> I've inlined some replies below.
> 
> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  > wrote:
> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz  > wrote:
>> 
>> this adds a method for compositors to change various draw attributes
>> for a surface
>> 
>> Signed-off-by: Mike Blumenkrantz > >
>> Signed-off-by: Jonas Ådahl >
> 
> 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.
>> +  
>> +  
> 
> 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

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-02 Thread Auke Booij 
On 1 June 2016 at 20:16, Yong Bakos  wrote:
> On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
>>
>> On Sat, 28 May 2016 08:39:59 -0500
>> Yong Bakos  wrote:
>>
>>> Hi Mike,
>>> Regarding the combination of type="array" enum="foo"...
>>>
 On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
  wrote:

 I've inlined some replies below.

 On Fri, May 27, 2016 at 1:13 PM Yong Bakos > wrote:
 On May 27, 2016, at 10:29 AM, Mike Blumenkrantz > wrote:
>
> this adds a method for compositors to change various draw attributes
> for a surface
>
> Signed-off-by: Mike Blumenkrantz  >
> Signed-off-by: Jonas Ådahl >

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

 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

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-02 Thread Jonas Ådahl 
On Thu, Jun 02, 2016 at 11:30:41AM +0300, Pekka Paalanen wrote:
> On Thu, 2 Jun 2016 15:39:47 +0800
> Jonas Ådahl  wrote:
> 
> > On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:
> > > On Wed, 1 Jun 2016 13:16:31 -0500
> > > Yong Bakos  wrote:
> > >   
> > > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  
> > > > wrote:  
> > > > > 
> > > > > On Sat, 28 May 2016 08:39:59 -0500
> > > > > Yong Bakos  wrote:
> > > > > 
> > > > >> Hi Mike,
> > > > >> Regarding the combination of type="array" enum="foo"...
> > > > >> 
> > > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
> > > > >>>  wrote:
> > > > >>> 
> > > > >>> I've inlined some replies below.
> > > > >>> 
> > > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  > > > >>> > wrote:
> > > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
> > > > >>> > wrote:
> > > >  
> > > >  this adds a method for compositors to change various draw 
> > > >  attributes
> > > >  for a surface
> > > >  
> > > >  Signed-off-by: Mike Blumenkrantz  > > >  >
> > > >  Signed-off-by: Jonas Ådahl  > > >  >
> > > > >>> 
> > > > >>> 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.
> > > >  +  
> > > >  +  
> > > > >>> 
> > > > >>> 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
>

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-02 Thread Pekka Paalanen 
On Thu, 2 Jun 2016 15:39:47 +0800
Jonas Ådahl  wrote:

> On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:
> > On Wed, 1 Jun 2016 13:16:31 -0500
> > Yong Bakos  wrote:
> >   
> > > On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:  
> > > > 
> > > > On Sat, 28 May 2016 08:39:59 -0500
> > > > Yong Bakos  wrote:
> > > > 
> > > >> Hi Mike,
> > > >> Regarding the combination of type="array" enum="foo"...
> > > >> 
> > > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
> > > >>>  wrote:
> > > >>> 
> > > >>> I've inlined some replies below.
> > > >>> 
> > > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  > > >>> > wrote:
> > > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz 
> > > >>> > wrote:
> > >  
> > >  this adds a method for compositors to change various draw attributes
> > >  for a surface
> > >  
> > >  Signed-off-by: Mike Blumenkrantz  > >  >
> > >  Signed-off-by: Jonas Ådahl  > >  >
> > > >>> 
> > > >>> 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.
> > >  +  
> > >  +  
> > > >>> 
> > > >>> 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

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-02 Thread Jonas Ådahl 
On Thu, Jun 02, 2016 at 10:24:19AM +0300, Pekka Paalanen wrote:
> On Wed, 1 Jun 2016 13:16:31 -0500
> Yong Bakos  wrote:
> 
> > On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
> > > 
> > > On Sat, 28 May 2016 08:39:59 -0500
> > > Yong Bakos  wrote:
> > >   
> > >> Hi Mike,
> > >> Regarding the combination of type="array" enum="foo"...
> > >>   
> > >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
> > >>>  wrote:
> > >>> 
> > >>> I've inlined some replies below.
> > >>> 
> > >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  > >>> > wrote:
> > >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz  > >>> > wrote:  
> >  
> >  this adds a method for compositors to change various draw attributes
> >  for a surface
> >  
> >  Signed-off-by: Mike Blumenkrantz  >  >
> >  Signed-off-by: Jonas Ådahl  >  >  
> > >>> 
> > >>> 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.
> >  +  
> >  +
> > >>> 
> > >>> 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

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-02 Thread Pekka Paalanen 
On Wed, 1 Jun 2016 13:16:31 -0500
Yong Bakos  wrote:

> On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
> > 
> > On Sat, 28 May 2016 08:39:59 -0500
> > Yong Bakos  wrote:
> >   
> >> Hi Mike,
> >> Regarding the combination of type="array" enum="foo"...
> >>   
> >>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
> >>>  wrote:
> >>> 
> >>> I've inlined some replies below.
> >>> 
> >>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos  >>> > wrote:
> >>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz  >>> > wrote:  
>  
>  this adds a method for compositors to change various draw attributes
>  for a surface
>  
>  Signed-off-by: Mike Blumenkrantz   >
>  Signed-off-by: Jonas Ådahl >  
> >>> 
> >>> 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.
>  +  
>  +
> >>> 
> >>> 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.

None other than it does not make sense now and I cannot see how it
could make sense in the future. I see it similar to allowing
"interface" attribute on  tags whose type is not "object" or
"new_id".

Therefore I am against the idea

Re: array and enum attributes (Was: [PATCH] xdg-shell: add draw states for xdg_surface)

2016-06-01 Thread Yong Bakos 
On May 30, 2016, at 3:54 AM, Pekka Paalanen  wrote:
> 
> On Sat, 28 May 2016 08:39:59 -0500
> Yong Bakos  wrote:
> 
>> Hi Mike,
>> Regarding the combination of type="array" enum="foo"...
>> 
>>> On May 27, 2016, at 12:30 PM, Mike Blumenkrantz 
>>>  wrote:
>>> 
>>> I've inlined some replies below.
>>> 
>>> On Fri, May 27, 2016 at 1:13 PM Yong Bakos >> > wrote:
>>> On May 27, 2016, at 10:29 AM, Mike Blumenkrantz >> > wrote:
 
 this adds a method for compositors to change various draw attributes
 for a surface
 
 Signed-off-by: Mike Blumenkrantz >
 Signed-off-by: Jonas Ådahl >
>>> 
>>> 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.
 +  
 +  
>>> 
>>> 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?

Thank you,
yong



signature.asc
Description: Message signed with OpenPGP using GPGMail
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel