-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(parser) complete fix for resuming matches from same index #2678
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(parser) complete fix for resuming matches from same index #2678
Conversation
I'm thinking perhaps we could simplify further in that I'm pretty sure the only RESUME match we care about would HAVE to be at position 0... so if the first matcher came back with non index === 0 match I think we could perhaps just ignore it completely in favor of the second. |
// need to advance one position and revert to full scanning before we | ||
// decide there are truly no more matches at all to be had | ||
if (!match && top.matcher.resumingScanAtSamePosition()) { | ||
advanceOne(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
advanceOne()
is no longer needed.
Otherwise it's all looks good!
Resolves #2649.
Both our prior definitions of resuming were "off the mark"... resuming properly actually means you have to briefly must have two parallel lines of regex search.
index
.index+1
...The prior solution ignored the need for number 2 above... so we resumed matching but ONLY looking for the remaining expressions (however far in the future) so we would miss a lot of other quite valid expressions.
Or another way to think of this might be a "rotating match".
Normally our regex matching is looking for say [A,B or C]:
But after we match B and decide to ignore it we really want to rotate the matches:
[C,A,B(but not the same B)]. So we do this:
Technically that last C is doing a little extra work potentially (in some cases it might match the exact same thing the first matcher does), but it shouldn't be too bad, as this is already an edge case.
An actual Java real-life case:
For simplicity our rules in Java are:
[string, "class" (begin keyword), number]
. Class having the magic "not proceeded by "." internal rule (which means the "class" match here will be ignored)... At that point technically we still need to "complete" the existing matcher and see if it might still matchnumber
... but if we do alone (without considering the full ruleset) then we end up skipping the string"bar"
and only highlighting 23 (the first number we find)...