Skip to content
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

Cleanup more (?:) from patterns #196

Merged
merged 9 commits into from
Jan 22, 2018
15 changes: 15 additions & 0 deletions src/xregexp.js
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,21 @@ 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 or lookahead
//
// If `match.index - 3` is negative, the substring will be too short to match, so the test will fail.
// For example: '1234'.substr(-1, 3) === '4'
nativ.test.call(/\(\?[:=!]/, match.input.substr(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.index < 1 ||
match.index + match[0].length >= match.input.length ||

// Avoid separating tokens when the following token is a quantifier
isQuantifierNext(match.input, match.index + match[0].length, flags)
) {
Expand Down
28 changes: 28 additions & 0 deletions tests/spec/s-xregexp.js
Original file line number Diff line number Diff line change
Expand Up @@ -800,6 +800,34 @@ 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('(?:.)');

expect(XRegExp(' (?: . )', 'x').source).toBe('(?:.)');
expect(XRegExp(' (?:#\n.#\n)', 'x').source).toBe('(?:.)');
});

it('should not add atom separator (?:) at the beginning or end of a lookahead 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 a negated lookahead 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