Perl-Critic / PPI

53 stars 44 forks source link

RFC: PPI::Exception subclasses -- use or toss? #143

Open moregan opened 9 years ago

moregan commented 9 years ago

PPI::Exception::ParserRejection and PPI::Exception::ParserTimeout exist in the source tree, but they are not used by PPI, and grep.cpan.me says they are not used on CPAN. Neither one adds anything to the base PPI::Exception. If there are no immediate plans to use them, it would be better to delete them. Thoughts, @wchristian @adamkennedy ?

There one spot in PPI::Document that thinks ParserTimeout gets thrown, but gets the class name wrong:

        # Pull and store the error from the lexer
        my $errstr;
        if ( _INSTANCE($@, 'PPI::Exception::Timeout') ) {
                $errstr = 'Timed out while parsing document';

Were these classes in use at one time?

wchristian commented 9 years ago

Haven't a thought here right now, but it seems like only PPI::Exception itself is used by outside modules: http://grep.cpan.me/?q=PPI%3A%3AException

adamkennedy commented 9 years ago

I'm relatively ok with tossing the sub-classes if nobody uses them, especially since the performance improvements make timeouts due to pathalogical cases (the only case I really case about) much rarer

moregan commented 9 years ago

Pull request #149 submitted to remove the PPI::Document->new timeout feature, including PPI::Exception::ParserTimeout

moregan commented 7 years ago

PPI::Exception::ParserTimeout was removed in PPI 1.224, PPI::Exception::ParserRejection was not.