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

Add special cases for UPPERCASE classes in GDScript syntax-coloring (JSON, UPNP, etc); and color super as a keyword #767

Open
AlfishSoftware opened this issue Dec 18, 2024 · 2 comments
Labels

Comments

@AlfishSoftware
Copy link

AlfishSoftware commented Dec 18, 2024

Godot version

4.3.stable

VS Code version

1.96.0 codium snap

Godot Tools VS Code extension version

2.3.0

System information

Ubuntu 24.04

Issue description

Opening an issue for the suggestion here #739 (comment) so it's not forgotten, as @DaelonSuzuka seems to agree.

It seems GDScript doesn't currently support semantic coloring, so it uses certain heuristics, like coloring UPPERCASE identifiers as constants. This is good enough for the majority of cases, but there's a few exceptions in Godot built-in classes that are somewhat easy to track.

These classes are known to be using UPPERCASE-only identifiers, exceptionally:

  • JSON (constructible with new)
  • UPNP (constructible with new)
  • OS
  • IP
  • JSONRPC
  • XRVRS

Since it's a very small list, it's easy to add them to GDScript.tmLanguage.json under pascal_case_class as exceptions, so they are matched as classes rather than constants. All 6 above should be added. If not, at least the first 2, which are constructible with new, and so it's extra weird to color them as constants.

"pascal_case_class": {
"match": "\\b([A-Z]+[a-z_0-9]*([A-Z]?[a-z_0-9]+)*[A-Z]?)\\b",
"name": "entity.name.type.class.gdscript"
},


Also, please move super away from builtin_classes, as it's a keyword, not a class.
It can be called like super(arg1, arg2) as well as super.my_base_method(etc), and both of these semantics are different from a class, specially from a basic Variant type.
If even void is a keyword, then super should surely not be a type.
More importantly, it raises an error when used as an identifier, unlike built-in class names. So it's a keyword.

"builtin_classes": {
"match": "(?<![^.]\\.|:)\\b(Vector2|Vector2i|Vector3|Vector3i|Vector4|Vector4i|Color|Rect2|Rect2i|Array|Basis|Dictionary|Plane|Quat|RID|Rect3|Transform|Transform2D|Transform3D|AABB|String|Color|NodePath|PoolByteArray|PoolIntArray|PoolRealArray|PoolStringArray|PoolVector2Array|PoolVector3Array|PoolColorArray|bool|int|float|Signal|Callable|StringName|Quaternion|Projection|PackedByteArray|PackedInt32Array|PackedInt64Array|PackedFloat32Array|PackedFloat64Array|PackedStringArray|PackedVector2Array|PackedVector2iArray|PackedVector3Array|PackedVector3iArray|PackedVector4Array|PackedColorArray|super)\\b",
"name": "entity.name.type.class.builtin.gdscript"
},

Steps to reproduce

func wrong_coloring() -> void:
	var data_to_send: PackedStringArray = ["a", "b", "c"]
	var json_string: String = JSON.stringify(data_to_send)
	var json: JSON = JSON.new()
	var error: Error = json.parse(json_string)
	
	var upnp: UPNP = UPNP.new()
	upnp.discover()
	upnp.add_port_mapping(7777)
func expected_coloring() -> void: # For comparison
	var regex: RegEx = RegEx.new()
	regex.compile("\\w-(\\d+)")
	var regex2: RegEx = RegEx.create_from_string("\\w-(\\d+)")
@DaelonSuzuka
Copy link
Collaborator

Thanks for making the issue, that was a big help.

UPPERCASE builtin classes

My instinct is to add these names to builtin_classes:

"builtin_classes": {
"match": "(?<![^.]\\.|:)\\b(Vector2|Vector2i|Vector3|Vector3i|Vector4|Vector4i|Color|Rect2|Rect2i|Array|Basis|Dictionary|Plane|Quat|RID|Rect3|Transform|Transform2D|Transform3D|AABB|String|Color|NodePath|PoolByteArray|PoolIntArray|PoolRealArray|PoolStringArray|PoolVector2Array|PoolVector3Array|PoolColorArray|bool|int|float|Signal|Callable|StringName|Quaternion|Projection|PackedByteArray|PackedInt32Array|PackedInt64Array|PackedFloat32Array|PackedFloat64Array|PackedStringArray|PackedVector2Array|PackedVector2iArray|PackedVector3Array|PackedVector3iArray|PackedVector4Array|PackedColorArray|super)\\b",
"name": "entity.name.type.class.builtin.gdscript"
},

This produces the desired highlighting, so is there a reason I should do it using pascal_case_class instead?

Image

super

Like this?

Image

@AlfishSoftware
Copy link
Author

AlfishSoftware commented Jan 26, 2025

This produces the desired highlighting, so is there a reason I should do it using pascal_case_class instead?

Yes! The theme ID is not exactly the same.

  • builtin_classes is currently listing only variant/basic types (except for super)
  • This makes it a bit similar to Godot editor, which does make a distinction between basic types, engine classes and user scripts (you can use different colors for those in Godot). A VSCode theme could at least color variant types differently if it wants to. Allowing better consistency between compatible themes.
  • Except for RID, all UPPERCASE classes listed above are Object-derived. None are basic types. So conceptually, they're the same as other engine classes, they just so happen to be a "pascal-case" exception that has 1-letter words.

So, in summary, there's no reason to make a distinction between these and "proper" pascal-case classes (they're conceptually the same), but there is a valid reason, theme-wise, to separate them from basic types.


super; Like this?

Yes. Though it might be best to be specific like keyword.language.super.gdscript in case a theme still wants to color it differently (e.g. if someone does prefer it colored like a type for whatever reason, e.g. got used to it). Regex group references like $1 work in the name field too, so you could have it generic like keyword.language.$1.gdscript to add a specific id to many keywords.

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

No branches or pull requests

2 participants