Skip to content

Commit

Permalink
[JavaScript] Fix occassional highlighting of division operators as re…
Browse files Browse the repository at this point in the history 
…gexp
  • Loading branch information
@wbond
wbond committed Feb 24, 2016
1 parent f06d2a5 commit eda3e39
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 5 deletions.
28 changes: 24 additions & 4 deletions JavaScript/JavaScript.sublime-syntax
View file
Expand Up @@ -246,9 +246,12 @@ contexts:

expressions:
- include: comments
- include: literal-regexp
- include: expressions-no-regexp

expressions-no-regexp:
- include: literal-string
- include: literal-string-template
- include: literal-regexp
- include: constructor
- include: literal-operators
- include: class
Expand Down Expand Up @@ -361,7 +364,16 @@ contexts:
scope: constant.character.escape.js

literal-regexp:
- match: '/(?=.+/[gimy]*)'
# Lookahead and prevent incorrect matches by finding a / following an
# identifier or number. Not perfect, but stops most incorrect matches
- match: '(?=({{identifier}}|0[xob]\d+|\d+|\d*\.\d*(e[-+]?\d+)?)\s*/[^/])'
push:
- include: comments
- match: '/'
scope: keyword.operator.arithmetic.js
pop: true
- include: expressions-no-regexp
- match: '/(?=.+/[gimy]*(?!\s*[a-zA-Z0-9_$]))'
scope: punctuation.definition.string.begin.js
push:
- meta_scope: string.regexp.js
Expand All @@ -376,6 +388,14 @@ contexts:
pop: true
push: scope:source.regexp.js

# Consume a division operator to prevent incorrect highlighting as a regexp
prevent-incorrect-regexp:
- include: comments
- match: '/'
scope: keyword.operator.arithmetic.js
- match: '(?=[\S\n])'
pop: true

constructor:
- match: '\b(new)\b\s+(?=({{identifier}}|\())'
scope: meta.instance.constructor.js
Expand Down Expand Up @@ -814,7 +834,7 @@ contexts:
- meta_scope: meta.group.braces.square.js
- match: '\]'
scope: meta.brace.square.js
pop: true
set: prevent-incorrect-regexp
- include: expressions

literal-number:
Expand Down Expand Up @@ -906,7 +926,7 @@ contexts:
function-call-params:
- match: '\)'
scope: meta.brace.round.js
pop: true
set: prevent-incorrect-regexp
- match: '\('
scope: meta.brace.round.js
push:
Expand Down
36 changes: 35 additions & 1 deletion JavaScript/syntax_test_js.js
View file
Expand Up @@ -210,7 +210,7 @@ var qux = 100;
if (100.0 > qux) {
// ^^^^^^^^^^^^^^^ meta.conditional
a;
// ^ meta.conditional meta.block
// ^ meta.conditional meta.block
}
// <- meta.conditional meta.block

Expand Down Expand Up @@ -440,3 +440,37 @@ new Date().getTime()
// ^ meta.group.braces.square
'test3': "asdf"
}

width/2 + lineStart * Math.sin(i * 30 * π/180)
// ^ keyword.operator.arithmetic
// ^ keyword.operator.arithmetic
// ^^^^^^^^^^^^^^^^^^^ meta.function-call.method

var reg = /a+/gimy.exec('aabb')
// ^^^^^^^^ string.regexp
// ^ punctuation.accessor

'aabbcc'.replace(/b+/, 'd')
// ^^^^ string.regexp

/a+/
// <- string.regexp

var g = 50

g / 20 + 30 /g
//^ keyword.operator.arithmetic
// ^ keyword.operator.arithmetic

var h = foo() / 20 + 30 /g
// ^ keyword.operator.arithmetic
// ^ keyword.operator.arithmetic

foo['bar']/ 20 + 30 /g
// ^ keyword.operator.arithmetic
// ^ keyword.operator.arithmetic

var result = 200 / 400 + 500 /
// ^ keyword.operator.arithmetic
// ^ keyword.operator.arithmetic
100;

5 comments on commit eda3e39

@subhaze
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this commit seems to have some regressions in it. I noticed the above issue on https://raw.githubusercontent.com/greensock/GreenSock-JS/master/src/uncompressed/TimelineMax.js

after pulling a lot of highlighting was lost

@wbond
Copy link
Member Author

@wbond wbond commented on eda3e39 Feb 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll check it out in the morning and add some more tests.

@wbond
Copy link
Member Author

@wbond wbond commented on eda3e39 Feb 24, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0de8659 will likely fix the issue you are seeing

@subhaze
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like! Still seeing some issues but about to finish up watching CONCACAF game and head to bed.

I'll try to look tomorrow and add some tests/report findings.

I was writing this one out

list[list.length] = 'test';
//                ^ keyword.operator.assignment.js
//                  ^ punctuation.definition.string.begin.js
//                    ^ string.quoted.single.js
//                       ^ punctuation.definition.string.end.js
//                        ^ punctuation.terminator.statement.js

but the mentioned commit resolved it... so not sure if it'd be overkill to add.

Anyhow, great work man, love seeing all this activity on the core packages!

@subhaze
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Didn't have time to look into the 'issues' i stated above yesterday, but looks like 3107 release resolved them!

Please sign in to comment.