Fix #779 - Delimiters are used without escaping. #782

Merged
merged 2 commits into from Sep 19, 2012

Conversation

Projects
None yet
3 participants
Collaborator

braddunbar commented Sep 19, 2012

Note that this implementation requires exactly one capturing group per delimiter. I think this is a reasonable expectation, but I'm not basing it on any statistics from real world delimiters.

substr is non-standard and could be replaced with slice.

Owner

braddunbar replied Sep 19, 2012

Quite right, thanks @jdalton! Fixed in ed83596.

Contributor

jdalton commented Sep 19, 2012

Note that this implementation requires exactly one capturing group per delimiter. I think this is a reasonable expectation, but I'm not basing it on any statistics from real world delimiters.

It's always required one capturing group because the functions used to process delimiters had the argument signature of match, code.

Collaborator

braddunbar commented Sep 19, 2012

Right, but now it requires exactly one capturing group, no more. Previously, adding a second capturing group would not have caused a problem.

Contributor

jdalton commented Sep 19, 2012

@braddunbar Ahhh... Whew, I seem to have avoided that in my implementation (happy accident)!

@jashkenas jashkenas added a commit that referenced this pull request Sep 19, 2012

@jashkenas jashkenas Merge pull request #782 from braddunbar/delimiters
Fix #779 - Delimiters are used without escaping.
9fc81c9

@jashkenas jashkenas merged commit 9fc81c9 into jashkenas:master Sep 19, 2012

Contributor

jdalton commented Sep 19, 2012

Perf is alright too (down ~30% in Chrome but up a ~20% in Safari). I dig the use of the match offset and leveraging |$.

Collaborator

braddunbar commented Sep 19, 2012

Yea, me too. I didn't even know the offset argument existed until this afternoon. :)

Contributor

jdalton commented Sep 19, 2012

Ya, the spec is the jam. I reference it at least a couple times a day – http://es5.github.com/#x15.5.4.11

Contributor

jdalton commented Sep 19, 2012

@braddunbar Just gotta say, I am still geeking out about your solution. Very very very cool.

Collaborator

braddunbar commented Sep 19, 2012

Thanks! :)

braddunbar deleted the braddunbar:delimiters branch Jan 27, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment