Skip Menu |
 

Preferred bug tracker

Please visit the preferred bug tracker to report your issue.

This queue is for tickets about the PPI CPAN distribution.

Report information
The Basics
Id: 34307
Status: resolved
Worked: 30 min
Priority: 0/
Queue: PPI

People
Owner: Nobody in particular
Requestors: matisse [...] spamcop.net
Cc:
AdminCc:

Bug Information
Severity: (no value)
Broken in: 1.201
Fixed in: (no value)



Subject: Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?
Download (untitled) / with headers
text/plain 440b
In PPI::Element the documentation describes insert_before() and insert_after() but the actual code seems to name those methods with TWO leading underscores: sub __insert_before { my $self = shift; $self->parent->__insert_before_child( $self, @_ ); } sub __insert_after { my $self = shift; $self->parent->__insert_after_child( $self, @_ ); } What's up with that? Are the docs wrong? Should the method names be changed? Something else?
Subject: Re: [rt.cpan.org #34307] Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?
Date: Fri, 21 Mar 2008 17:41:10 +1100
To: bug-PPI [...] rt.cpan.org
From: Adam Kennedy <adamkennedybackup [...] gmail.com>
Download (untitled) / with headers
text/plain 1.3k
The source code contains two versions, the public insert_before, and the internal and extremely unsafe non-param-checking __insert_before. The public method is for the public, and checks params properly. The internal methods do the actual changes and don't check params at all. The exist to allow internal code to use an optimised path, rather than slowing it down by excessive param-checking. Adam K Matisse Enzer via RT wrote: Show quoted text
> Thu Mar 20 20:34:17 2008: Request 34307 was acted upon. > Transaction: Ticket created by MATISSE > Queue: PPI > Subject: Are PPI::Element::__insert_before() and PPI::Element::__insert_after() > really private? > Broken in: 1.201 > Severity: (no value) > Owner: Nobody > Requestors: matisse@spamcop.net > Status: new > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=34307 > > > > In PPI::Element the documentation describes insert_before() and > insert_after() but the actual code seems to name those methods with TWO > leading underscores: > > sub __insert_before { > my $self = shift; > $self->parent->__insert_before_child( $self, @_ ); > } > > sub __insert_after { > my $self = shift; > $self->parent->__insert_after_child( $self, @_ ); > } > > What's up with that? Are the docs wrong? Should the method names be > changed? Something else? >
On Fri Mar 21 02:47:51 2008, adamkennedybackup@gmail.com wrote: Show quoted text
> The source code contains two versions, the public insert_before, and > the > internal and extremely unsafe non-param-checking __insert_before. > > The public method is for the public, and checks params properly. > > The internal methods do the actual changes and don't check params at > all. The exist to allow internal code to use an optimised path, rather > than slowing it down by excessive param-checking.
Thanks for the explanation. I've attached a test script that shows the private method working where the public method fails. I suspect this is because the node i am inserting in the test isa 'PPI::Document', rather than some other type - but I don't know how I would create a node type that the public insert_before() would allow. So perhaps the documentation should mention this somehow? In my test script i do: my $new_node = PPI::Document->new( \$new_content ); where what I suppose what I should do (if it worked) would be: my $new_node = PPI::Statement->new( \$new_content );
Download ppi_test.pl
text/x-perl 1.4k
use strict; use warnings; use Test::More; use PPI; my %test_cases = ( 'use Foo;' => 'use Matisse::Foo;', 'use Foo::Bar;' => 'use Matisse::Foo::Bar;', 'use Foo::Bar qw( Biff Baz Boffo );' => 'use Matisse::Foo::Bar qw( Biff Baz Boffo );', ); my $test_count = 2 * scalar keys %test_cases; Test::More::plan( tests => $test_count ); foreach my $orig_code ( sort keys %test_cases ) { my $expected_code = $test_cases{$orig_code}; my $method = 'insert_before'; my $got_code_using_public_insert_before = modify_node_using( $method, $orig_code ); Test::More::is( $got_code_using_public_insert_before, $expected_code, "Public $method '$orig_code'" ); } foreach my $orig_code ( sort keys %test_cases ) { my $expected_code = $test_cases{$orig_code}; my $method = '__insert_before'; my $got_code_using_private_insert_before = modify_node_using( $method, $orig_code ); Test::More::is( $got_code_using_private_insert_before, $expected_code, "Private $method '$orig_code'" ); } sub modify_node_using { my $method = shift; my $orig_code = shift; my $ppi_doc = PPI::Document->new( \$orig_code ); my $include = $ppi_doc->find_first('PPI::Statement::Include'); my $new_content = $include->content(); $new_content =~ s/(\s)Foo/$1Matisse::Foo/; my $new_node = PPI::Document->new( \$new_content ); $include->$method($new_node); $include->remove(); my $got_code = $ppi_doc->content(); return $got_code; }
Download (untitled) / with headers
text/plain 419b
I found a way to create a new 'PPI::Statement::Include' from a string. So, maybe this should be added to the docs, or maybe the new() for PPI::Element should be changed to handle this directly? my $code_string = 'use Foo::Bar::Baz;'; my $new_doc = PPI::Document->new( \$code_string ); my $new_include = $new_doc->find_first('PPI::Statement::Include'); # $new_include->isa('PPI::Statement::Include')
Subject: Re: [rt.cpan.org #34307] Are PPI::Element::__insert_before() and PPI::Element::__insert_after() really private?
Date: Mon, 24 Mar 2008 13:35:30 +1100
To: bug-PPI [...] rt.cpan.org
From: Adam Kennedy <adamkennedybackup [...] gmail.com>
Download (untitled) / with headers
text/plain 1.3k
1. Create the document from the string. 2. Extract the statement you care about from that document. 3. Insert it into the other document. Matisse Enzer via RT wrote: Show quoted text
> Queue: PPI > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=34307 > > > On Fri Mar 21 02:47:51 2008, adamkennedybackup@gmail.com wrote: >
>> The source code contains two versions, the public insert_before, and >> the >> internal and extremely unsafe non-param-checking __insert_before. >> >> The public method is for the public, and checks params properly. >> >> The internal methods do the actual changes and don't check params at >> all. The exist to allow internal code to use an optimised path, rather >> than slowing it down by excessive param-checking. >>
> > Thanks for the explanation. > > I've attached a test script that shows the private method working where > the public method fails. > > I suspect this is because the node i am inserting in the test isa > 'PPI::Document', rather than some other type - but I don't know how I > would create a node type that the public insert_before() would allow. > > So perhaps the documentation should mention this somehow? > > In my test script i do: > > my $new_node = PPI::Document->new( \$new_content ); > > where what I suppose what I should do (if it worked) would be: > > my $new_node = PPI::Statement->new( \$new_content ); > >
No changes to the code required, resolving this bug


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

Please report any issues with rt.cpan.org to rt-cpan-admin@bestpractical.com.