microsoft / TypeScript-Sublime-Plugin

IO wrapper around TypeScript language services, allowing for easy consumption by editor plugins
Apache License 2.0
1.72k stars 235 forks source link

Fix region scope #681

Closed rgant closed 5 years ago

rgant commented 5 years ago

Addresses #550. Changes the scope of the error highlights from "keyword" to "invalid.illegal" so themes can control the colors of the highlights.

msftclas commented 5 years ago

CLA assistant check
All CLA requirements met.

DanielRosenwasser commented 5 years ago

Is there a way I can see this in action? Maybe a screenshot?

Additionally, I tried using scope hunter to check whether scopes were being applied correctly and it doesn't look like they are:

image

rgant commented 5 years ago

This is an example of how it looks for me using the theme settings I applied:

screen shot 2018-11-08 at 10 14 17
<dict>
    <key>name</key>
    <string>Invalid – Illegal</string>
    <key>scope</key>
    <string>invalid.illegal</string>
    <key>settings</key>
    <dict>
        <key>foreground</key>
        <string>#F92672</string>
    </dict>
</dict>

I don't think that scope hunter is actually showing you the TypeScript-Sublime-Plugin added region either. As even without my changes I don't see the scope "keyword" which is used here.

rgant commented 5 years ago

I spent some time trying to dig through sublime's api to get the scope of an added region. Short answer is that I don't see a way to do that using the exposed API. The actual implementation of add_regions is buryed in the built-in sublime_api module, and I'm not prepared to decompile it to try and interpret C source.

But I think it's pretty clear from the Sublime Documentation that invalid is the best scope for an error in code: https://www.sublimetext.com/docs/3/scope_naming.html#invalid

DanielRosenwasser commented 5 years ago

Seems reasonable. I think we can always revert if it turns out this is too disruptive for some reason.