[Telepathy] [PATCH] Introduce tp:type attribute

[Alp Toker]

Simon McVittie wrote:
> On Fri, 11 May 2007 at 18:31:04 +0100, Alp Toker wrote:
>   
>> There are also a few args which still need to be marked up, where I
>> wasn't certain of the arg type required by the spec.
>>     
>
> Do you have a list, or a quick way to find them? Those probably
> represent things which should be clarified in the spec.
>   

I'll come up with a list. It's probably just a matter of reading the 
spec more closely. I was looking to get some feedback before getting too 
involved in this patch.

>   
>> @@ -19,7 +19,7 @@
>>    <interface name="org.freedesktop.Telepathy.Channel.Interface.ChatState">
>>      <tp:requires interface="org.freedesktop.Telepathy.Channel"/>
>>      <method name="SetChatState">
>> -      <arg direction="in" name="state" type="u">
>> +      <arg direction="in" name="state" type="u" tp:type="Channel_Chat_State">
>>          <tp:docstring>
>>            The new state: one of the values of ChannelChatState.
>>          </tp:docstring>
>>     
>
> Looks good.
>
>   
>> @@ -21,7 +21,7 @@
>>    <interface name="org.freedesktop.Telepathy.Channel.Interface.Group">
>>      <tp:requires interface="org.freedesktop.Telepathy.Channel"/>
>>      <method name="AddMembers">
>> -      <arg direction="in" name="contacts" type="au">
>> +      <arg direction="in" name="contacts" type="au" tp:type="Contact_Handle[]">
>>          <tp:docstring>
>>            An array of contact handles to invite to the channel
>>          </tp:docstring>
>>     
>
> I'm less sure about this syntax, but OK... arrays as top-level object
> are pretty common, so I suppose we need an unobtrusive syntax for them.
>   

Yeah, I had the same thought but turns out this is pretty easy to parse 
this in xsl or whatever -- you really don't want to introduce some 
complicated way of specifying type references in XML here. We will have 
to introduce some syntax for specifying dictionaries too. Again, 
"Dictionary<A,B>" is easily parsed and matches the generics syntax of 
many static languages.

> Handles are a tricky one to deal with, too... in telepathy-glib we just
> have TpHandle, which covers all the sorts of handle (and is a typedef
> for guint). I'd actually suggest not marking up any handle arguments specially,
> since whether a handle is valid is context-dependent (it's not an enum
> or anything).
>
> ... or are handles something you need to treat specially in
> telepathy-sharp?
>   

Nope. See below.

>   
>> @@ -169,12 +169,12 @@
>>        </tp:possible-errors>
>>      </method>
>>      <method name="GetHandleOwners">
>> -      <arg direction="in" name="handles" type="au">
>> +      <arg direction="in" name="handles" type="au" tp:type="Handle[]">
>>          <tp:docstring>
>>            A list of integer handles representing members of the channel
>>          </tp:docstring>
>>        </arg>
>> -      <arg direction="out" type="au">
>> +      <arg direction="out" type="au" tp:type="Handle[]">
>>          <tp:docstring>
>>            An array of integer handles representing the owner handles of
>>            the given room members, in the same order, or 0 if the
>>     
>
> FYI, these are handles of type contact - the whole Group interface is in
> terms of contacts. You've been inconsistent about Contact_Handle vs
> Handle.
>   
Well spotted. This was a regex oversight. Will fix this.
> It'd be good if we put a tp:type on the enums and flag-sets too,
> thinking about it - if we use a different D-Bus type somewhere for
> something of the same C/C#/etc. type, I'd consider that to be a bug, and
> we should have some XSLT that goes through the spec making that
> assertion.
>   

I'm not sure what you mean. Enums and flag-sets can only sensibly be 
backed by a primitive (usually integer, string at a push) type, so 
"tp:type" would be useless here. My patch from a few weeks ago added an 
ordinary "type" attribute to enums and flag-sets if that's what you 
mean, and it does the job fine.

>   
>>            One of the values of ChannelTextSendError
>>          </tp:docstring>
>>        </arg>
>> -      <arg name="timestamp" type="u">
>> +      <arg name="timestamp" type="u" tp:type="Timestamp">
>>          <tp:docstring>
>>            The Unix timestamp indicating when the message was sent
>>     
>
> I'm sceptical about this one - we never define Timestamp anywhere. Or
> are you thinking of special-casing this in each set of code-generation
> tools so it comes out as time_t in C, some sort of datetime or timestamp
> object in C#, etc.?
>   

Think of tp:type as a kind of semantic mark-up or tag/label (well, not 
really, I know). Many binding generators won't do anything with 
Timestamp typed args, but it will still let us document the concept of a 
Unix timestamp in one place with a nice hyperlink from the interface doc 
page, allow tools like telepathy-inspector to print the date as a string 
etc. Ditto for Contact_Handle and the others... while they are 
conceptually similar to a C typedef, I wouldn't be surprised if 
generators ignore the tp:type and expose uints in the API -- the 
documentation and protocol validation benefits still exist.

>   
>>      <method name="GetChannelType">
>> -      <arg direction="out" type="s">
>> +      <arg direction="out" type="s" tp:type="Interface">
>>          <tp:docstring>The interface name</tp:docstring>
>>     
>
> Hmm. I can see your reasoning here, but... same sort of issues as
> Timestamp. Although, this one is a better candidate for having some sort
> of special-casing in the generation tools, since 's' is the obvious
> correct way to represent an interface name on D-Bus.
>   

Again, the main benefit is documentation and automated validation here. 
Generators should just ignore/report tp:types they don't understand.

>   
>> -      <arg name="handle_type" type="u">
>> +      <arg name="handle_type" type="u" tp:type="Handle_Type">
>>          <tp:docstring>
>>            An integer representing the type of handle this channel communicates with, which is zero if no handle is specified
>>          </tp:docstring>
>>        </arg>
>> -      <arg name="handle" type="u">
>> +      <arg name="handle" type="u" tp:type="Handle">
>>          <tp:docstring>
>>            A handle indicating the specific contact, room or list this channel communicates with, or zero if it is an anonymous channel
>>     
>
> In the case of handles whose type is given by another parameter,
> providing type-safety between handle types seems unlikely.
>   

Sure, but it is still an arg holding a "Handle". Automated validation 
and documentation to be gained even in this case, and there is nothing 
to lose by accurately labelling the content in every possible case. A 
well specified protocol is a happy protocol.