Max Bowsher wrote:
> Garrett Rooney 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?
>>>
>>> This would mean that as well as being able to write code like:
>>>
>>> $ra->get_log([$paths], $rev, 1, 20, 0, 0, sub { ... });
>>>
>>> which is a bit obtuse (quick, without access to the docs, which of
>>> those '0'
>>> params is 'discover_changed_paths', and which is 'strict_node_history'?).
>>> The changes I'm thinking of would let you write code like this:
>>>
>>> $ra->get_log({ paths => [ $paths ],
>>> start => $rev,
>>> end => 1,
>>> limit => 20,
>>> discover_changed_paths => 0,
>>> strict_node_history => 0,
>>> receiver => sub { ... } });
>>>
>>> Although it's more verbose, it's much more maintainable.
>>>
>>> Technically, this would involve any function that takes greater than 'n'
>>> params looking to see if the first param is a hash ref, and unpacking the
>>> parameters from the hash ref if so.
>>>
>>> I expect (but have not yet checked) that this could probably be
>>> automated.
>
> I would be delighted to be proved wrong, but I am very pessimistic about
> the above (at least, not without integrating the functionality into the
> core of SWIG itself - which would be a possible avenue, but would mean
> that the enhancement couldn't be made available in Subversion until it
> was released in SWIG and we had increased the minimum SWIG version to
> the new release).
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).
I've done this without disturbing the other code in Client.pm. Broadly, the
approach is to maintain a datastructure that contains metadata about each
function -- it's name, whether it's a class or an object method, what the
named parameters to that method are, and the mapping between a named
parameter and it's position in the argument list that will be passed to the
underlying svn_client_... function.
This datastructure is then used to create the functions named in the data
structure.
Apply the patch, run "make swig-pl", and then try this:
#!/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,
});
It's then relatively easy to (for example), make some of those arguments
optional, so that
...
$c->ls({ path_or_url => '...' });
...
works, with (say) 'revision' defaulting to 'HEAD' and 'recurse' defaulting to 0.
Worthwhile? If so, I'll continue, adjusting documentation and so on as
necessary. Would one big patch that does all of this be preferred, or
multiple patches, one per function?
N
=== subversion/bindings/swig/perl/native/Client.pm
==================================================================
--- subversion/bindings/swig/perl/native/Client.pm (revision 15841)
+++ subversion/bindings/swig/perl/native/Client.pm (local)
@@ -818,12 +818,87 @@
=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 -- list of args passed to the method. Each arg is listed in
+ # the same order it should be passed to the real function. List
+ # entries are (currently) param names, but should really be hash
+ # refs that hold the name of the param, and additional metadata
+ # (is this param required, what's the default value if it's
+ # omitted, and so on).
+ 'ls' => { type => 'obj',
+ args => [ qw(path_or_url revision recurse) ]
+ },
+);
+
+# 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') {
+ # Iterate over the named arguments, putting them in
+ # to @call_args
+ foreach my $arg (@{ $method_defs{$method}{args} }) {
+ # XXX error checking here
+ push @call_args, $args[0]->{$arg}
+ }
+
+ # Include a pool param if it's provided
+ push @call_args, $args[0]->{pool} if exists $args[0]->{pool};
+
+ } else { # Using positional params
+ @call_args = @args; # Straight copy, nothing else to do
+ }
+
+ # If the user supplied a pool then we need to insert $ctx into
+ # the argument list as the last but one arg. If they didn't
+ # supply a pool then just push the context as the last arg, and
+ # use the SVN::Client pool (if available)
+ 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
+ push @call_args, $ctx;
+
+ # If the SVN::Client object has a valid pool then supply that
+ 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 built up 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 Fri Nov 24 15:28:12 2006