Improvement to greedy-flag #967
Conversation
This patch expands the idea of the greedy flag and significantly improves it, by matching against the whole text instead of just the next couple of tokens. This does not only improve the results, but it should also slightly improve performance.
|
The issue #963 inspired me to look into improving the greedy-flag feature and I am quite excited about the result. The idea is essentially the same as before, but instead of matching a greedy-pattern against the concatenated string of the next few tokens, it matches against the whole text. With this patch the following code matches correctly: / '1' '2' '3' '4' '5' /
"foo `a` `b` `c` `d` bar" |
|
Sounds awesome! I'll try to get some time to review this later this week. |
|
Thank you @zeitgeist87! |
|
This looks good to me, @zeitgeist87. It seems to works really well, kudos! Merge it whenever you want. |
| @@ -280,9 +280,15 @@ var _ = _self.Prism = { | |||
| lookbehindLength = 0, | |||
| alias = pattern.alias; | |||
|
|
|||
| if (greedy && !pattern.pattern.global) { | |||
| // Without the global flag, lastIndex won't work | |||
| pattern.pattern = RegExp(pattern.pattern.source, pattern.pattern.flags + "g"); | |||
There was a problem hiding this comment.
@zeitgeist87: How recent is the flags property? It looks like it's not supported very well... I just ran into errors running npm test and I think they are due to this.
Maybe we could refactor like so:
var flags = pattern.pattern.toString().match(/[imuy]*$/)[0];
pattern.pattern = RegExp(pattern.pattern.source, flags + "g");What do you think?
There was a problem hiding this comment.
@Golmote Yes you are right it is not supported well at all. Good catch. Your solution is perfect and we should add it.
There was a problem hiding this comment.
@Golmote Shall I add this fix or do you?
There was a problem hiding this comment.
@zeitgeist87 I just did, thanks. ;)
There was a problem hiding this comment.
I would rather we added a separate polyfill for flags instead of refactoring our code not to use it. That way, once support is more broad, we can just ditch the polyfill.
This patch expands the idea of the greedy flag and significantly
improves it, by matching against the whole text instead of just
the next couple of tokens. This does not only improve the
results, but it should also slightly improve performance.