You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by Daniel Shahaf <da...@apache.org> on 2013/07/07 15:14:09 UTC
Re: svn commit: r1488693 -
/subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
On Sun, Jun 02, 2013 at 12:17:58PM -0000, rschupp@apache.org wrote:
> Author: rschupp
> Date: Sun Jun 2 12:17:58 2013
> New Revision: 1488693
>
> URL: http://svn.apache.org/r1488693
> Log:
> Documentation and code cleanup changes only.
>
> * subversion/bindings/swig/perl/native/Client.pm
> - POD: Add section "ADDITIONAL METHODS" listing all methods wrapped,
> but not (yet) documented.
+0 to backport this part of the change.
> - Code: Sort the list @_all_fns.
> - POD: Use modern sub parameter conventions in POD (no more "shift"ing).
These all non-functional changes. We don't normally backport such changes
to stable branches, unless needed to prevent text conflicts down the road.
> - POD: s/$ctx/$client/ which is a more appropriate name for a SVN::Client
> object.
> - POD: s/$cinfo/$commit_info/, s/$citem/$commit_item/
These mechanical changes make the diff a PITA to review, because (a) it's
now 1000 lines long, (b) it contains all sorts of mechanical changes mixed
in with each other and with functional changes (and with a few
whitespace-only changes that the log message doesn't mention).
I've gone ahead and reviewed it (and I'll cast a +1), but next time please
try and make your commits easier to review.
Cheers,
Daniel
> - POD: Move =item for "info" into correct spot wrt sorting.
Re: svn commit: r1488693 -
/subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Daniel Shahaf <da...@apache.org>.
On Sun, Jul 07, 2013 at 06:11:32PM +0300, Daniel Shahaf wrote:
> Roderich Schupp wrote on Sun, Jul 07, 2013 at 17:04:07 +0200:
> > On Sun, Jul 7, 2013 at 3:14 PM, Daniel Shahaf <da...@apache.org> wrote:
> >
> > > > - POD: Use modern sub parameter conventions in POD (no more
> > > "shift"ing).
> > >
> > > These all non-functional changes. We don't normally backport such changes
> > >
> >
> > There's a difference between Subversion proper and the (Perl, Python, Ruby)
> > bindings. The latter are useless without proper documentation,
>
> Nothing is useful without proper documentation, but devil's advocate
> says the bindings can be quite usable if you document only the delta
> between SVN::Client->info42 and svn_client_info42().
>
> > so IMHO the POD is "functional".
Never mind my previous response, I missed that the simple_prompt() sub was
part of POD, not part of code. So, yes, that one was a functional change.
Re: subversion.apache.org/docs/swig-py ? Re: Perl bindings docstrings
Re: svn commit: r1488693 - /subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Roderich Schupp <ro...@gmail.com>.
On Tue, Jul 9, 2013 at 4:37 PM, Daniel Shahaf <d....@daniel.shahaf.name>wrote:
> Separately, is there a reason the compiled swig-* docs aren't online on
> http://subversion.apache.org/docs/, alongside the C and javahl ones? At
> least the Perl ones are on CPAN[1],
>
[1]
> http://search.cpan.org/~mschwern/Alien-SVN-v1.7.3.1/src/subversion/subversion/bindings/swig/perl/native/Client.pm
>
I'm very grateful to Michael Schwern for providing these, but there's no
guarantee he will do that forever.
(I don't think anyone ever asked that question on users@, though.)
>
Maybe this an indication that nobody actually uses the Perl bindings?
I've seen lotsa Perl scripts doing Subversion stuff, but almost all are
simply using the CLI to access Subversion. Even scripts in the contrib
section of our repository (e.g. svn_load_dirs.pl) do it that way.
Looking into Debian I only found two actual users of the Perl bindings
(i.e. "reverse dependants" of Debian package libsvn-perl).
One is SVN::Web, an almost obsolete Web UI. The other is svn-git :)
Cheers, Roderich
subversion.apache.org/docs/swig-py ? Re: Perl bindings docstrings
Re: svn commit: r1488693 -
/subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Roderich Schupp wrote on Tue, Jul 09, 2013 at 09:18:56 +0200:
> On Mon, Jul 8, 2013 at 5:32 PM, Ben Reser <be...@reser.org> wrote:
> > Unless someone has invented real AI I just don't see how you can automate
> > this.
>
> But certainly one could whip up a simple Perl script (oder editor macros) to
> spare us from the tedium of manually copying and pasting the C doc comments,
> remove the leading * on each line, turn "@a foo" into "C<$foo>" etc.
While at it, you could also change #apr_pool_t to C<SVN::Pool>,
#svn_opt_revision_t to (the Perl equivalent you described), etc.
Separately, is there a reason the compiled swig-* docs aren't online on
http://subversion.apache.org/docs/, alongside the C and javahl ones? At
least the Perl ones are on CPAN[1], but searching for the Python ones
yields mostly stackoverflow threads asking where they are.
(I don't think anyone ever asked that question on users@, though.)
Daniel
[1]
http://search.cpan.org/~mschwern/Alien-SVN-v1.7.3.1/src/subversion/subversion/bindings/swig/perl/native/Client.pm
Re: Perl bindings docstrings Re: svn commit: r1488693 - /subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Roderich Schupp <ro...@gmail.com>.
On Mon, Jul 8, 2013 at 5:32 PM, Ben Reser <be...@reser.org> wrote:
> > So it's as simple as "extract docstring from C header and reformat as
> POD".
>
Actually I meant "So it's NOT as simple...", at least if you want to fully
automate this.
> Exactly this. We need this documentation. Sometimes we're behind on
> producing it, but once produced it's rarely ever wrong. There is no
> way for a Perl user to know how we've wrapped things from the C API.
>
+1
> So in short, suggesting that we remove this documentation or produce
> it in some automated way is probably going to produce far worse
> results.
>
+1
I looked at the Perl bindings for Gtk+ and it seems that they generate only
the simple doc stuff (i.e. method signature and list of parameters and
their types),
while any textual description is by hand (though may have been
culled intially from the C level docs). Anyway, they're in better position
to do this,
since they don't use Swig, but generate XS themselves. Hence they already
"know" how types and signatures are mapped, while for Subversion this
is only known to Swig.
> Unless someone has invented real AI I just don't see how you can automate
> this.
But certainly one could whip up a simple Perl script (oder editor macros) to
spare us from the tedium of manually copying and pasting the C doc comments,
remove the leading * on each line, turn "@a foo" into "C<$foo>" etc.
Cheers, Roderich
Re: Perl bindings docstrings Re: svn commit: r1488693 - /subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Ben Reser <be...@reser.org>.
On Mon, Jul 8, 2013 at 7:27 AM, Roderich Schupp
<ro...@gmail.com> wrote:
> I'll have a look at how the Perl bindings for Gtk+ do this
> (http://gtk2-perl.sourceforge.net/) Does anybody know of other examples?
>
> On the other hand, there's not a one-to-one correspondence of
> Perl vs C level parameters, e.g.:
> - editor (or callback) + baton pairs in C are replaced by
> a single Perl callback (typically a closure) in Perl
> - svn_opt_revision_t arguments in C may be actually
> specified as numbers, strings ("HEAD"), date strings ("{12-04-2013}")
> or (unlikely) as _p_svn_opt_revision_t objects in Perl
> So it's as simple as "extract docstring from C header and reformat as POD".
Exactly this. We need this documentation. Sometimes we're behind on
producing it, but once produced it's rarely ever wrong. There is no
way for a Perl user to know how we've wrapped things from the C API.
So in short, suggesting that we remove this documentation or produce
it in some automated way is probably going to produce far worse
results.
Unless someone has invented real AI I just don't see how you can automate this.
Re: Perl bindings docstrings Re: svn commit: r1488693 - /subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Roderich Schupp <ro...@gmail.com>.
On Sun, Jul 7, 2013 at 7:33 PM, Daniel Shahaf <da...@apache.org> wrote:
> Or perhaps we should look into a way to generate
> the Perl bindings' docstrings from the C API docstrings?
>
I'll have a look at how the Perl bindings for Gtk+ do this
(http://gtk2-perl.sourceforge.net/) Does anybody know of other examples?
On the other hand, there's not a one-to-one correspondence of
Perl vs C level parameters, e.g.:
- editor (or callback) + baton pairs in C are replaced by
a single Perl callback (typically a closure) in Perl
- svn_opt_revision_t arguments in C may be actually
specified as numbers, strings ("HEAD"), date strings ("{12-04-2013}")
or (unlikely) as _p_svn_opt_revision_t objects in Perl
So it's as simple as "extract docstring from C header and reformat as POD".
Cheers, Roderich
Perl bindings docstrings Re: svn commit: r1488693 -
/subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Daniel Shahaf <da...@apache.org>.
On Sun, Jul 07, 2013 at 05:54:03PM +0200, Roderich Schupp wrote:
> On Sun, Jul 7, 2013 at 5:11 PM, Daniel Shahaf <da...@apache.org> wrote:
>
> > Nothing is useful without proper documentation, but devil's advocate
> > says the bindings can be quite usable if you document only the delta
> > between SVN::Client->info42 and svn_client_info42().
> >
>
> First of all, you have to tell people that there is a SVN::Client->info42
> at all.
> The current POD only talks about SVN::Client->info, you have to look
> into the SVN::Client (Perl) source to see that info42 is in the @all_fn
> list,
> so it's probably wrapped (of course, this could also just be a typo).
> So that't the reason for the ADDITIONAL METHODS section.
>
> As for the "delta" (Perl vs C) argument: all Perl etc bindings for large
> C-based libraries have this problem. With my Perl developer hat on
> I would probably scoff at your suggestion - if I want to hack Subversion
> in Perl why should I be forced to look at C documentation?
>
Yes it would be useful to have documents in pod/perldoc format, but how
do we maintain them?
If a change is made to the docstring of a released API (yes, this should
be rare, but it's not unheard of: svn_client_relocate, svn_client_blame5),
how do we ensure that the docstring of the wrapped API gets the same change?
Is it really the best use of volunteer time to manually maintain a copy
of all our docstrings?
% wc subversion/include/*.h | tail -n1
43515 205066 1587361 total
Or perhaps we should look into a way to generate
the Perl bindings' docstrings from the C API docstrings?
> Cheers, Roderich
Re: svn commit: r1488693 - /subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Roderich Schupp <ro...@gmail.com>.
On Sun, Jul 7, 2013 at 5:11 PM, Daniel Shahaf <da...@apache.org> wrote:
> Nothing is useful without proper documentation, but devil's advocate
> says the bindings can be quite usable if you document only the delta
> between SVN::Client->info42 and svn_client_info42().
>
First of all, you have to tell people that there is a SVN::Client->info42
at all.
The current POD only talks about SVN::Client->info, you have to look
into the SVN::Client (Perl) source to see that info42 is in the @all_fn
list,
so it's probably wrapped (of course, this could also just be a typo).
So that't the reason for the ADDITIONAL METHODS section.
As for the "delta" (Perl vs C) argument: all Perl etc bindings for large
C-based libraries have this problem. With my Perl developer hat on
I would probably scoff at your suggestion - if I want to hack Subversion
in Perl why should I be forced to look at C documentation?
Cheers, Roderich
Re: svn commit: r1488693 -
/subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Daniel Shahaf <da...@apache.org>.
Roderich Schupp wrote on Sun, Jul 07, 2013 at 17:04:07 +0200:
> On Sun, Jul 7, 2013 at 3:14 PM, Daniel Shahaf <da...@apache.org> wrote:
>
> > > - POD: Use modern sub parameter conventions in POD (no more
> > "shift"ing).
> >
> > These all non-functional changes. We don't normally backport such changes
> >
>
> There's a difference between Subversion proper and the (Perl, Python, Ruby)
> bindings. The latter are useless without proper documentation,
Nothing is useful without proper documentation, but devil's advocate
says the bindings can be quite usable if you document only the delta
between SVN::Client->info42 and svn_client_info42().
> so IMHO the POD is "functional".
You changed:
sub {
my $x = shift;
my $y = shift;
}
to:
sub {
my ($x, $y) = @_;
}
That is not a functional change. It is not visible to callers or to
documentation readers.
Re: svn commit: r1488693 - /subversion/trunk/subversion/bindings/swig/perl/native/Client.pm
Posted by Roderich Schupp <ro...@gmail.com>.
On Sun, Jul 7, 2013 at 3:14 PM, Daniel Shahaf <da...@apache.org> wrote:
> > - POD: Use modern sub parameter conventions in POD (no more
> "shift"ing).
>
> These all non-functional changes. We don't normally backport such changes
>
There's a difference between Subversion proper and the (Perl, Python, Ruby)
bindings. The latter are useless without proper documentation, so IMHO
the POD is "functional". If these SVN::* modules were real CPAN modules,
I wouldn't touch them with a ten foot pole because of the obviously outdated
and incomplete documentation.
Cheers, Roderich