Add $<n> and $<name> as alternative backreference syntax in replacement text #181
Conversation
src/xregexp.js
Outdated
| var replacementTokens = [ | ||
| /\$(?:<([\w$]+)>)/g, | ||
| /\$(?:{([\w$]+)}|(\d\d?|[\s\S]))/g | ||
| ]; |
There was a problem hiding this comment.
I'd rather do this in one pass (rather than 2 or 3) for speed. But that aside, as written it doesn't make sense to do this in two passes since you're doing the same handling for $2 in the replacer function even when it isn't relevant.
I think what you want here is to make replacementToken /\$(?:{([\w$]+)}|<([\w$]+)>|(\d\d?|[\s\S]))/g, then change replacer ($0, $1, $2) to replacer($0, $1, $2, $3) or something like replacer($0, bracketed, angled, dollarToken), then put bracketed = bracketed || angled at the top of the replacer function.
There was a problem hiding this comment.
Ah, that's exactly right! Thanks for the suggestion, I've pushed some updates implementing it.
src/xregexp.js
Outdated
| function replacer ($0, $1, $2, $3) { | ||
| $1 = $1 || $2 | ||
| function replacer ($0, bracketed, angled, dollarToken) { | ||
| bracketed = bracketed || angled |
There was a problem hiding this comment.
Please add a trailing semicolon
src/xregexp.js
Outdated
| @@ -1157,6 +1157,8 @@ XRegExp.matchChain = function(str, chain) { | |||
| * backreference n/nn. | |||
| * - ${n} - Where n is a name or any number of digits that reference an existent capturing | |||
| * group, inserts backreference n. | |||
| * - $<n> - Where n is a name or any number of digits that reference an existent capturing | |||
There was a problem hiding this comment.
See above where $&, $0 are on the same line. Would you mind doing the same here, and grouping this with ${n}?
|
|
||
| // Backreference to a nonparticipating capturing group | ||
| expect('test'.replace(XRegExp('t|(?<test>t)', 'g'), ':${test}:')).toBe('::es::'); | ||
| expect('test'.replace(XRegExp('t|(?<test>t)', 'g'), ':$<test>:')).toBe('::es::'); | ||
| }); | ||
|
|
There was a problem hiding this comment.
Can you add a test for mixing ${n} and $<n> syntax in the same replacement string?
|
This is looking good. In addition to addressing the few minor comments I just added, can you rebase this and fix the conflict?
|
… replacement text
…lacement text I had some trouble using a single regex for the replacement, so I replaced the $<name> syntax first in a separate pass.
josephfrazier
force-pushed
the
backreference-angle-bracket
branch
from
91c8451
to
4418d10
Compare
Fixes #177
I had some trouble using a single regex for the replacement, so I replaced
the $ syntax first in a separate pass.