Nik Clayton wrote:
> Nik Clayton wrote:
>>>> On 7/20/06, Nik Clayton <nik@ngo.org.uk> wrote:
>>>>> Assuming I get the time to do the work, would anyone be
>>>>> interested in patches to the Perl bindings to support named
>>>>> parameters as well as positional parameters?
> [...]
>>
>> OK. Here's a proof-of-concept implementation that adjusts
>> SVN::Client::ls() so that it can take named params (new style) or
>> positional params (old style).
>
> Here's an updated patch. This one adjusts the data structure that stores
> metadata about the parameters to each SVN::Client function to also
> include information about the method's parameter list, with a
> Params::Validate validation specification.
>
> This is then used to support optional parameters with default values.
And the attached patch starts updating the documentation for ls(), which is
the only function modified so far.
As before, apply the patch, run "make swig-pl".
The following code should then work:
#!/usr/bin/perl
use strict;
use warnings;
use lib '/path/to/subversion/blib/lib'; # XXX adjust this as necessary
use SVN::Client;
use Data::Dumper;
my $c = SVN::Client->new();
my $u = 'file:///home/nik/.svk/svn'; # XXX adjust this as necessary
# Old style
print Dumper $c->ls($u, 'HEAD', 0);
# New style
print Dumper $c->ls({
path_or_url => $u,
revision => 'HEAD',
recurse => 0,
});
# New style, revision and recurse named params both optional
print Dumper $c->ls({ path_or_url => $u });
# Old style, revision and recurse positional params both optional
print Dumper $c->ls($u);
Assuming people think this is a good idea, what's the best way to proceed?
I'm happy to continue to develop this piecemeal in my SVK repo. That'll
result in a very large patch later though.
The work's amenable to being incorporated bit-by-bit in to SVN::Client
-- although that would leave the slightly odd situation where some
functions have this functionality and others don't.
Alternatively, I'm happy to develop this on a branch in the main repo
(depending on your policy on giving commit bits out), so that it's more
visible, and easier for people to try out.
Thoughts?
N
=== subversion/bindings/swig/perl/native/Client.pm
==================================================================
--- subversion/bindings/swig/perl/native/Client.pm (revision 15841)
+++ subversion/bindings/swig/perl/native/Client.pm (revision 15846)
@@ -12,6 +12,8 @@
uuid_from_url uuid_from_path url_from_path revprop_get
revprop_list info);
+use Params::Validate qw(:all);
+
=head1 NAME
SVN::Client - Subversion client functions
@@ -136,6 +138,26 @@
=back
+=head1 NAMED PARAMETERS AND POSITIONAL PARAMETERS
+
+Some methods, such as C<ls()>, can be called with either named parameters
+or positional parameters. Named parameters are passed by specifying
+a hashref as the single parameter, with keys corresponding to method names.
+
+Where a method supports both mechanisms it is shown as follows.
+
+=head2 method
+
+ method({
+ arg1 => 'val1',
+ arg2 => 'val2',
+ });
+
+To call that method with positional parameters just list the arguments
+in the same order.
+
+ method(val1, val2);
+
=head1 METHODS
The following methods are available:
@@ -476,8 +498,15 @@
every path committed in $revision; the values are svn_log_changed_path_t
objects.
-=item $ctx-E<gt>ls($target, $revision, $recursive, $pool);
+=item $ctx-E<gt>ls
+ my %dirents = $ctx->ls({
+ path_or_url => $target,
+ revision => $revision, # optional, default is 'HEAD'
+ recurse => $recursive, # optional, default is 0
+ pool => $pool, # optional
+ });
+
Returns a hash of svn_dirent_t objects for $target at $revision.
If $target is a directory, returns entries for all of the directories'
@@ -818,12 +847,129 @@
=cut
+my %method_defs = (
+ # Keys are method names
+ # Value is a hash ref. Keys in the hash mean:
+ #
+ # type -- method type, 'obj' == object method, takes SVN::Client
+ # as first param. 'class' == class method, does not take
+ # SVN::Client as first param
+ #
+ # args -- array ref of argument specs.
+ #
+ # Each entry is a hash ref (in order of the params appearance
+ # in the positional list). The hash has two keys. 'name' is
+ # params name, 'spec' is a hash ref suitable for feeding to
+ # Params::Validate to validate this parameter.
+ 'ls' => {
+ type => 'obj',
+ args => [
+ { name => 'path_or_url',
+ spec => { type => SCALAR, }, },
+ { name => 'revision',
+ spec => { type => SCALAR,
+ default => 'HEAD', }, },
+ { name => 'recurse',
+ spec => { type => BOOLEAN,
+ default => 0, }, },
+ ],
+ },
+);
+
+# Build object methods
+foreach my $method (keys %method_defs) {
+ no strict 'refs';
+ my $real_func = \&{"SVN::_Client::svn_client_$method"};
+
+ if($method_defs{$method}{type} eq 'obj') {
+ *{"SVN::Client::$method"} = sub {
+ my $self = shift;
+ my @args = @_;
+
+ my $ctx = $self->{ctx};
+
+ my @call_args = (); # Args to call the real function with
+
+ # If $args[0] is a hash ref then we're using named params
+ if(ref $args[0] eq 'HASH') {
+ # The definition in %method_defs isn't a valid
+ # Params::Validate definition. Turn it in to one,
+ # caching the result in $method_defs{$method}{_nam_valid}
+ my %v_spec;
+ if(! exists $method_defs{$method}{_nam_valid} ) {
+ foreach my $arg (@{ $method_defs{$method}{args} }) {
+ $v_spec{$arg->{name}} = $arg->{spec};
+ }
+
+ $method_defs{$method}{_nam_valid} = \%v_spec;
+ } else {
+ %v_spec = %{ $method_defs{$method}{_nam_valid} };
+ }
+
+ my %v_args = validate(
+ @args, { %v_spec,
+ pool => { optional => 1 } }
+ );
+
+ # Populate @call_args with the validated arguments, in order
+ foreach my $arg (@{ $method_defs{$method}{args} }) {
+ push @call_args, $v_args{$arg->{name}};
+ }
+
+ # Add the pool if it was provided
+ push @call_args, $v_args{pool} if exists $v_args{pool};
+ } else { # Using positional params
+ # The definition in %method_defs isn't a valid
+ # Params::Validate definition. Turn it in to one,
+ # caching the result in $method_defs{$method}{_pos_valid}
+ my @spec = ();
+ if(! exists $method_defs{$method}{_pos_valid}) {
+ foreach my $arg (@{ $method_defs{$method}{args} }) {
+ push @spec, $arg->{spec};
+
+ $method_defs{$method}{_pos_valid} = \@spec;
+ }
+ } else {
+ @spec = @{ $method_defs{$method}{_pos_valid} };
+ }
+
+ my @v_args = validate_pos(@args, @spec, { optional => 1 });
+
+ @call_args = @v_args;
+ }
+
+ # If the user supplied a pool then we need to insert $ctx into
+ # the argument list as the last but one arg.
+ if(ref($call_args[-1]) eq '_p_apr_pool_t' or
+ ref($call_args[-1]) eq 'SVN::Pool') {
+ splice(@call_args, $#call_args, 0, $ctx);
+ } else {
+ # No pool supplied. Add the $ctx as the next argument
+ push @call_args, $ctx;
+
+ # If the SVN::Client object has a valid pool then add
+ # that too
+ if(defined $self->{pool} and
+ (ref $self->{pool} eq '_p_apr_pool_t' or
+ ref($self->{pool} eq 'SVN::Pool'))) {
+ push @call_args, $self->{pool};
+ }
+ }
+
+ # Call the real function with the final argument list
+ return $real_func->(@call_args);
+ };
+ } else {
+ # Code to deal with class methods goes here...
+ }
+}
+
# import methods into our name space and wrap them in a closure
# to support method calling style $ctx->log()
foreach my $function (qw(checkout update switch add mkdir delete commit
status log blame diff merge cleanup relocate
revert resolved copy move revprop_set propset
- proplist revvprop_list export ls cat import
+ proplist revvprop_list export cat import
propget uuid_from_url uuid_from_path
url_from_path revprop_get revprop_list
info))
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Dec 6 22:26:39 2006