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

Fix for #207, Some escaped '$' characters are not unescaped #222

Merged
merged 4 commits into from
Dec 2, 2015
Merged

Fix for #207, Some escaped '$' characters are not unescaped #222

merged 4 commits into from
Dec 2, 2015

Conversation

gautam-ndk
Copy link
Contributor

The issue was that the regex was not capturing $ which are not followed by alphanumericals. So, cases like "BAR$" and "$$" were not handled.
Made changes to take care of these.

@gautam-ndk
Copy link
Contributor Author

I hope the changes fix the issue.
But, I dont know why the rubocup fails for the code I dint make any changes to. Any idea of that ?

@bkeepers
Copy link
Owner

Thanks for the pull request, @dhunnapotha! It looks like there are still a couple build failures. Would you mind fixing them up and pinging me when they're ready? Thanks!

@gautam-ndk
Copy link
Contributor Author

Sure. Checking them out.

Removed that regex and put that code into ruby logic
@gautam-ndk
Copy link
Contributor Author

@bkeepers The test case failures are fixed. Am assuming I dont need to take care of the rubocop gemspec offenses. Let me know if it is not the case.

@bkeepers
Copy link
Owner

bkeepers commented Dec 2, 2015

Am assuming I dont need to take care of the rubocop gemspec offenses. Let me know if it is not the case.

Those are fixed in master. Thanks for the PR!

bkeepers added a commit that referenced this pull request Dec 2, 2015
…r_v2

Fix for #207, Some escaped '$' characters are not unescaped
@bkeepers bkeepers merged commit 80e8a2d into bkeepers:master Dec 2, 2015
@gautam-ndk
Copy link
Contributor Author

Cool. Thanks!

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.

2 participants