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

[Javascript] Syntax highlighting changes #141

Closed
austenpayan opened this issue Feb 9, 2016 · 43 comments
Closed

[Javascript] Syntax highlighting changes #141

austenpayan opened this issue Feb 9, 2016 · 43 comments

Comments

@austenpayan
Copy link

  1. dot/period is now red instead of white
  2. object literal property names are yellow like strings instead of previous white
  3. colons in object literals are purple instead of previous white
  4. $ (dollar sign) is white instead of previous red
  5. = (equal sign) used when assigning a function to a variable is white instead of previous red
@wbond
Copy link
Member

wbond commented Feb 9, 2016

Please provide some examples so we can write some tests to ensure that scopes are being properly assigned.

Are you certain these issues are Monokai specific?

@subhaze
Copy link
Contributor

subhaze commented Feb 9, 2016

"$ (dollar sign) is white instead of previous red"

Dollar sign should be white, it has no meaning (on its own or in a variable declaration) in JS what so ever.

[Edited to be less general about statement above]

@austenpayan
Copy link
Author

screen shot 2016-02-09 at 12 01 38 pm

var data = {};

data.name = "Austen";

var literal = {
    name : "Austen",
    location : "Seattle"
}


var randomFunction = function() {
    console.log('Equal sign is white');
}


$scope.randomAngularControllerFunction = function() {
    console.log("Dollar sign is white")
}

@austenpayan
Copy link
Author

@wbond I have only used Monokai so I can't speak for other color themes, sorry!

@austenpayan
Copy link
Author

@subhaze perhaps. That one was the least jarring change. I wouldn't be opposed to keeping it white since it indeed has no meaning in vanilla JS

@wbond
Copy link
Member

wbond commented Feb 9, 2016

While the colors are different than before, I am not sure there is really any error here.

From looking at the scopes, everything seems to be registered properly.

Here is a similar PHP file, and it seems to use the same colors for the same elements. The one exception seems to be the : is purple in JS, whereas => is red in PHP.

screen shot 2016-02-09 at 3 11 01 pm

@elzii
Copy link

elzii commented Feb 9, 2016

Assuming this stemmed from my original post on the forums, (thanks @austenpayan - covered it all + more). The main one for me was nested objects being colored as strings. See the top $el : { /* ... */ } in this screenshot:

screenshot

Having $ return as red would be nice too, was helpful for discerning jQuery element variables from normal ones.

@austenpayan
Copy link
Author

@elzii I agree, this is the most important ^

@elzii
Copy link

elzii commented Feb 9, 2016

@subhaze

Dollar sign should be white, it has no meaning in JS what so ever.

while that may be true, it was quite nice when working with jQuery objects frequently. consider the following block of code:

screenshot

provides a nice visual indicator of what you're working with

@wbond
Copy link
Member

wbond commented Feb 9, 2016

@elzii The yellow you are seeing in the object keys isn't an error, but is the color the syntax uses for unquoted strings. Notice how $( and ) are white. I suppose you could argue that they aren't unquoted strings. I'm not an expert on the JS grammar and the names used for various parts.

@elzii
Copy link

elzii commented Feb 9, 2016

@wbond Ah, gotcha. In my post on the forum, perhaps I should have been more clear, I don't mean to suggest any of it is a bug. More that the changes are jarring for a significant amount of users and a quick consensus around the office was a preference for the older syntax. If it's something that can be resolved with a custom package, I'll pursue that route, just aimed at bringing them to the attention in case it was something overlooked. Thanks!

@wbond
Copy link
Member

wbond commented Feb 9, 2016

Well, these changes fixed errors in the scopes. From what I can tell, the old syntax included lots of incorrect information. That isn't to say some of the new scopes may be incorrect also.

The scopes are used by all sorts of things in Sublime Text, such as indexing, controlling what is spell-checked, the goto symbol quick panel, etc, not just the color scheme.

I'm not disagreeing with you that the old color highlighting may look more pleasant. However, I believe the way forward is to create color schemes that make a pleasant blend of colors for what users want. Unfortunately this isn't the simplest thing since different languages use different mixes of various symbols.

Just as an example, here is the new JS scopes and Go used with Base 16 eighties:

screen shot 2016-02-09 at 3 49 31 pm

screen shot 2016-02-09 at 3 49 56 pm

From looking at the scopes in the JS, I think that constant.other.object.key.js is possibly incorrect. It happens to add the orange to the :. As I mentioned in my previous comment, the keys are highlighted with string.unquoted.label. I'm not sure if this is semantically correct. I do know that when spell checking is turned on, strings are checked for spelling errors.

With all of that said, rolling back to an old release of Sublime Text that has plenty of known bugs that have been fixed would be unfortunate.

@wbond wbond changed the title [Javascript] Syntax highlighting changes (Monokai specific) [Javascript] Syntax highlighting changes Feb 9, 2016
@eridal
Copy link

eridal commented Feb 9, 2016

Adding info about Sunburst

problem

Here's the example code used:

var foo = {};

foo.key = bar;

var literal = {
  foo: 'foo',
  bar: 'bar',
};

var msg = function () {
  var msg = 'the text';
  console.log(msg);
}

data.baz = function () {
  var obj = {
    some: some('foo')
  };
  console.log(obj.some())
};

@subhaze
Copy link
Contributor

subhaze commented Feb 9, 2016

For comments on preferring the broken behavior of treating the $ as a different scope, I'd say creating a jQuery syntax extending the current JS syntax would be the way to go.

I was so happy to see this fixed in the core package and not having to rely on third party packages to fix it.

@radium-v
Copy link

radium-v commented Feb 9, 2016

I didn't want to chime in, but felt it would be useful after checking out JavascriptNext (which the new JS syntax highlighting is based on). I discovered a list of node-specific keywords which are slightly annoying while working in vanilla JS. I attached a screenshot where the variable buffer is inexplicably white (theme is base16-eighties.dark).

screen shot 2016-02-09 at 4 25 38 pm

@darkobits
Copy link

JavaScript syntax highlighting with base16-ocean-dark is totally hosed as well. Almost everything in this screenshot that is colored red was previously off-white (#BAC5C8), and unquoted keys in object literals are now green (the same color used for string literals).

screen shot 2016-02-09 at 1 48 39 pm

@Zerrien
Copy link

Zerrien commented Feb 10, 2016

Color Scheme: Twilight
Putting my example in here:
comparison3

I didn't look too much into what is causing them, but:
All variables/core objects are highlighted now. (Expected: no highlight)
this keyword is no longer highlighted separate. (Expected: special highlight)
function call names are highlighted the same as variables. (Expected: no highlight)
console and Error were changed to reserved color. (Well, this is okay, same color as document)
body is no longer... a special reserved word? (Expected: red)
non-function attribute assignment is red. (Expected: white)
non-function attribute name is green instead of white. (This is okay, I think.)
default with a : is considered an object declaration? (expected: default is case-colored, colon is white? Maybe assignment color (red?))

I'm sure there are others. I'm okay with change, but some changes are weird to me.

Thanks for the consideration. Code pasted below:

function a() {
}
console.log("??");
this.that.b.k(function() {
})
b.that = 0;
this.that.b = function() {
}
this.that();
functionDeclation = function() {
}
this.functionDec = function() {
}
function functionD() {
}
if(testA || testB && testC == D) {
}
function a() {
    this.that = 0;
    a.that = 1
}
document
document.body
document.body.appendChild()
console.warn()
Error
_elem.warn()
_elem.deep.warn();
switch(a) {
    if(a) {
        case 1:
            break;
        default:
            break;
    }
}
obj = {
    a: _elem,
    "a": c,
    "A": function() {
    },
    A: function() {
    }
}

@tunnckoCore
Copy link

I think it would be handled by theme developer, because the new core engine. I don't think it's something that Sublime should fix. We should pay attention and wait for theme authors to update.


I also don't like that object properties are same color as strings, but..
Also i've just notice that function execution that not pass parameters colors the parenthesis

screenshot from 2016-02-10 09-43-03

Notice that done() have colored parenthesis, when not passed params, otherwise is as expected.

@ahwayakchih
Copy link

Hi,

As a quick fix, I've modified local copy of Monokai Extended (Monokai Extended.tmTheme inside the sublime-package archive) by adding:

        <dict>
            <key>name</key>
            <string>JS: Object keys</string>
            <key>scope</key>
            <string>punctuation.separator.key-value.js, string.unquoted.label.js</string>
            <key>settings</key>
            <dict>
                <key>fontStyle</key>
                <string></string>
                <key>foreground</key>
                <string>#ffffff</string>
            </dict>
        </dict>

Hope that helps.

@ahwayakchih
Copy link

Just noticed that object key with function as value gets different color:

screenshot from 2016-02-10 16 08 12

var obj = {
    one: 1,
    two: () => 2,
    three: function returnThree () {
        return 3;
    },
    four: "4"
}

It probably should stay white, same as rest of key names.

@wbond
Copy link
Member

wbond commented Feb 10, 2016

@ahwayakchih I believe that has always been that way. The reason is that the scope for functions is entity.name.function, which is used to populate the Goto Anything menu. That scope is colored green.

Personally, I don't think it would be an improvement to make all functions white.

@timothyshaw
Copy link

I'm also wishing I had the old JS highlighting. I understand that there are improvements to the highlighting, which is much appreciated, but not having any highlighting for jQuery makes the code much harder to read.

Are there any current packages for highlighting jQuery syntax, including dollar signs? Could there be an optional jQuery highlighting baked into ST that could be turned on by the user? Or this just needs to be solved by 3rd party packages?

@ahwayakchih
Copy link

@wbond I'm just not sure it is the best solution and here's why: when passing a function through a variable, it gets color of "regular" key name:

screenshot from 2016-02-10 17 29 17

var f = function f () {};
var obj = {
    test: f
};

obj.test is a function, but it is not colored as such. I'm guessing that's hard to implement without more info about rest of the source code.

I do like that it gets color of a function in case of:

somePreviouslyDeclaredObject.test = function test () {}

I just think that it looks a bit "messy" when different colors are used for keys in object literal, i.e., when some of the keys are white and others are green (or whatever colors are used by selected theme).

Another problem is with = and function expression:

screenshot from 2016-02-10 17 23 53

var t2 = function test () {};
var t3 = () => 1;

Please notice how = it is getting the same color as function and variable names when using function f () syntax, but stays white when using new () => ("arrow function") syntax. I believe it should stay white in both cases.

@wbond
Copy link
Member

wbond commented Feb 10, 2016

@ahwayakchih I'm not sure what solution of mine you are referring to isn't the best. Above I was just explaining why object keys assigned functions are green.

As I mentioned in a previous comment, scopes generated by syntaxes are used by all different parts of Sublime Text. The most important thing is that they are as correct as possible since it is not just color that depends on it.

In some situations syntax files won't be able to apply "correct" scope since the code would need to be executed to determine variable values changes, etc. However, that doesn't mean we shouldn't apply correct scopes wherever possible.

In terms of a solution, it sounds like the best course of action would be to create (or find) a color scheme that uses the colors you prefer for given language tokens.

@ahwayakchih
Copy link

@wbond sorry for misunderstanding, i didn't mean "your solution", only "previous and current implementation". Is there a way to make a color scheme to use different color for entity.name.function, depending on where it shows up, e.g., as object literal key vs function declaration or expression?

The problem with = character getting different colors is a separate thing of course - not about object keys.

@wbond
Copy link
Member

wbond commented Feb 10, 2016

@ahwayakchih I haven't looked at the scopes to see if there is a meta scope for an object literal. If so, you could target meta… entity.name.function.

I'll try to get the = fix in, however this issue is so long/multifacted I may miss it when I get to implementation. If you want to help out, you could post a new issue about that specific example.

@ahwayakchih
Copy link

@wbond i could not find object literal meta. I tried meta.function.json.js entity.name.function.js (that is the only scope i could find for function after colon in https://github.com/sublimehq/Packages/blob/master/JavaScript/JavaScript.sublime-syntax) but that did not work. Anyway, sorry for going offtopic with this.

Thanks, I'll post new issue regarding = problem posted as #153.

@jonschlinkert
Copy link

fwiw, I was able to easily update sublime-monokai-extended to revert colors back to what they were before. It seems (as has been mentioned already in this thread) that the changes were a result of scopes actually being fixed and/or added. This is a good thing! thanks!

@RaphaelDDL
Copy link

I still couldn't understand why object properties are coloured as strings. I mean:

var literal = {
    name : "Austen",
    location : "Seattle"
}

Why name and location are coloured as yellow instead of white. That's the only thing that concerns me; the other changes are okay (even though I liked $ being red, it doesn't have to be as it has no meaning in vanilla).

~~

Also, quick off-topic: @wbond grats for officially being part of Sublime team now. Kudos.

@wbond
Copy link
Member

wbond commented Feb 11, 2016

Status update:

After reviewing lots of feedback and speaking with @jskinner, we are going to change object keys to NOT be scoped as unquoted strings. This feels like the correct change since object keys are really more like variable names than strings. We will also be fixing the : so it isn't scoped as a constant.other.

In the process I am hoping to add a meta scope for object keys, so color scheme authors can handle keys that are function definitions how they want, as suggested by @ahsankhatri.

Finally, I am going to be working on adding tests for #96 and getting that merged in to improve the JS syntax overall.

@ahwayakchih
Copy link

@wbond thanks! although meta scope for object literal keys could be useful, i found a way to color them without additional scope. What would be maybe even more useful, is some additional scope for actual function name (as in: function name () {}) as i tried to explain at #154 (comment) (second half of the post, sorry for it being so long).

@wbond
Copy link
Member

wbond commented Feb 12, 2016

I've make tweaks to object key scopes in 10073d7. See the commit for the details.

In addition, I merged in a bunch of improvements to JS from @ccampbell, added tests and fixed some other edge cases I encountered.

These tweaks will be part of the next dev build.

@wbond wbond closed this as completed Feb 12, 2016
@tt0mmy
Copy link

tt0mmy commented Feb 12, 2016

Dollar sign should be white, it has no meaning in JS what so ever.

what about for string interpolation in es2015?

var editor = "sublime";
`I use ${editor} as my editor of choice`

@wbond
Copy link
Member

wbond commented Feb 12, 2016

@tantommy The commit I linked to includes the current changes that were made, which should make it possible for users to create color schemes that highlight $ as they see fit. By default, $ is not specially highlighted.

Currently the syntax does not support that feature you mentioned, although you could open an issue to add it, if you like.

@subhaze
Copy link
Contributor

subhaze commented Feb 12, 2016

Edited my statement above about $ to be a bit more specific.

@wbond it has support via the old name/spec "literal-quasi" but is now named template literals. Giving the syntax file a glance it seems to support it pretty well aside from using the older name 'quasi'.

@FichteFoll
Copy link
Collaborator

@subhaze indeed, except that it uses the entity scope. That should be something else combined with something "embedded" instead, but I'll open that as a different issue.

@nightpool
Copy link

question: (and forgive me if this was already fixed, I'm not on the dev builds as far as I know) why does this syntax highlighting happen?
image
Leaving aside the way that 90% of my code is now red by default, why is "extensions" treated in a different way then "postarchive"? They're both objects, right?

@jonschlinkert
Copy link

@wbond just wanted to say how much I appreciate the hard work you've done on this. makes my whole dev experience nicer. thank you!

@wbond
Copy link
Member

wbond commented Feb 26, 2016

@nightpool The current version of JavaScript has been completely overhauled since build 3103, so highlighting will definitely look different, and many of the scopes have been corrected or improved. I'm not sure what color scheme you are using, but here is an example of that code using Base 16 eighties dark:

screen shot 2016-02-25 at 10 47 20 pm

XKit is scoped as a support.class, whereas extensions and postarchive are meta.property.object and load_posts() is variable.function.

@azizarslan
Copy link

Resolution: Sublime Text downgrade 3083. (temporarily)

OS X download address: https://download.sublimetext.com/Sublime%20Text%20Build%203083.dmg

@wbond
Copy link
Member

wbond commented Mar 1, 2016

@azizarslan No, don't downgrade to 3083. Then JS will be very broken in many ways, and there are a quite a number of known bugs in 3083. There is much faster, better JS support in build 3107: https://www.sublimetext.com/3dev. Highlights include support for almost all ES6 features, plus about a 400% increase in performance. It has been rewritten almost from scratch, and is continually improving.

@FichteFoll
Copy link
Collaborator

3107 is a dev build and not available for unlicenced copies though.

@roblav96
Copy link

Add this to your prefered sublime color scheme file:

<dict>
    <key>name</key>
    <string>STACK DAT DOUGH</string>
    <key>scope</key>
    <string>punctuation.dollar</string>
    <key>settings</key>
    <dict>
        <key>foreground</key>
        <string>#F92672</string>
    </dict>
</dict>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests