[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1767197 - in /subversion/trunk/subversion: include/private/svn_log.h include/svn_ra_svn.h libsvn_ra_svn/client.c libsvn_ra_svn/protocol libsvn_subr/log.c svnserve/serve.c

From: Daniel Shahaf <d.s_at_daniel.shahaf.name>
Date: Mon, 31 Oct 2016 00:45:42 +0000

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
> Log:
> Implement svn_ra_list in ra_svn. The wire protocol for this command
> has been insprired by the get-log and get-dir commands alike.
>
> * subversion/libsvn_ra_svn/protocol
> (2.1 Capabilities): Add the 'list' capability.
> (3.1.1. Main Command Set): Add the protocol of the 'list' command.
>
> * subversion/include/svn_ra_svn.h
> (SVN_RA_CAPABILITY_LIST): Declare this new capability in code.
>
> * subversion/svnserve/serve.c
> (list_receiver_baton_t,
> list_receiver,
> list): Implement the new command.
> (main_commands): Register the new command handler.
> (construct_server_baton): Advertise the new capability.
>
> * subversion/libsvn_ra_svn/client.c
> (ra_svn_has_capability): Check for the new capability as well.
> (ra_svn_list): Client-side implementation of the protocol.
> (ra_svn_vtable): Register the new list function.
>
> * subversion/include/private/svn_log.h
> (svn_log__list): Add a new internal API to allow for logging the
> new list command.
>
> * subversion/libsvn_subr/log.c
> (svn_log__list): Implement.
>
> Modified: subversion/trunk/subversion/libsvn_ra_svn/protocol
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_ra_svn/protocol?rev=1767197&r1=1767196&r2=1767197&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_ra_svn/protocol (original)
> +++ subversion/trunk/subversion/libsvn_ra_svn/protocol Sun Oct 30 23:43:06 2016
> @@ -206,6 +206,8 @@ capability and C indicates a client capa
> retrieval of inherited properties via the get-dir and
> get-file commands and also supports the get-iprops
> command (see section 3.1.1).
> +[S] list If the server presents this capability, it supports the
> + list command (see section 3.1.1).
>
> 3. Commands
> -----------
> @@ -487,6 +489,18 @@ second place for auth-request point as n
> response: ( inherited-props:iproplist )
> New in svn 1.8. If rev is not specified, the youngest revision is used.
>
> + 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/

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

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.

> + response: ( )
> + New in svn 1.10. If rev is not specified, the youngest revision is used.
> +
Received on 2016-10-31 01:47:58 CET

This is an archived mail posted to the Subversion Dev mailing list.