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

Adjust parsing to handle a variety of test cases, in particular secondary units #3

Open
wants to merge 17 commits into
base: master
Choose a base branch
from

Conversation

horgh
Copy link

@horgh horgh commented Jan 24, 2018

Hello!

This makes fairly large changes to the regexes. My primary motivation is to better parse secondary unit types and numbers. I've included a number of new test cases.

I realize the changes are a bit ugly and probably brittle in some ways, but I thought I'd send this anyway. I've run them against a larger set of data and by and large it seems an improvement.

One thought I had while working on this was that it might be nice to localize all of the assignments to %_. As you can see, I did that in a few spots as I found it helpful (and in a few cases, required). Doing this everywhere could make the behaviour more understandable, as right now I believe some of what we parse comes out of branches we backtrack out of.

Thank you!

horgh added 7 commits January 23, 2018 16:43
Also remove an out of place partial sentence
Docs say this about this option "pretend to optimize your code, but
actually introduce bugs"
This makes a number of tweaks to the regexes to improve parsing. I
include test cases for everything now supported. There are still some
formats that are not parsing perfectly, but this makes a number of
improvements.
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