Follow naming conventions

[tomasstankovic]

Fixes #201

[Victorystick]

You've moved var, but forgot let.

[tomasstankovic]

There it is.

[Victorystick]

And just like that, I'm sold. 👍

[Victorystick]

Just realized the scope you've added is storage.type.var.js, but I guess that works just the same for let since its value isn't constant, but variable.

[winstliu]

@simurai do you think this will cause any problems for syntax authors? This change doesn't affect One Light/Dark, but still wanted to check with you anyway :).

[MaximSokolov]

Maybe it would be to make a separate scope for var and let.
name would be 'storage.type.${1}.js' but I am not sure.

this will cause any problems for syntax authors?

.storage {
  &.modifier {
    color: @color1;
  }
  &.type {
    color: @color2;
  }
}

var, let would be @Color2. So, yes. It would affect on some syntax themes. But I don't think that is a problem.

[MaximSokolov]

since its value isn't constant, but variable

Nope. It's because it is var. Sorry, don't have docs with examples yet. I am working on it. TextMate docs are very unclear.

[simurai]

do you think this will cause any problems for syntax authors?

Not really for syntax authors, more for users that got used to a certain color. But from the core themes, it looks like only Atom dark would be affected:

So Atom dark could be adjusted to keep the same color. Or if Atom dark's intention is to show storage.modifier differently than storage.type, then you could argue that it's currently wrong and this change would correct it.

[MaximSokolov]

more for users that got used to a certain color

That's correct. Nothing we can do about it. At least after this PR users can change color for var/let:

.js {
  .storage.type {
    &.var, &.let {
      color: @color;
    }
  }
}

[winstliu]

Ok, I'll merge this then and see how it goes.

[simurai]

Ok, I'll merge this then and see how it goes.

👍 In case we get issues for Atom dark about it, we can change the color back to blue for var/let.

[MaximSokolov]

let should be storage.type.let.js. Unique scope for each language symbol.

In case we get issues for Atom dark about it, we can change the color back to blue for var/let.

Do you want change each theme where colors for storage.type and storage.modifier are different?

[winstliu]

let should be storage.type.let.js

I disagree with this. var and let are fundamentally the same.

[pchaigno]

let should be storage.type.let.js. Unique scope for each language symbol.

While I agree that having more subscopes increases customization possibilities, I think having a unique scope for each symbol is excessive. You should consider the impact it would have on the maintainability of the syntax definition.

[MaximSokolov]

I disagree with this. var and let are fundamentally the same.

If you think these scopes are fundamentally the same, you can suggest new common subscope to atom/atom#8430 like storage.type.{common}.var These keywords have different behavior. Is int and double are fundamentally the same? Maybe. I saw scope primitive in Java. Either a new subscope or
storage.type.let.

You should consider the impact it would have on the maintainability of the syntax definition.

Don't understand. What do you mean by "maintainability of the syntax definition." Is &.var, &.let instead of &.var a big impact?

[MaximSokolov]

When I see {rootScope}.{languageSymbol} I expected it would be match only languageSymbol but not languageSymbol and anythingElse

[winstliu]

@MaximSokolov I would argue that int and double should be under the same scope as well. And regarding maintainability, &.primitive would be much cleaner than &.int, &.double, &.long, &.float, &..., no?

[MaximSokolov]

@50Wliu A added a primitive under ❓ mark. Would do you think about let and var? Should they have a common scope?

[winstliu]

Yeah. Let's keep it at storage.type.var.js for now to minimize changes and then take a look at things again when Atom's nearing 2.0?

[MaximSokolov]

take a look at things again when Atom's nearing 2.0

Why? These scope are optional. It wouldn't broke anything.

[MaximSokolov]

Also It's not about let and var. It's about common approach we should write on docs. I suggest: match sa many optional scopes as you can and if symbols have something in common create a supscope for them.

[winstliu]

It wouldn't broke anything.

It already broke language-html's specs (https://travis-ci.org/atom/atom/jobs/77525653#L243), and as we saw previously, it does (sometimes) change the syntax colors. You can see an effect of that here: #191

[MaximSokolov]

It already broke language-html's specs

Unit tests should test only one module, correct?

You can see an effect of that here: #191

I don't think it because of this PR.

it does (sometimes) change the syntax colors.

Yes. var and let are highlighted correctly now.

[MaximSokolov]

You can see an effect of that here: #191

It's because constructor is entity.name.constructor should be entity.name.function.constructor

That exactly what I am trying to fix. Languages don't follow the convention.

[winstliu]

This PR will not be making it into Atom 1.0.8. The scope changes broke both language-html's specs and Atom core specs (the latter of which appear to be especially difficult to update). In the interest of getting 1.0.8 out as painlessly as possible, Atom's reverted back to the previous version of language-javascript.

@MaximSokolov:
There isn't exactly a "convention" yet - as you mentioned before, the Textmate docs are only guidelines, and that's all the languages have right now. You've proposed a convention, but that hasn't been finalized. We intend to move forwards and I think it would be best to get others' feedback on your proposals, and then allow more people to test and adapt to the changes. Don't forget that many users have edited their styles.less to fit their own needs - changing scopes without warning may cause them to receive the "wrong" color or actually lose highlighting.

/cc @atom/feedback

[MaximSokolov]

The scope changes broke Atom core specs

In that case reverted back is a best solution. Agree.

[MaximSokolov]

There isn't exactly a "convention" yet - as you mentioned before, the Textmate docs are only guidelines, and that's all the languages have right now.

Yes. That's the problem. Most of languages don't follow "guidelines".

Don't forget that many users have edited their styles.less to fit their own needs - changing scopes without warning may cause them to receive the "wrong" color or actually lose highlighting.

Don't forget how many language specific rules must add syntax theme developer to make theme looks the same for general languages. Otherwise syntax theme would be look different for languages don't following "guidelines".

var is highlighted as storage.modifier This is "wrong" color.

It would be much harder to follow "guidelines" if we would wait until Atom 2.0. Would be released a lot of new syntax themes and languages. Most of theme/language developers don't known about grammar "guidelines". Even if they do, TextMate docs quite useless without examples. That means developers would copy code from general languages/themes. Would be a lot of languages/themes don't following "guidelines". Would be a lot of "wrong" colors.

[pchaigno]

@MaximSokolov Do you contribute to any syntax definition?

[MaximSokolov]

@pchaigno:

Do you contribute to any syntax definition?

Do you mean to any language-*? I only fixed a small bug

I saw most scopes of languages from atom repo when I worked on Atom grammar "guidelines" Some languages don't follow "guidelines"

Also I known that syntax themes would look bad without a language specific rules

Are you trying to say that I don't undestand much about languages' grammar? Maybe. If you have arguments against my suggestion it would be great if you will share them

[pchaigno]

Are you trying to say that I don't understand much about languages' grammar?

No, the discussion until now seem to show that you know what you're talking about 👌
I just think that you underestimate the difficulty of maintaining a grammar were each symbol has its own scope.

If you have arguments against my suggestion it would be great if you will share them

I already did ;)

the impact it would have on the maintainability of the syntax definition.

A grammar with a scope per keyword would be really verbose and it's already difficult to debug issues in current state. I'm all for well-defined scopes but I don't think we should sacrifice the syntax definition for the possibility to customize everything.

[simurai]

Would the specs pass if var and let got changed from storage.modifier.js to storage.type.js? So it would change to an existing sub-category, but not create new ones.

It would also match how TextMate currently highlights vars: https://tmtheme-editor.herokuapp.com/#!/editor/theme/Monokai

[winstliu]

No, both language-html and Atom currently test to make sure var is scoped as storage.modifier.js.

[Victorystick]

That's too bad. Why is it much more difficult to update the specs of Atom core?

I agree whole-heatedly that common subscopes and adherence to a common set of guidelines is the only way to make syntax themes work just as well across all languages.

I encouraged similar changes to atom/language-go and kevinsawiki/monokai. Go had a unique set of scopes that no other language used. The syntax theme had accordingly adopted a set of special rules to make Go highlight like other languages. The solution: common subscopes.

[MaximSokolov]

I just think that you underestimate the difficulty of maintaining a grammar were each symbol has its own scope.
...
A grammar with a scope per keyword would be really verbose and it's already difficult to debug issues in current state. I'm all for well-defined scopes but I don't think we should sacrifice the syntax definition for the possibility to customize everything.

I undestand that it's difficult . That's why I called these scopes optional. However:

When I see {rootScope}.{languageSymbol} I expected it would be match only languageSymbol but not languageSymbol and anythingElse

var and let should be just storage.type then

No, both language-html and Atom currently test to make sure var is scoped as storage.modifier.js.

Why do unit tests for language-html and Atom currently test to make sure var is scoped as storage.modifier.js? Seems like wrong usage of modular tests.

We should discuss more important things:

changing scopes without warning may cause them to receive the "wrong" color or actually lose highlighting.

[winstliu]

Why do unit tests for language-html and Atom currently test to make sure var is scoped as storage.modifier.js? Seems like wrong usage of modular tests.

I agree that those aren't the prettiest tests, but language-html tests to make sure that embedded JS works correctly (hence it needs to test for the JS scopes), and Atom needs to make sure grammars work, so it arbitrarily decided to use JS (and I think HTML in some other spots).

Regarding the 'lose highlighting' part:
Let's say I'm new to CSS/LESS and don't know that & exists. Therefore, to target storage.modifier.js in my less file, I might just write

.storage.modifier.js
{
    color: ...
}

So if var is suddenly changed to storage.type.var.js, then no more highlighting :(. Not that common, but it could definitely happen.

[MaximSokolov]

Therefore, to target storage.modifier.js in my less file, I might just write

Good example. Beginners have to edit their less files because of wrong color of var scope.

Your example is synthetic. I agree with @simurai's supposition:

do you think this will cause any problems for syntax authors?

Not really for syntax authors, more for users that got used to a certain color.

I don't see any way to make languages follow guidelines and don't break syntax highlighting. Still I think it worth it.

[lee-dohm]

I don't see any way to make languages follow guidelines and don't break syntax highlighting. Still I think it worth it.

I think we all agree that it is worth it. Some of us just feel that the change has to be communicated (which takes time) so that people don't feel like we've pulled the rug out from under them or that Atom is broken.

[MaximSokolov]

Still no suggestion. Just wait until Atom 2.0?

[lee-dohm]

That's not at all what I said. If you're intent on assuming the worst, there isn't anything I can do to dissuade you.

[MaximSokolov]

That's not at all what I said. If you're intent on assuming the worst, there isn't anything I can do to dissuade you.

Sorry for misunderstanding. I just want to known your opinion about what we have to do.
Should languages follow guidelines? Or we should stay all as it is now for backward compatibility. And if they should, when we should change "wrong" scopes. How to notify users? Create a doc page? Should we update languages separatly? We can create a branch for each language and update all languages at the same time. No one didn't suggest anything.

[lee-dohm]

Grammars and themes should both work from a well-designed, unified system. As for the rest of it, I'm working on some ideas. When I have more that I can share, I'll communicate it out.

[MaximSokolov]

Should languages follow guidelines?

Grammars and themes should both work from a well-designed, unified system.

I guess it means yes.

[lee-dohm]

I mean what I said: Grammars and themes should both work from a well-designed, unified system. If I had meant "yes", I would have said that.

[simurai]

Just wait until Atom 2.0?

• I think for "small fixes" that don't have a big impact: No, can be done already now.
• Enforcing a new naming convention/specification: Yes, unfortunately needs to wait for 2.0.

With "small fixes" I mean:

• changing things that are currently wrong
• don't need changes in all themes or lots of language packages
• shouldn't annoy too many users

So I think this PR still goes under the "small fixes", because most themes are not affected. Some are, like Monokai which is pretty popular, but if TextMate's version is the original, then Atom's port has been "wrong" from the beginning and now would get "fixed". Some users might get annoyed, but some might appreciate it.

Having a new naming convention/specification before 2.0 would be kinda hard to enforce, no? We would've to:

• Update all language packages
• Update all themes
• Update all styles.less

Or communicate it to package/theme authors, but I think that's a hard sell trying to do it between major versions.

How to go forward?

1. Keep making "small fixes".
2. Since defining a new naming convention/specification takes time, it's great to already start thinking about it now. Even if it doesn't get added to Atom right away and needs to wait for 2.0. After an initial draft and feedback, I like the suggestion of making a branch or fork and start to test it out with a few languages and themes. And keep refining the convention/specification until we feel confident about it.

@MaximSokolov Your enthusiasm trying to improve the grammars and syntax highlighting is great. 👏 But I hope you agree that the impact of making big changes needs to be considered too.

[MaximSokolov]

Well, if you will look at my proposed convention you will see that it differs from the TextMate convention just little. It has only one new root scope — punctuation And it's already used in most languages. The rest of the new scopes are optional (e.g. variable -> variable.argument) That means it wouldn't change highlighting (not sure about specs though).

So what I am going to do:

1. Finish work on documentation

   I need your help: we have to decide which scopes would be in the convention (pay attention to scopes under 🆕 and ❓ ). Suggest new scopes.

2. Then I create an issue for each languages from atom repo with list of "wrong" scopes.

[MaximSokolov]

Even if we wouldn't change anything we still should have a convention. Many languages' symbols not tagged now. At least we won't have to change new symbols' scopes with clear documentation.