On 31.10.2016 01:45, Daniel Shahaf wrote:
> stefan2_at_apache.org wrote on Sun, Oct 30, 2016 at 23:43:07 -0000:
>> Author: stefan2
>> Date: Sun Oct 30 23:43:06 2016
>> New Revision: 1767197
>>
>> URL: http://svn.apache.org/viewvc?rev=1767197&view=rev
[snip]
>> + list
>> + params: ( path:string [ rev:number ] depth:word
>> + ( field:dirent-field ... ) ( pattern:string ... ) )
>> + Before sending response, server sends dirent, ending with "done".
>
> Typo: s/dirent/dirents/
Fixed in r1768185.
>> + dirent: ( rel-path:string kind:node-kind ? ( size:number
>> + has-props:bool created-rev:number
>> + [ created-date:string ] [ last-author:string ] ) )
>> + | done
>
> Why should size,has-props,created-rev be all present or all absent?
> Wouldn't it be better to let each element (other than the first two) be
> optional, i.e.,
>
> dirent: ( rel-path:string kind:node-kind
> ? [ size:number ]
> [ has-props:bool ]
> [ created-rev:number ]
> [ created-date:string ]
> [ last-author:string ] )
>
> ? This decouples the protocol from the backend (specifically, from what
> bits the backend has cheaply available).
Yes, that would be better.
It would require to either make has_props a tristate
or extending the protocol to allow for optional bools.
I think we should do the latter because regardless of
whether we have them at the protocol level or not, we
must specify a value in the output data struct at the
receiver side. So, we might as well define it to FALSE
for n/a data.
This does not effect any existing protocol usage or
code as the format patterns of the existing commands
do not change. They are represented as "words" in
the pre-parsed "item" structure, so even old code
receiving an optional boolean would parse them fine -
in case we were to add optional booleans to existing
command responses in the future.
And while we are at it, we should make vwrite_tuple()
actually support optional tristates and numbers.
>
> Typo: there is one ")" too many.
>
>> + dirent-field: kind | size | has-props | created-rev | time | last-author
>
> Don't you need here a "| word" alternative, like get-dir has? I think
> that's not a type documentation, but a forward compatibility indicator,
> i.e., indicating that more words may be added in the future.
Hm, makes sense. I guess that usage of "| word" should be
commented on in the file. I for one, mis-interpreted it.
I'll make all these changes later today.
-- Stefan^2.
Received on 2016-11-05 13:52:58 CET