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

Allow whitespace after rating ranges #195

Merged
merged 2 commits into from
Apr 24, 2014

Conversation

aiannacc
Copy link
Owner

e.g. parse 5000+, in addition to 5000+
Reorder parsing of ranges like 4000-5000 to before 5000-, as otherwise
4000-5000 will now be interpreted as 4000-

e.g. parse 5000+, in addition to 5000+
Reorder parsing of ranges like 4000-5000 to before 5000-, as otherwise
4000-5000 will now be interpreted as 4000-
@aiannacc
Copy link
Owner Author

Anybody want to check this one for me?

@michaeljb
Copy link
Collaborator

I don't understand the whitespace (assuming that's what "whitespare" was supposed to be) change, in the first comment it looks like you just typed "5000+" twice.

Regardless, it looks like parsing the range doesn't work right. In the console, I got this:

> GS.parseProRange('4000-5000')
Object {min: 4000, max: 4000}

@aiannacc
Copy link
Owner Author

This was to fix the issue #193, in which florrat discovered that appending a comma to the rating criteria disables it. That was because I was using a lookahead: (?!\S) at the end of the regex, which prohibits a non-whitespace character from immediately following the expression.

I had thought to prevent parsing unintended range criteria, but on further consideration, such table names are pretty far fetched. Something like "AI's game 2012-2013 was awesome! #vpon" is pretty much never going to happen, while florrat's "florrat 4000+, #vpon" is obviously quite reasonable. People probably also try stuff like "... 4000+#vpon" to save space.

Edit: okay, my lookahead wouldn't even have protected against "2012-2013". It would have to be something even weirder like "Dec2012-Jan2013 were awesome."

And yes, I seem to have broken the parsing logic. Sorry, I'll fix and test more thoroughly before resubmitting.

Matches of the form "2000-3000" were being interpreted first in its
entirety as a bounded range and then again as "2000-".  Now the match
checks are in an if-else chain instead, so the first match ends the
parsing.
@aiannacc
Copy link
Owner Author

Fixed and tested.

@michaeljb
Copy link
Collaborator

I can take a look this evening.

@aiannacc aiannacc changed the title Allow whitespare after rating ranges Allow whitespace after rating ranges Apr 24, 2014
michaeljb added a commit that referenced this pull request Apr 24, 2014
@michaeljb michaeljb merged commit 7bf6b2f into beta Apr 24, 2014
@michaeljb michaeljb deleted the flexible-rating-criteria-parsing branch April 24, 2014 07:18
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

2 participants