You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Nik Clayton <ni...@ngo.org.uk> on 2006/07/20 09:53:51 UTC
Named params for Perl bindings
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.
Any suggestions as to what an appropriate value for 'n' might be? I suspect
that '3' is a sensible minimum (i.e., any function that takes 3 or more
params should also support named parameters), but I also think that if the
work's going to happen then, for consistency, all functions should be converted.
Just to stress -- this wouldn't remove the existing calling conventions.
Just add another option.
Thoughts?
Also, having just spent 20 minutes chasing around the maze of files in
subversion/bindings, if anyone's got a "This is how SWIG is used with
Subversion" document, I'd appreciate a peek :-)
N
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Named params for Perl bindings
Posted by John Peacock <jp...@rowman.com>.
Nik Clayton wrote:
> How do people feel about making the Perl Subversion bindings dependent
> on non-core (i.e., they don't ship with Perl) modules from CPAN? I'm
> specifically thinking of Params::Validate, which would simplify the
> above somewhat.
I would be in favor of this, but I think that you would have to make sure that
dependency was added to the Makefile.PL that Subversion uses to build the Perl
bindings (i.e. that it would force P::V to be installed before the bindings
could be built/tested).
John
--
John Peacock
Director of Information Research and Technology
Rowman & Littlefield Publishing Group
4501 Forbes Blvd
Suite H
Lanham, MD 20706
301-459-3366 x.5010
fax 301-429-5747
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Named params for Perl bindings
Posted by Nik Clayton <ni...@ngo.org.uk>.
Max Bowsher wrote:
> Garrett Rooney wrote:
>> On 7/20/06, Nik Clayton <ni...@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.
>
> 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).
I've got a reasonable idea how to do this now, I might have a proof of
concept patch for one function by the end of this train journey.
How do people feel about making the Perl Subversion bindings dependent on
non-core (i.e., they don't ship with Perl) modules from CPAN? I'm
specifically thinking of Params::Validate, which would simplify the above
somewhat.
N
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: Named params for Perl bindings
Posted by Nik Clayton <ni...@ngo.org.uk>.
Nik Clayton wrote:
> Nik Clayton wrote:
>>>> On 7/20/06, Nik Clayton <ni...@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
Re: Named params for Perl bindings
Posted by Nik Clayton <ni...@ngo.org.uk>.
Nik Clayton wrote:
>>> On 7/20/06, Nik Clayton <ni...@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
Re: Named params for Perl bindings
Posted by Nik Clayton <ni...@ngo.org.uk>.
Max Bowsher wrote:
> Garrett Rooney wrote:
>> On 7/20/06, Nik Clayton <ni...@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
Re: Named params for Perl bindings
Posted by Max Bowsher <ma...@ukf.net>.
Garrett Rooney wrote:
> On 7/20/06, Nik Clayton <ni...@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).
>> Any suggestions as to what an appropriate value for 'n' might be?
>> I suspect that '3' is a sensible minimum (i.e., any function that
>> takes 3 or more params should also support named parameters), but I
>> also think that if the work's going to happen then, for
>> consistency, all functions should be converted.
Personally, I'd set 'n' at zero, just for consistency's sake.
>> Just to stress -- this wouldn't remove the existing calling conventions.
>> Just add another option.
>>
>> Thoughts?
If it can be done in an automated way: sounds good!
If it cannot, as I fear: would introduce too much laborious-to-maintain
code to be worthwhile.
>> Also, having just spent 20 minutes chasing around the maze of files in
>> subversion/bindings, if anyone's got a "This is how SWIG is used with
>> Subversion" document, I'd appreciate a peek :-)
>
> I don't know if there's a simple document, but Max Bowsher has been
> looking at the swig stuff lately, so if you ask him nicely perhaps he
> could condense what he learned into something easily digestible...
A lot of it is experience rather than knowledget, but I guess as a
starting point:
*.i files are actual modules.
*.swg files are %included by *.i files and other *.swg files.
proxy/*.swg files are autogenerated.
include/*.swg files are manually written.
Support routines live in language specific libraries:
python/libsvn_swig_py
perl/libsvn_swig_perl
ruby/libsvn_swig_rb
There is a pure-Python/Perl/Ruby set of modules that wrap the ones
generated with SWIG, to provide a more natural interface and/or extra
helpers:
python/svn/
perl/native/*.pm
ruby/svn/
Max.
Re: Named params for Perl bindings
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 7/20/06, Nik Clayton <ni...@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.
I'd love to see that available in the perl bindings, it would make
things FAR more readable.
> 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.
>
> Any suggestions as to what an appropriate value for 'n' might be? I suspect
> that '3' is a sensible minimum (i.e., any function that takes 3 or more
> params should also support named parameters), but I also think that if the
> work's going to happen then, for consistency, all functions should be converted.
>
> Just to stress -- this wouldn't remove the existing calling conventions.
> Just add another option.
>
> Thoughts?
>
> Also, having just spent 20 minutes chasing around the maze of files in
> subversion/bindings, if anyone's got a "This is how SWIG is used with
> Subversion" document, I'd appreciate a peek :-)
I don't know if there's a simple document, but Max Bowsher has been
looking at the swig stuff lately, so if you ask him nicely perhaps he
could condense what he learned into something easily digestible...
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org