parseperl-discuss Mailing List for Parse::Perl (Page 3)
Brought to you by:
adamkennedy
Menu
▾
▴
parseperl-discuss — Parse::Perl Discussion List
You can subscribe to this list here.
| 2005 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
(1) |
Jul
(10) |
Aug
(8) |
Sep
(1) |
Oct
|
Nov
(3) |
Dec
|
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2006 |
Jan
(2) |
Feb
(2) |
Mar
|
Apr
|
May
|
Jun
|
Jul
(5) |
Aug
(2) |
Sep
(12) |
Oct
(26) |
Nov
|
Dec
|
| 2007 |
Jan
|
Feb
|
Mar
(2) |
Apr
|
May
(1) |
Jun
|
Jul
|
Aug
(1) |
Sep
(1) |
Oct
(2) |
Nov
|
Dec
|
| 2008 |
Jan
|
Feb
|
Mar
|
Apr
|
May
(3) |
Jun
(1) |
Jul
(3) |
Aug
|
Sep
(3) |
Oct
|
Nov
|
Dec
(1) |
| 2009 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
(2) |
Nov
|
Dec
|
| 2010 |
Jan
|
Feb
|
Mar
(4) |
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
| 2014 |
Jan
(1) |
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
|
Oct
|
Nov
|
Dec
|
|
From: Chris D. <ch...@cl...> - 2006-09-25 18:10:48
|
I discovered a problem with anonymous hashref parsing (a pair of
failing tests added to SVN as rev 1103). The attached fix passes all
PPI and Perl::Critic tests, but breaks the following common idiom:
my %foo = map({$_ => 1} @foo);
I think the lexer needs a lookahead to detect the lack of a following
comma to distinguish the following cases:
map({$_ => 1} @foo
foo({$_ => 1}, @foo);
That seems like a non-trivial change to the Lexer code, so I'm going
to set it aside for now.
Chris
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Adam K. <ad...@ph...> - 2006-09-23 04:08:15
|
Roger Releasing now (good thing too, because I need it to release Module::Inspector 0.01). :) Adam K Chris Dolan wrote: > Adam, > > In case you haven't already released PPI v1.118: I bless SVN rev 1079 > as ready for release. It works with Perl::Critic HEAD and I've > tested the heck out of PPI::Token::Number. > > Chris > > -- > Chris Dolan, Software Developer, http://www.chrisdolan.net/ > Public key: http://www.chrisdolan.net/public.key > vCard: http://www.chrisdolan.net/ChrisDolan.vcf > > > > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Parseperl-discuss mailing list > Par...@li... > https://lists.sourceforge.net/lists/listinfo/parseperl-discuss |
|
From: Chris D. <ch...@ch...> - 2006-09-23 03:31:57
|
Adam, In case you haven't already released PPI v1.118: I bless SVN rev 1079 as ready for release. It works with Perl::Critic HEAD and I've tested the heck out of PPI::Token::Number. Chris -- Chris Dolan, Software Developer, http://www.chrisdolan.net/ Public key: http://www.chrisdolan.net/public.key vCard: http://www.chrisdolan.net/ChrisDolan.vcf |
|
From: Adam K. <ad...@ph...> - 2006-09-22 06:12:03
|
The Number::Unicode thing is an odd one. As I understand it, a number like 123.123.123.123 (yes, that looks like an IP) or any sequence of numbers seperated by dots is parsed as some sort of literal unicode sequence. I'm still a little fuzzy on the exact details, but apparent it's something odd like that. So we need a Number::Unicode to represent that. As for decimal, my gut says we need to make the base class completely abstract. I guess it's less likely to be ::Number::Decimal and more likely something similar to Number::Real, unless we split into Number::Decimal and Number::IEEE or something for the odd cases. But talk to Audrey in #perl6 about the unicode thing, she knows more. Adam K Chris Dolan wrote: > On Sep 22, 2006, at 12:06 AM, Adam Kennedy wrote: > >> Audrey had the right idea. >> >> Instead of having a ->base or whatever, we should be subclassing >> Number into a set of subclasses, one for each type of number Perl >> supports. >> >> So Number::Boolean, Number::Hexidecimal, Number::Unicode etc >> >> So ignoring the parsing half, I'd start by setting up the subclasses, >> and then look at how the implementation we need fits into that model. > > Excellent. That sounds like a good approach to me. But I don't know > what Number::Unicode means... Also, should there be a Number::Decimal, > or should that just be Number.pm? > >> But I'm pretty excited about this, since I've wanted it done for ages. >> >> And with ->literal working at the Number level, it's fairly easy to >> start pushing it upwards, add support for strings, and anonymous >> hashref constructors and so on, until we can handle any arbitrarily >> deeply nested thing, so long as it is all literals with no need to >> resort to the Perl interpreter (and thus avoid the Halting Problem >> issues). >> >> Are you planning on branching SVN for this? Or should we just release >> 1.118 and then you can hack away at the trunk. (I'm cool with either) > > Trunk, I think. I'm an intermittent hacker ($kid and $job), so a branch > would probably just bitrot. My "short term" means about a month. > > In any case, my first task will be test coverage, as I said before. I > want to get the current stuff debugged before adding new features. > > Chris > > P.S. > Is "hexidecimal" some odd aussie spelling for hexadecimal? :-P > I inferred from the POD today that it pains you to spell > "tokeniser"/"tokenizer"! > I tried really hard not to americanize the spelling all over the POD > this morning... > > -- > Chris Dolan, Software Developer, http://www.chrisdolan.net/ > Public key: http://www.chrisdolan.net/public.key > vCard: http://www.chrisdolan.net/ChrisDolan.vcf > > |
|
From: Chris D. <ch...@ch...> - 2006-09-22 05:28:23
|
On Sep 22, 2006, at 12:06 AM, Adam Kennedy wrote: > Audrey had the right idea. > > Instead of having a ->base or whatever, we should be subclassing > Number into a set of subclasses, one for each type of number Perl > supports. > > So Number::Boolean, Number::Hexidecimal, Number::Unicode etc > > So ignoring the parsing half, I'd start by setting up the > subclasses, and then look at how the implementation we need fits > into that model. Excellent. That sounds like a good approach to me. But I don't know what Number::Unicode means... Also, should there be a Number::Decimal, or should that just be Number.pm? > But I'm pretty excited about this, since I've wanted it done for ages. > > And with ->literal working at the Number level, it's fairly easy to > start pushing it upwards, add support for strings, and anonymous > hashref constructors and so on, until we can handle any arbitrarily > deeply nested thing, so long as it is all literals with no need to > resort to the Perl interpreter (and thus avoid the Halting Problem > issues). > > Are you planning on branching SVN for this? Or should we just > release 1.118 and then you can hack away at the trunk. (I'm cool > with either) Trunk, I think. I'm an intermittent hacker ($kid and $job), so a branch would probably just bitrot. My "short term" means about a month. In any case, my first task will be test coverage, as I said before. I want to get the current stuff debugged before adding new features. Chris P.S. Is "hexidecimal" some odd aussie spelling for hexadecimal? :-P I inferred from the POD today that it pains you to spell "tokeniser"/"tokenizer"! I tried really hard not to americanize the spelling all over the POD this morning... -- Chris Dolan, Software Developer, http://www.chrisdolan.net/ Public key: http://www.chrisdolan.net/public.key vCard: http://www.chrisdolan.net/ChrisDolan.vcf |
|
From: Adam K. <ad...@ph...> - 2006-09-22 05:06:21
|
Audrey had the right idea. Instead of having a ->base or whatever, we should be subclassing Number into a set of subclasses, one for each type of number Perl supports. So Number::Boolean, Number::Hexidecimal, Number::Unicode etc So ignoring the parsing half, I'd start by setting up the subclasses, and then look at how the implementation we need fits into that model. But I'm pretty excited about this, since I've wanted it done for ages. And with ->literal working at the Number level, it's fairly easy to start pushing it upwards, add support for strings, and anonymous hashref constructors and so on, until we can handle any arbitrarily deeply nested thing, so long as it is all literals with no need to resort to the Perl interpreter (and thus avoid the Halting Problem issues). Are you planning on branching SVN for this? Or should we just release 1.118 and then you can hack away at the trunk. (I'm cool with either) Adam K Chris Dolan wrote: > Here's my goals for PPI::Token::Number for the near future: > * Improve test coverage > - "0" > - "0_x1" > - "_" in general > - base() > * Add support for exponential notation > * Consider removing/upgrading support for base256 > * Ponder support for literal() > http://rt.cpan.org/Public/Bug/Display.html?id=14868 > Something like that is needed to get rid of the last bad > stringy eval in Perl::Critic > > Chris > -- > Chris Dolan, Software Developer, Clotho Advanced Media Inc. > 608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703 > vCard: http://www.chrisdolan.net/ChrisDolan.vcf > > Clotho Advanced Media, Inc. - Creators of MediaLandscape Software > (http://www.media-landscape.com/) and partners in the revolutionary > Croquet project (http://www.opencroquet.org/) > > > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Parseperl-discuss mailing list > Par...@li... > https://lists.sourceforge.net/lists/listinfo/parseperl-discuss |
|
From: Chris D. <ch...@cl...> - 2006-09-22 05:01:32
|
Here's my goals for PPI::Token::Number for the near future:
* Improve test coverage
- "0"
- "0_x1"
- "_" in general
- base()
* Add support for exponential notation
* Consider removing/upgrading support for base256
* Ponder support for literal()
http://rt.cpan.org/Public/Bug/Display.html?id=14868
Something like that is needed to get rid of the last bad
stringy eval in Perl::Critic
Chris
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Adam K. <ad...@ph...> - 2006-09-22 03:00:57
|
So I've added you as co-maint for the main PPI namespace, in a few days that should get picked up by RT, I think. If that doesn't work I'll add you for ALL the PPI namespaces, and we'll see if that makes it work. Adam K Chris Dolan wrote: > Adam, > > I'd like to help clear RT tickets for PPI. Can you either add CDOLAN > to the RT list, or add this email list to the RT cc list? > > Some of my fellow Perl::Critic developers have been posting PPI bugs > that I keep missing. > > Thanks, > Chris > > -- > Chris Dolan, Software Developer, http://www.chrisdolan.net/ > Public key: http://www.chrisdolan.net/public.key > vCard: http://www.chrisdolan.net/ChrisDolan.vcf > > > > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Parseperl-discuss mailing list > Par...@li... > https://lists.sourceforge.net/lists/listinfo/parseperl-discuss |
|
From: Adam K. <ad...@ph...> - 2006-09-22 02:29:00
|
I'm not entirely sure how to do that, but I'll take a look. Adam K Chris Dolan wrote: > Adam, > > I'd like to help clear RT tickets for PPI. Can you either add CDOLAN > to the RT list, or add this email list to the RT cc list? > > Some of my fellow Perl::Critic developers have been posting PPI bugs > that I keep missing. > > Thanks, > Chris > > -- > Chris Dolan, Software Developer, http://www.chrisdolan.net/ > Public key: http://www.chrisdolan.net/public.key > vCard: http://www.chrisdolan.net/ChrisDolan.vcf > > > > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Parseperl-discuss mailing list > Par...@li... > https://lists.sourceforge.net/lists/listinfo/parseperl-discuss |
|
From: Adam K. <ad...@ph...> - 2006-09-22 02:15:42
|
Welcome to the insanity Chris :)
Adam K
Chris Dolan wrote:
> Peter,
>
> Thanks for the report. Those are now fixed in PPI SVN revision 1054.
> Amusingly, *= and /= were two unrelated bugs.
>
> Chris
>
> On Sep 21, 2006, at 11:18 AM, Peter Guzis wrote:
>
>> Hello Chris,
>>
>> PPI doesn't properly parse some multi-character operators such as *=
>> and /=, even though the POD says these are supported. Instead, the
>> aforementioned operators are recognized separately as *, =, /, and =.
>>
>> I got the policy working with a very simple fix, so I did not submit
>> another PPI bug report. My workaround was to concatenate adjacent
>> operators ( e.g. *, =) and skip processing the second operator (=)
>> separately. If you want to fix PPI instead, that would be an even
>> better solution.
>>
>> Let me know if you need more information.
>>
>> Thanks,
>>
>> Peter
>>
>> On 9/21/06, *Chris Dolan* <ch...@cl... <mailto:ch...@cl...>>
>> wrote:
>>
>> Hey Peter,
>>
>> I was just looking over the ProhibitMismatchedOperators policy and
>> saw these lines:
>>
>> # work around PPI operator parsing bugs
>>
>> return if $prev_elem->isa('PPI::Token::Operator');
>>
>> What's that all about? Anything I can try to fix?
>>
>> Chris
>>
>>
>
> --
> Chris Dolan, Software Developer, Clotho Advanced Media Inc.
> 608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
> vCard: http://www.chrisdolan.net/ChrisDolan.vcf
>
> Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
> (http://www.media-landscape.com/) and partners in the revolutionary
> Croquet project (http://www.opencroquet.org/)
>
>
>
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> Take Surveys. Earn Cash. Influence the Future of IT
> Join SourceForge.net's Techsay panel and you'll get the chance to share your
> opinions on IT & business topics through brief surveys -- and earn cash
> http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Parseperl-discuss mailing list
> Par...@li...
> https://lists.sourceforge.net/lists/listinfo/parseperl-discuss
|
|
From: Chris D. <ch...@cl...> - 2006-09-21 18:22:13
|
Peter,
Thanks for the report. Those are now fixed in PPI SVN revision
1054. Amusingly, *= and /= were two unrelated bugs.
Chris
On Sep 21, 2006, at 11:18 AM, Peter Guzis wrote:
> Hello Chris,
>
> PPI doesn't properly parse some multi-character operators such as
> *= and /=, even though the POD says these are supported. Instead,
> the aforementioned operators are recognized separately as *, =, /,
> and =.
>
> I got the policy working with a very simple fix, so I did not
> submit another PPI bug report. My workaround was to concatenate
> adjacent operators ( e.g. *, =) and skip processing the second
> operator (=) separately. If you want to fix PPI instead, that would
> be an even better solution.
>
> Let me know if you need more information.
>
> Thanks,
>
> Peter
>
> On 9/21/06, Chris Dolan <ch...@cl... > wrote:
> Hey Peter,
>
> I was just looking over the ProhibitMismatchedOperators policy and
> saw these lines:
>
> # work around PPI operator parsing bugs
>
> return if $prev_elem->isa('PPI::Token::Operator');
>
> What's that all about? Anything I can try to fix?
>
> Chris
>
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Chris D. <ch...@ch...> - 2006-09-21 14:07:54
|
Adam, I'd like to help clear RT tickets for PPI. Can you either add CDOLAN to the RT list, or add this email list to the RT cc list? Some of my fellow Perl::Critic developers have been posting PPI bugs that I keep missing. Thanks, Chris -- Chris Dolan, Software Developer, http://www.chrisdolan.net/ Public key: http://www.chrisdolan.net/public.key vCard: http://www.chrisdolan.net/ChrisDolan.vcf |
|
From: Chris D. <ch...@cl...> - 2006-08-31 18:51:23
|
Adam,
Sorry for posting these bugs here instead of to RT, but it looks like
you were about to do a 1.116 release, so I thought I'd hurry. I've
found these regressions by running the Perl-Critic test suite against
the PPI svn head (r955).
Code:
{
print 123;
}
sub a {
return 123;
}
1.115 parse tree:
PPI::Document
PPI::Structure::Block { ... }
PPI::Statement
PPI::Token::Word 'print'
PPI::Token::Number '123'
PPI::Token::Structure ';'
PPI::Statement::Sub
PPI::Token::Word 'sub'
PPI::Token::Word 'a'
PPI::Structure::Block { ... }
PPI::Statement::Break
PPI::Token::Word 'return'
PPI::Token::Number '123'
PPI::Token::Structure ';'
1.116 parse tree:
PPI::Document
PPI::Statement
PPI::Structure::Block { ... }
PPI::Statement
PPI::Token::Word 'print'
PPI::Token::Number '123'
PPI::Token::Structure ';'
PPI::Token::Word 'sub'
PPI::Token::Word 'a'
PPI::Structure::Block { ... }
PPI::Statement::Break
PPI::Token::Word 'return'
PPI::Token::Number '123'
PPI::Token::Structure ';'
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Chris D. <ch...@cl...> - 2006-08-31 18:34:26
|
Adam,
I wanted to catch you before you released PPI 1.116 if possible.
With 1.115, the code "-t;" parsed as
PPI::Document
PPI::Statement
PPI::Token::Operator '-t'
PPI::Token::Structure ';'
With 1.116, it parses as:
PPI::Document
PPI::Statement
PPI::Token::Number '-'
PPI::Token::Word 't'
PPI::Token::Structure ';'
I believe this happened in SVN change r953.
Chris
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Chris D. <ch...@ch...> - 2006-07-20 14:52:25
|
On Jul 20, 2006, at 2:42 AM, Adam Kennedy wrote:
>>> my $doc = PPI::Document->new( $something,
>>> readonly => 1,
>>> );
>> I was thinking something quite similar earlier today, but I
>> couldn't think of a good way to implement it without either 1)
>> sprinkling "if ($self->top->readonly)" everywhere, or 2) multiple
>> inheritance or mixin of a ::Mutable class for the non-readonly case.
>
> Agreed, actually checking it in every ->prune call would be onerous
> and slow things down, and doing some form of mutation or mixin for
> non-read-only seems infeasible without having entire seperate
> trees, tripling the number of classes. :/
Like you suggested, I'll play with PPI-XS before actually trying to
optimize PPI, but this is an interesting topic.
How about using caches with epoch number? It might still be too much
overhead (especially in $self->top), but here's some noodling:
package PPI::Node;
sub remove {
...
$self->top->inc_epoch;
...
}
sub inc_epoch {
$self->{epoch}++;
# maybe delete some caches pre-emptively?
}
sub epoch {
$self->{epoch} || 0;
}
sub content {
my ($self) = @_;
my $epoch = $self->top->epoch;
$self->{cache} = {epoch => $epoch} if (!$self->{cache} || $self-
>{cache}->{epoch} != $epoch);
return $self->{cache}->{content} ||= join q{}, map { $_-
>content } @{$self->{children}};
}
If the lexer can be exempt from caching and the ratio of reads to
writes is large (like Perl::Critic), this could be a win, but I'm not
sure.
Chris
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Adam K. <ad...@ph...> - 2006-07-20 07:44:40
|
>> my $doc = PPI::Document->new( $something, >> readonly => 1, >> ); > > I was thinking something quite similar earlier today, but I couldn't > think of a good way to implement it without either 1) sprinkling "if > ($self->top->readonly)" everywhere, or 2) multiple inheritance or mixin > of a ::Mutable class for the non-readonly case. Agreed, actually checking it in every ->prune call would be onerous and slow things down, and doing some form of mutation or mixin for non-read-only seems infeasible without having entire seperate trees, tripling the number of classes. :/ > Wait, are you suggesting a global readonly flag, or a per-instance > readonly? It sounds like the former. I was instead thinking of the > latter. What I'm thinking is that we make ->readonly an advisory flag. If you create with ->readonly, then at least we (or anyone else) can know that the document as a whole is _intended_ to be readonly. We might not actually enforce it at the add_before level, but it can be checked in all sorts of places like PPI::Transform and so on, that deal with entire documents. It means at least anything that cares about readonly has the option to look for it. It also means we can then IN ADDITION to the advisory ->readonly attribute, we can implement a global ReadOnly mode to modify the function undef PPI::... Once that global ReadOnly is enabled, PPI::Document->new would simply die or return undef, and refuse to allow any document to be created that _doesn't_ have the ->readonly flag. So it would be a partly-cooperative, partly-enforced concept. Enforced at the per-document level, but cooperative at the per-element level. Because indeed doing ->top->readonly all over the place would be fairly hideous and slow things down (although we can look at adding it down the line). Adam K |
|
From: Chris D. <ch...@ch...> - 2006-07-20 07:24:56
|
On Jul 20, 2006, at 12:57 AM, Adam Kennedy wrote: > Both of those look like fairly straight forward CPU vs memory > tradeoffs. > > Both assume the document is readonly (which PPI itself doesn't do) > although that's fine in your case. That's right. > The second one is dangerous, because it assumes that ALL PPI document > loaded in the entire process will always be readonly. It would be > unsafe > if Perl::Critic was ever used in the same process as something that > manipulated documents. Agreed, which is why I didn't commit it to Perl::Critic. > I think we might be able to make some trivial changes to make what you > are doing a little safer though. > > Firstly, we could add a ->readonly flag to PPI::Document, which you > would set with > > my $doc = PPI::Document->new( $something, > readonly => 1, > ); I was thinking something quite similar earlier today, but I couldn't think of a good way to implement it without either 1) sprinkling "if ($self->top->readonly)" everywhere, or 2) multiple inheritance or mixin of a ::Mutable class for the non-readonly case. > I've had this document syntax planned for some time now, so we > could do > things like tabwidth => 8 and so on. > > What that would then allow you to do is to have some sort of a module > that aggressively tweaks PPI's internals for the read-only case. > I'd be > cool with you guys maintaining it, and then bundling it into the main > PPI distribution. > > When this mode was enabled, the document constructor would refuse to > create any document that didn't have ->readonly set to true, so > that the > assumptions set by your optimisation hacks would be forced to hold > true. Wait, are you suggesting a global readonly flag, or a per-instance readonly? It sounds like the former. I was instead thinking of the latter. > This way, if you are running a dedicated perlcritic process, you can > turn on ReadOnly mode and make perlcritic go faster, but in the > case you > need to mix critic into some other program like an editor, it will > still > work, just slower. That certainly makes sense, whether global or per-instance. > A simple technique is already used by PPI::XS, which overlays faster > versions of several functions over the top of the default PPI ones. > > Further, if you happen to know a decent XS coder, get them to do some > hacking on PPI::XS. The framework and backwards+forwards compatibility > capability is already in place, we just need people to write faster > versions of single functions. > > Between XS improvements, and a formalised ReadOnly mode, you should > have > much much greater scope for writing more and more-varied > optimisations. > > How does that sound? Great. I'll play with PPI::XS. Chris -- Chris Dolan, Software Developer, http://www.chrisdolan.net/ Public key: http://www.chrisdolan.net/public.key vCard: http://www.chrisdolan.net/ChrisDolan.vcf |
|
From: Adam K. <ad...@ph...> - 2006-07-20 05:59:25
|
Both of those look like fairly straight forward CPU vs memory tradeoffs. Both assume the document is readonly (which PPI itself doesn't do) although that's fine in your case. The second one is dangerous, because it assumes that ALL PPI document loaded in the entire process will always be readonly. It would be unsafe if Perl::Critic was ever used in the same process as something that manipulated documents. I think we might be able to make some trivial changes to make what you are doing a little safer though. Firstly, we could add a ->readonly flag to PPI::Document, which you would set with my $doc = PPI::Document->new( $something, readonly => 1, ); I've had this document syntax planned for some time now, so we could do things like tabwidth => 8 and so on. What that would then allow you to do is to have some sort of a module that aggressively tweaks PPI's internals for the read-only case. I'd be cool with you guys maintaining it, and then bundling it into the main PPI distribution. When this mode was enabled, the document constructor would refuse to create any document that didn't have ->readonly set to true, so that the assumptions set by your optimisation hacks would be forced to hold true. This way, if you are running a dedicated perlcritic process, you can turn on ReadOnly mode and make perlcritic go faster, but in the case you need to mix critic into some other program like an editor, it will still work, just slower. A simple technique is already used by PPI::XS, which overlays faster versions of several functions over the top of the default PPI ones. Further, if you happen to know a decent XS coder, get them to do some hacking on PPI::XS. The framework and backwards+forwards compatibility capability is already in place, we just need people to write faster versions of single functions. Between XS improvements, and a formalised ReadOnly mode, you should have much much greater scope for writing more and more-varied optimisations. How does that sound? Adam K Chris Dolan wrote: > The Perl::Critic team has been working on some performance > improvements, some of which rely on caching PPI information. If you > have created any PPI performance hacks, we'd love to hear about them, > either on this list or on de...@pe... (subscribe via > http://perlcritic.tigris.org/servlets/ProjectMailingListList) > > To see our accomplishments to date, take a look at (for example): > > PPI::Document::find() caching > http://perlcritic.tigris.org/source/browse/perlcritic/trunk/Perl- > Critic/lib/Perl/Critic/Document.pm?rev=527&view=markup > > PPI::Node::content() caching > http://perlcritic.tigris.org/servlets/ReadMsg?list=dev&msgNo=464 > > Chris > > -- > Chris Dolan, Software Developer, http://www.chrisdolan.net/ > Public key: http://www.chrisdolan.net/public.key > vCard: http://www.chrisdolan.net/ChrisDolan.vcf > > > > > ------------------------------------------------------------------------- > Take Surveys. Earn Cash. Influence the Future of IT > Join SourceForge.net's Techsay panel and you'll get the chance to share your > opinions on IT & business topics through brief surveys -- and earn cash > http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV > _______________________________________________ > Parseperl-discuss mailing list > Par...@li... > https://lists.sourceforge.net/lists/listinfo/parseperl-discuss |
|
From: Chris D. <ch...@ch...> - 2006-07-20 05:13:47
|
The Perl::Critic team has been working on some performance improvements, some of which rely on caching PPI information. If you have created any PPI performance hacks, we'd love to hear about them, either on this list or on de...@pe... (subscribe via http://perlcritic.tigris.org/servlets/ProjectMailingListList) To see our accomplishments to date, take a look at (for example): PPI::Document::find() caching http://perlcritic.tigris.org/source/browse/perlcritic/trunk/Perl- Critic/lib/Perl/Critic/Document.pm?rev=527&view=markup PPI::Node::content() caching http://perlcritic.tigris.org/servlets/ReadMsg?list=dev&msgNo=464 Chris -- Chris Dolan, Software Developer, http://www.chrisdolan.net/ Public key: http://www.chrisdolan.net/public.key vCard: http://www.chrisdolan.net/ChrisDolan.vcf |
|
From: Jeffrey G. <dr...@po...> - 2006-02-18 06:38:43
|
Patch for Document.pm - The find() member actually returns an arrayref
of nodes, not a list of nodes as mentioned. find() isn't mentioned
further on in the POD, so this area is the only one that needs to be
patched. diff -ru output follows, along with a thought.
--cut here--
--- Document.pm Fri Feb 17 22:02:49 2006
+++ Document.pm.patched Fri Feb 17 22:03:40 2006
@@ -23,7 +23,7 @@
$Document->prune('PPI::Token::Comment');
# Find all the named subroutines
- my @subs = $Document->find(
+ my $subs = $Document->find(
sub { $_[1]->isa('PPI::Statement::Sub') and $_[1]->name }
);
--cut here--
And now for the thought. C<find()> is great at what it does, and can
easily handle the task I've set before it, but here's the situation.
I'm parsing code like:
--cut here--
if(level_1()) {
if(level_2()) {
}
else {
}
else {
}
--cut here--
and I want to collect the if() and else{} blocks at the top level,
without having the find() function descend into the if(){} block as I
want to process those nodes specially later. According to the find()
docs I can stop it from descending into the subtrees by returning
undef, but this also causes the if() node I was interested in to not be
collected in the output.
This can, of course, be mitigated by something like:
--cut here--
my @outside_var=();
$doc->find(sub{$_[1]->isa('PPI::Token::Word') and
$_[1]->content =~ /^if|else$/ and
push @outside_var,$_[1];
return undef;});
--cut here--
And so on. However, note that we're throwing away the output from
find() because it's all getting collected into @outside_var, and we
have to use a closure because find() won't accept a reference and pass
it along to the subroutine.
Of course, we can still create a reasonably generic packaging function
like this:
--cut here--
sub branch_walker {
my $collection = shift;
return sub {
$_[1]->isa('PPI::Token::Word') and
$_[1]->content =~ /^if|else$/ and
push @$collection,$_[1];
return undef
}
}
my @outside_var=();
$doc->find(branch_walker->(\@outside_var));
--cut here--
But that sort of circumlocution is unnecessary, when we can just pass
our collection variable in to C<find()> as an optional second
parameter, which passes the collection variable along to our finder
subroutine in $_[2].
--cut here--
my @collection = ();
$doc->walk(sub{
$_[1]->isa('PPI::Token::Word') and
$_[1]->content =~ /^if|else$/ and do {
push @$_[2],$_[1];
return undef
};
return 1;
});
--cut here--
I suspect, however, that the notion of C<walk()> either violates the
DOM spirit, which I can certainly understand, or it's just somewhere
else in the documentation. Comments? Thoughts?
--
Jeff Goff <dr...@po...>
|
|
From: Chris D. <ch...@ch...> - 2006-02-09 17:47:22
|
Adam et al.,
I've discovered that an empty list falsely returns undef from its
location() method. Consider this code snippet: "()" which yields a
tree like this:
PPI::Document
PPI::Structure::List ( ... )
If I try to access the location of that List, I get undef. For example:
use Data::Dumper;
use PPI::Document;
$d = PPI::Document->new(\"()");
$d->index_locations;
print Dumper($d->find("PPI::Structure::List")->[0]->location);
$VAR1 = undef;
This should instead do something like this:
print Dumper($d->find("PPI::Structure::List")->[0]->{start}-
>location);
$VAR1 = [1,1,1];
The reason is that the List has no children, and thus no PPI::Token
instances to hold a location. Instead, List nodes have "start" and
"finish" properties that hold their bracket tokens.
To solve this, I propose that the PPI::Structure::List class
overrides location() to return the location of its start token. If
this sounds sensible, I'd be happy to implement it.
Chris
--
Chris Dolan, Software Developer, http://www.chrisdolan.net/
Public key: http://www.chrisdolan.net/public.key
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
|
|
From: Chris D. <ch...@cl...> - 2006-01-25 17:05:53
|
Adam, Several users of Perl::Critic are being affected by the PPI::Statement::Sub bug that I patched in CVS on the 17th. Would you be willing to do another CPAN release incorporating this fix? Thanks, Chris -- Chris Dolan, Software Developer, Clotho Advanced Media Inc. 608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703 vCard: http://www.chrisdolan.net/ChrisDolan.vcf Clotho Advanced Media, Inc. - Creators of MediaLandscape Software (http://www.media-landscape.com/) and partners in the revolutionary Croquet project (http://www.opencroquet.org/) |
|
From: Chris D. <ch...@cl...> - 2006-01-23 03:28:53
|
I found a bug in the regexp parsing. I started trying to write the
test for the regression myself, but I'm unclear on what the correct
result should be for the t.data/08_regression/*.dump file, so I'm
posting here instead.
In CVS head, I get the following:
% perl -I lib -MPPI::Dumper -MPPI -e'PPI::Dumper->new(PPI::Document-
>new(\"/./ ; print 1"))->print'
PPI::Document
PPI::Statement
PPI::Token::Operator '/'
PPI::Token::Operator '.'
PPI::Token::Regexp::Match '/ ; print 1'
My guess is that the correct result should be something like
PPI::Document
PPI::Statement
PPI::Token::Regexp::Match '/./'
PPI::Token::Whitespace ' '
PPI::Token::Structure ';'
PPI::Token::Whitespace ' '
PPI::Statement
PPI::Token::Word 'print'
PPI::Token::Whitespace ' '
PPI::Token::Number '1'
If this tree looks right, I'd be happy to write up and commit the
test as a TODO.
Thanks,
Chris
--
Chris Dolan, Software Developer, Clotho Advanced Media Inc.
608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703
vCard: http://www.chrisdolan.net/ChrisDolan.vcf
Clotho Advanced Media, Inc. - Creators of MediaLandscape Software
(http://www.media-landscape.com/) and partners in the revolutionary
Croquet project (http://www.opencroquet.org/)
|
|
From: Chris D. <ch...@ch...> - 2005-11-02 06:40:21
|
Right, done. I also committed the negation work. Chris On Nov 1, 2005, at 11:39 PM, Adam Kennedy wrote: > Please back it out and put it under a branch. > > Unicode is the change I'm the most nervous about including and it > may need to hang around on the branch for a few releases until I'm > 100% confident on including it. > > At the very least I want it enabled and put through a full > tinderbox cycle. > > And thanks for the negations, they are fine. > > Adam K > > Chris Dolan wrote: > >> Hi all, >> I'm new to the PPI project -- Adam just added me to the >> sourceforge CVS committers list. >> I started work on a Unicode improvement yesterday to support basic >> UTF-8 I18N .pm files (see RT for more detail). At Adam's >> suggestion I was supposed to commit it to a branch, but I >> mistakenly committed it to the CVS trunk. Not a glorious >> beginning for me... Well, at least the changes were all well >> tested. Question: should I leave it, or back it out and re-commit >> on the branch I already created? >> I also am making a trivial change to extend the PPI::Element >> overload to include '!=' and 'ne' in addition to '==' and 'eq'. >> This one I will deliberately post to the trunk (since it's easy >> and obvious) as soon as I get an answer for the above question. >> Chris >> -- >> Chris Dolan, Software Developer, www.chrisdolan.net <http:// >> www.chrisdolan.net> >> Public key: http://www.chrisdolan.net/public.key >> > > > ------------------------------------------------------- > SF.Net email is sponsored by: > Tame your development challenges with Apache's Geronimo App Server. > Download > it for free - -and be entered to win a 42" plasma tv or your very own > Sony(tm)PSP. Click here to play: http://sourceforge.net/geronimo.php > _______________________________________________ > Parseperl-discuss mailing list > Par...@li... > https://lists.sourceforge.net/lists/listinfo/parseperl-discuss > -- Chris Dolan, Software Developer, Clotho Advanced Media Inc. 608-294-7900, fax 294-7025, 1435 E Main St, Madison WI 53703 Clotho Advanced Media, Inc. - Creators of MediaLandscape Software (http://www.media-landscape.com/) and partners in the revolutionary Croquet project (http://www.opencroquet.org/) |
|
From: Adam K. <ad...@ph...> - 2005-11-02 05:40:42
|
Please back it out and put it under a branch. Unicode is the change I'm the most nervous about including and it may need to hang around on the branch for a few releases until I'm 100% confident on including it. At the very least I want it enabled and put through a full tinderbox cycle. And thanks for the negations, they are fine. Adam K Chris Dolan wrote: > Hi all, > > I'm new to the PPI project -- Adam just added me to the sourceforge CVS > committers list. > > I started work on a Unicode improvement yesterday to support basic UTF-8 > I18N .pm files (see RT for more detail). At Adam's suggestion I was > supposed to commit it to a branch, but I mistakenly committed it to the > CVS trunk. Not a glorious beginning for me... Well, at least the > changes were all well tested. Question: should I leave it, or back it > out and re-commit on the branch I already created? > > I also am making a trivial change to extend the PPI::Element overload to > include '!=' and 'ne' in addition to '==' and 'eq'. This one I will > deliberately post to the trunk (since it's easy and obvious) as soon as > I get an answer for the above question. > > Chris > > -- > > Chris Dolan, Software Developer, www.chrisdolan.net > <http://www.chrisdolan.net> > > Public key: http://www.chrisdolan.net/public.key > > > |
1 message has been excluded from this view by a project administrator.
