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

Re: Named params for Perl bindings

From: Nik Clayton <nik_at_ngo.org.uk>
Date: 2006-11-30 21:57:51 CET

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.

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 (/mirror) (revision 15845)
+++ subversion/bindings/swig/perl/native/Client.pm (/local) (revision 15845)
@@ -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
@@ -818,12 +820,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 Thu Nov 30 22:56:36 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.