On 31.10.2016 01:45, Daniel Shahaf wrote:
stef...@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.