subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Daniel Shahaf <...@daniel.shahaf.name>
Subject 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
Date Mon, 31 Oct 2016 00:45:42 GMT
stefan2@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.
> +

Mime
View raw message