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.

Reply via email to