Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

issue 27: parse x3 as x operator plus a number #33

Merged
merged 1 commit into from
Feb 27, 2014

Conversation

moregan
Copy link
Collaborator

@moregan moregan commented Feb 4, 2014

Fix for Issue #27, plus tests.

This change includes the elimination of existing code that would separate the 'x' operator from a number immediately following, but only when preceded by a small handful of token types, e.g: "a"x3 This change eliminates paying attention to the preceding token.

@moregan
Copy link
Collaborator Author

moregan commented Feb 4, 2014

I put tests in t/ppi_token_operator.t, rather than in t/data/, since I find the former easier to work with. Are new tests still being written for t/data/ ?

@adamkennedy
Copy link
Collaborator

The t/data tests should be parsing tests that do not or can not specify
where in the implementation they are being implemented.

I would recommend them where possible (or at least put a copy of the
failing parse case in there for future regression testing).

The use of ppi_token_operator.t implies your test is related specifically
to that single class, which these tests are not necesarily going to.

However, I'd rather someone fix it, so doing it your way should be fine.

Just please don't convert the existing tests over, they represent a
hard-earned store of regression tests.

On 3 February 2014 19:07, moregan notifications@github.com wrote:

I put tests in t/ppi_token_operator.t, rather than in t/data/, since I
find the former easier to work with. Are new tests still being written for
t/data/ ?

Reply to this email directly or view it on GitHubhttps://github.com//pull/33#issuecomment-34026723
.

@moregan
Copy link
Collaborator Author

moregan commented Feb 26, 2014

A/B testing with Perl::Critic against a sampling of CPAN discovered that x=>$x was being parsed as x= > $x on master. Fix with tests added to x-operator-not-recognized branch with 8cb2974

Fix x=>$x being mistakenly parsed as x= > $x.
@wchristian wchristian merged commit 27a6ed9 into Perl-Critic:master Feb 27, 2014
@wchristian
Copy link
Member

Looks good and passes tests, merged. :)

@moregan moregan deleted the x-operator-not-recognized branch November 10, 2014 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants