ols icon indicating copy to clipboard operation
ols copied to clipboard

Incorrect semantic token type for compile-time variable declarations

Open goldenbergdaniel opened this issue 1 year ago • 4 comments

When declared with a type specifier, compile-time constants are given the semantic type of type instead of variable. The screenshots below are from VS Code using the latest version of the OLS extension.

Screenshot 2024-08-11 145058 Screenshot 2024-08-11 145757

goldenbergdaniel avatar Aug 11 '24 19:08 goldenbergdaniel

This is happening because ols uses the type of a variable for the semantic highlighting.

SomeStruct :: struct {} // <-- correct color
struct_constant :: SomeStruct{} // <-- wrong color (highlighted as Struct Type)
array_constant :: []int{1, 2, 3} // <-- wrong color (highlighted as Array Type)
some_constant :: 10 // <-- correct color

To fix this, ols should inspect the right-hand side of constant declaration to determine whether it's a value or a type before applying semantic highlighting.

doongjohn avatar Apr 19 '25 15:04 doongjohn

Do you guys believe that this could be mitigated by some change on the odin.tmLanguage.json file? I see there's a section that defines the type assignment (starting at line 139):

		"type-assignment": {
			"name": "meta.definition.variable.odin",
			"begin": "\\b([A-Za-z_]\\w*)\\s*(:\\s*:)\\s*(?=(struct|union|enum|bit_set|bit_field)\\b)",
			"beginCaptures": {
				"1": { "name": "entity.name.type.odin" },
				"2": { "name": "keyword.operator.assignment.odin" },
				"3": { "name": "storage.type.odin" }
			},
			"end": "(?=^)|(?<=\\})",
			"patterns": [ { "include": "#type-declaration" } ]
		},

I'd love to test this, but I'd need to do more research on how to build LPS for vscode first.

caiolaytynher avatar Jul 24 '25 01:07 caiolaytynher

Constant in a different module still has incorrect color.

import "vendor:raylib"

f :: proc() {
	red := raylib.RED;
	//            ^^^ <-- It's a Constant value but colored as Type.
}

doongjohn avatar Aug 02 '25 11:08 doongjohn

I'll think this'll mean I need to properly resolve this, I'll reopen the issue.

BradLewis avatar Aug 02 '25 16:08 BradLewis