Improve JavaScript language grammars

[ccampbell]

This is related to #63 and integrating the Javascript Next grammars in 82b5883.

The changes in Javascript Next introduced a bunch of regressions from the Javascript grammars in Sublime Text 2 and Sublime Text 3 stable. The aggressive green function calls were just one of them.

This Pull Request attempts to fix some of the things that were broken. I'm aware there are differences between language grammars and themes, but in this case I think these things should work without needing to modify existing themes.

BEFORE

AFTER

@jskinner @Benvie @babel I don't know the proper forum for these changes cause right now there are 3 different people independently maintaining different versions of Javascript Next language grammars. It would probably make sense to standardize one version.

[zertosh]

@ccampbell, I maintain babel-sublime, which is an early fork of JavaScriptNext with JSX support. I rewrote matching class. I think it'll help you here.

[wbond]

@ccampbell Thanks for your work on this.

Could you add some tests? The Testing section on http://www.sublimetext.com/docs/3/syntax.html has some info, and there are tests for a number of syntaxes in this repo now. For instance:

• https://github.com/sublimehq/Packages/blob/master/D/syntax_test_d.d
• https://github.com/sublimehq/Packages/blob/master/Go/syntax_test_go.go.

[FichteFoll]

Relevant discussion regarding the highlighting of $: https://forum.sublimetext.com/t/change-color-for-in-js/16981

I vote for support.function.jquery.js (even though it's not always used as a function).

[ccampbell]

@wbond I will try to find some time to work on it. I just noticed that the latest dev build was released in the stable channel which I imagine will draw some more attention to these issues.

One issue I just noticed is that if you go to a syntax test file and build using Syntax Tests - All Syntaxes it works fine, but if you pick Syntax Tests it says the current file is not a syntax test file. You can see from the screenshot:

I'm using the dev build 3102

[ccampbell]

I'm also noticing that if I change an existing assertion to be something completely invalid like // <- adsoifjpaoidfj it is still telling me that all the assertions passed so I'm not sure the tests are working correctly or perhaps I'm doing something wrong

[titoBouzout]

Why not use @bathos syntax that was written from scratch with the new model and includes all the features from the language. Instead of patching this...

[wbond]

@ccampbell It may be an issue that you are trying to test syntaxes that aren't installed/symlinked into Packages/. The tests use a bunch of the core Sublime Text functions to load them up and into the lexer. Thus, it uses the standard package loading code. I wrote some code to handle situations where a file was loaded from the non-symlinked Packages/ path, however, that may be allowing you to run a file with the same name that exists in the shipped packages, but located in a different place on disk.

In the next dev build I'll try to add a sanity check to ensure the files have the same contents, and alert the user otherwise.

[wbond]

@titoBouzout We definitely aren't opposed to improving syntaxes wholesale, it just requires a bit more testing. I spent some time on @FichteFoll's new YAML syntax, but we want to try and make sure performance doesn't degrade too much with all of the new features/correctness.

I haven't looked at your syntax definition @bathos, but if you would be interested in contributing it towards the shipped packages, I'd be happy to discuss further.

@ccampbell You've obviously put work into improving the existing syntax, and the reason this PR/thread exists. Do you have any thoughts on the various community JS syntaxes?

I think the only balance we have to strike, in general, is making sure we don't break things for users who aren't using the latest version of a given language, and that we don't significantly slow down parsing of larger files. I think with some collaboration, these should all be achievable.

[ccampbell]

@wbond I have the javascript package symlinked into ~/Library/Application Support/Sublime Text 3/Packages, but none of the others. Should I be doing something differently?

@titoBouzout I just installed and checked the Ecmascript syntax and it has a lot of the same issues as the existing syntax.

I think there is a misunderstanding amongst developers about the differences between grammars/scopes and themes. You should not need to install a custom theme in order for your code to be highlighted correctly assuming you the grammar is using the correct scopes. That is the why I created this pull request to begin with.

It even says in the README

I’ve used existing tmLanguage conventions, plus JSNext and Babel Sublime, as guides for scope naming. Nonetheless there’s a fair amount of divergence. Some of this is just the consequence of disambiguating previously conflated elements.

In a few cases, the original Sublime JS tmLanguage had errors, and Babel and JSNext preserved them. I chose to correct these at the risk of decreasing compatibility with existing themes. For example, ‘with’ is not an operator. But there aren’t many of these and they are usually minor things that won’t affect most themes.

I don't disagree with the sentiment, but can you imagine if you had to install a custom theme for every language that you write in? It should be possible to fix the original bugs without breaking compatibility with existing themes. If I want to use the Monokai theme that ships with Sublime Text then I should be able to, and it should correctly highlight all functions/methods/classes and populate the goto menu based on the scopes defined in the grammar. As far as I know, Javascript is currently the only language that does not do this correctly out of the box.

I do think the Ecmascript package is pretty close to being compatible. I think it would require some of the same fixes included with this pull request, but I opted to make the changes here since this is the official repo for the Sublime Text packages.

Regardless of which path the Sublime Text folks want to go in, I think it will be useful to get some unit tests in place here so then we can focus on improving the existing grammars and adding new javascript functionality without breaking existing themes.

[bathos]

@ccampbell @wbond, hey-o. I really didn't design ES Sublime with standard inclusion in mind at all, quite true. While I did include a lot of what I'd called 'interoperability scopes' to allow targeting in different ways, it's definitely not 1:1 with the original JS definition and I wouldn't want it to be. However I'd be open to forking it and developing a more 'generic' version that's more similar to the old JS syntax if this is really something people want. I suspect Babel Sublime is the better candidate for becoming the new default, however.

[titoBouzout]

I don't see a clear path and there are at least 3 definitions. I dislike to see the "duplicated" effort, and split of goodness, I mean in a personal note. (you can do as you wish if you are happy and enjoy). Maybe as this language has lot of attention it can lead the path, even if that means to slightly break themes or introduce something like scope maps, or modifying chunks progressively in current themes. I know with my thinking I may underestimate the amount of work required. I'm sorry for that. It is true Im not very familiar with grammar and themes, but I do see things breaking here and there, almost no language works out of the box for me.

[wbond]

@titoBouzout The best way to help when you see things not working in a language is report it with an example of what is wrong and how it should be improved. Small fixes are pretty quick to implement, review and test. Bigger changes require more testing and concern over backwards compatibility breaks.

Honestly, PHP and JS are probably the syntaxes that could use the most love right now, to my knowledge. We have some PRs open for them, it is just down to creating tests and reviewing things before we merge them in. They will take some work, but we will get there, one step at a time.

[titoBouzout]

Thanks wbond will do

[wbond]

@ccampbell Since you were trying to run the tests for Go, that would need to be symlinked also. I should write a shell script for Windows and OS X that allows for easily symlinking/joining all of the package dirs, and undoing it later.

[wbond]

Just a heads up, due to the volume of concern with JS syntax scope changes in 3103, I am going to be reviewing this PR, adding tests and resolving the issue with objects keys and :.

[ccampbell]

@wbond sounds good! Sorry I haven't gotten around to adding tests. I'm happy to help out I just haven't had a chance to work on it (I may have some time this weekend). In terms of the actual changes here I think how to treat $ was the biggest unresolved issue. I noticed that came up in other discussions as well, but I'm not sure if there was a clear agreement on a solution.

[bathos]

@ccampbell I'd second support.function. Support is for "things provided by a framework or library," according to the tmLanguage docs, and it's fairly common for themes to have a color specifically for "support".

[wbond]

I rebased these on top of master, added tests and made a number of fixed.

The major item to mention here is that I got rid of the special case for $( and $.. Instead, we now have:

• variable.other.dollar for any variable starting with $
• punctuation.dollar for the $ at the beginning of such a variable
• meta.object-literal.key.dollar for a non-string object key that starts with $
• punctuation.dollar for the $ at the beginning of such a variable
• variable.other.dollar.only for a var that is just $

This should allow color scheme developers to target how they want. I've noticed some want to highlight the $ specifically in the context of a longer var, some want to treat jQuery specially and others want all $ vars to be colored for the sake of identifying "element" vars.

I'm obviously open to discussion if people think things should be tweaked more, but let's open a new issue for that.