EditorSyntax icon indicating copy to clipboard operation
EditorSyntax copied to clipboard

Synxtax color entity.name.type not validated for [type] over storage.type scope

Open PhilipHaglund opened this issue 6 years ago • 6 comments
trafficstars

Environment

  • Editor and Version: Visual Studio Code Version: 1.35.0 (system setup) Commit: 553cfb2c2205db5f15f3ee8395bbd5cf066d357d Date: 2019-06-04T01:18:19.664Z Electron: 3.1.8 Chrome: 66.0.3359.181 Node.js: 10.2.0 V8: 6.6.346.32 OS: Windows_NT ia32 10.0.17763

  • Your primary theme: PowerShell ISE

Issue Description

The entity.name.type is not validated over storage.type when defined in settings.json nor in theme.json. When defining a type, i.e. [string], and using a statement, i.e. function, they both use the same syntax color from the storage.type scope. Is the entity.name.type scope not defined for types?

Screenshots

Example code. Pasted the tokenColorCustomization from Settings.json in the same file for example purposes. image

[string] type image

function statement image

Expected Behavior

In PowerShell ISE the statements uses the Keyword token with color '#00008b' and the types uses the Type token with color '#008080'. Is there another way to distinguish these scopes or tokens that I've missed or is the powershell.tmLanguage.json missing something?

PhilipHaglund avatar Jun 12 '19 08:06 PhilipHaglund

I'm just wondering why storage.type.powershell overrides meta.function.powershell on the "function" keyword. I see that javascript does the same thing, but why does storage.type take precedence over meta.function? Meta.function is more specific and allows for the function keyword to be colorized separate from the generic storage.type.

EDIT: It's because meta is a special keyword that doesn't get a precedence, and refers to the entire function block, not the function keyword itself.

JustinGrote avatar Aug 13 '19 22:08 JustinGrote

Also entity.name.type refers to the name of the function, e.g. function MyFunction, not the "function" keyword itself.

image

I'm working on a grammar injection extension that will add a more specific "storage.type.function.powershell" to the function keyword, so you can theme it appropriately, until such time the new and improved grammar shown in a pull request becomes the primary option.

JustinGrote avatar Aug 13 '19 23:08 JustinGrote

@JustinGrote, in PowerShell, functions are really static methods (which also get the scope entity.name.function), not types, so entity.name.type would not be the right choice. The name of a PowerShell class would be an entity.name.type scope.

Somewhere people thought that function and string are both the same thing (type, though usually function ends up storage and string ends up support). Hence the JavaScript issue with var and const and class and function and the like are also considered type. var and const are actually storage.modifier, not type, and class/function is a keyword, not type. The thought, that the keyword is going to define a type should not result in the keyword being considered a 'type'.

I have spent a bunch of time addressing this in my own theme, specifically capturing the javascript and typescript scopes and forcing them to the same color as C# scopes.

Think about the following: image image

If class was also storage type, how would we differentiate it from the type references? Type references are not entity's so the entity.name scope is not valid. The entity is coming after, in this case, the method of the class is the entity.

This is all just food for thought.

msftrncs avatar Aug 14 '19 05:08 msftrncs

@msftrncs I think you got my response confused with @PhilipHaglund.

@PhilipHaglund is saying that the function keyword should be entity.name.type.

I agree with you that it should not, it should be storage.type.function.powershell. The function name itself should be entity.name.function.powershell, per my example, as well as keyword.declaration.function.powershell to be in line with Sublime Scope Naming conventions

image

@PhilipHaglund I have made a vscode extension that addresses this issue and properly formats the function so that you can theme it appropriately. It's an effective workaround until the EditorSyntax file gets revamped (looks like work has stalled on that front for now)

https://marketplace.visualstudio.com/items?itemName=justin-grote.better-powershell-syntax-highlighting

Install this and you'll now see it will format correctly. image

JustinGrote avatar Aug 14 '19 16:08 JustinGrote

Thanks @JustinGrote will check that out! :) Since I've not very familiar with OSS, specifically with issues, should I close this one and mark as a workaround or leave it open?

PhilipHaglund avatar Aug 15 '19 07:08 PhilipHaglund

There is no reason to continue to support storage.type for keywords. Sublime's recommendation could really be improved by putting that reference in the keyword section. The preferred scope should be the section items appear in. Obviously many grammar's still do not apply the recommended scope selections suggestions, or we wouldn't be drowned in a sea of storage.type (JS and TS).

Furthermore, its surprising, as many times as I have looked at the Sublime document, I never noticed the abundant suggestion to use multiple scope selections, while the Atom editor's theme system doesn't support it. Themes in Atom have to match the entire selector (which they cannot do, because the space is a separator character in the selector expression). Its this reason that probably prevents grammars such as JS/TS from adopting the recommendation correctly.

msftrncs avatar Aug 16 '19 06:08 msftrncs