Skip to content

Commit

Permalink
Cleanup more (?:) from patterns
Browse files Browse the repository at this point in the history
Following up on slevithan#164, this
change prevents a `(?:)` from being inserted in the following places:

* At the beginning of a non-capturing group (the end is already handled)
* Before or after a `|`
* At the beginning or the end of the pattern

This solution isn't as complete as the one suggested in
slevithan#179, but it's a decent
stopgap.
  • Loading branch information
josephfrazier committed May 1, 2017
1 parent a661cd3 commit 3827487
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/xregexp.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,14 @@ function getContextualTokenSeparator(match, scope, flags) {
// No need to separate tokens if at the beginning or end of a group
match.input[match.index - 1] === '(' ||
match.input[match.index + match[0].length] === ')' ||
// No need to separate tokens if at the beginning of a non-capturing group
match.input.slice(match.index - 3, 3) === '(?:' ||
// No need to separate tokens if before or after a `|`
match.input[match.index - 1] === '|' ||
match.input[match.index + match[0].length] === '|' ||
// No need to separate tokens if at the beginning or end of the pattern
match.input[match.index - 1] === undefined ||
match.input[match.index + match[0].length] === undefined ||
// Avoid separating tokens when the following token is a quantifier
isQuantifierNext(match.input, match.index + match[0].length, flags)
) {
Expand Down
15 changes: 15 additions & 0 deletions tests/spec/s-xregexp.js
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,21 @@ describe('XRegExp()', function() {
expect(XRegExp('(#\n.#\n)', 'x').source).toBe('(.)');
});

it('should not add atom separator (?:) at the beginning or end of non-capturing groups in simple cases', function() {
expect(XRegExp('(?: . )', 'x').source).toBe('(?:.)');
expect(XRegExp('(?:#\n.#\n)', 'x').source).toBe('(?:.)');
});

it('should not add atom separator (?:) at the beginning or end of the pattern in simple cases', function() {
expect(XRegExp(' ( . ) ', 'x').source).toBe('(.)');
expect(XRegExp(' (#\n.#\n) ', 'x').source).toBe('(.)');
});

it('should not add atom separator (?:) around | in simple cases', function() {
expect(XRegExp('( a | b )', 'x').source).toBe('(a|b)');
expect(XRegExp('(#\na#\n|#\nb#\n)', 'x').source).toBe('(a|b)');
});

it('should allow whitespace between ( and ? for special groups', function() {
expect(XRegExp('( ?:)', 'x').source).toBe('(?:)');
expect(XRegExp('( ?=)', 'x').source).toBe('(?=)');
Expand Down

0 comments on commit 3827487

Please sign in to comment.