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

Overhaul syntax highlighting #342

Merged
merged 11 commits into from
Apr 11, 2022

Conversation

DaelonSuzuka
Copy link
Collaborator

@DaelonSuzuka DaelonSuzuka commented Apr 6, 2022

This satifies #196 by adding grammars to support .tscn, .tres, .import, .gdnlib, .gdns, and .godot files.

The GDResource grammar has special string highlighting in a number of cases:

  • TypeNames like a Node or resource's type field are green like regular use of a type
  • NodePath like a Node's name field or the argument of a NodePath constructor are orange
  • resource paths are yellow
  • embedded GDScript resources (check built-in when adding a script) have full syntax highlighting
  • embedded shader resources have full syntax highlighting
GDResource screenshots

.tscn
Code_q5uqtRrnLc

.import
Code_D4lIF9zfMt

.gdns
Code_M7h4gMwebU

There are many improvements to the existing GDScript grammar, including:

  • adding Godot 4 classes and keywords
  • separate tagging of control flow keywords(if, else, for, etc)
  • correct handling of many previously missed edge cases
  • fixed regressions caused by recent bug fixes

I made a potentially controversial decision to give special highlighting to string literals that are used as NodePaths. This include the parameters of get_node() and has_node(), as well as the path specified using the $ operator.

There is very basic support for .shader files. Essentially, they're just highlighed using glsl rules, but it's better than nothing, and there's now a hook to come back and develop a better custom grammar.

The gdresource grammer for .tscn and .tres files also correctly highlights embedded shader code and embedded gdscript.

I added a folder for example files that can be used to quickly check that special cases are handled correctly. Currently the only file is syntaxes\examples\gdscript1.gd, but I want to add similar examples for the other files types(including a gdscript2.gd example). I would greatly appreciate contributions to these test files, because a better suite of test files will greatly reduce the risk of syntax highlighting regressions.

This PR is marked as a draft because there were upstream changes to the gdscript syntax that I need to take the time to merge by hand, since the nested regex rules can interact in very surprising ways.

@DaelonSuzuka DaelonSuzuka changed the title Recreated branch from old repository Overhaul Syntax Highlighting Apr 6, 2022
@atirut-w
Copy link

atirut-w commented Apr 6, 2022

  • adding Godot 4 classes and keywords

Wouldn't that conflict with Godot 3's grammar?

@Calinou
Copy link
Member

Calinou commented Apr 6, 2022

Wouldn't that conflict with Godot 3's grammar?

I think the best decision for now would be to have support for both Godot 3 and Godot 4 keywords, preferring Godot 3 in case of a conflict. This ensures the extension can support both Godot 3.x and Godot 4.0 projects for a while.

@DaelonSuzuka
Copy link
Collaborator Author

DaelonSuzuka commented Apr 6, 2022

  • adding Godot 4 classes and keywords

Wouldn't that conflict with Godot 3's grammar?

That does not appear to be the case. Some expiriments with matching the new @decorator syntax suggest that it will also be compatible with both.

However, walking this line is one of the reasons I want to develop a suite of test files, so we can all be more confident that regressions don't make it out to users.

I think the best decision for now would be to have support for both Godot 3 and Godot 4 keywords, preferring Godot 3 in case of a conflict. This ensures the extension can support both Godot 3.x and Godot 4.0 projects for a while.

I think it's very likely that both languages can be fully supported by one grammar.

IF there ends up being a serious incompatibility, the most robust solution might be to support as big a common subset as possible with the grammar, and add tagging support to the language server, since the editor will know for sure which version is being parsed. (I will eventually work on this anyways, because I want to have semantic highlighting based on context.)

@DaelonSuzuka
Copy link
Collaborator Author

DaelonSuzuka commented Apr 11, 2022

I'm marking this ready for review even though I could easily spend another week polishing.

Attached are some screenshots of the test file I've been using to develop the gdscript grammar. On the left is 1.2.0, on the right is this PR.

Test File Screenshots

Lines 12-20: I've redone signal declarations, including a fix for #334
Lines 23-55: The function declaration rule was fundamentally broken and had to be rebuilt

mGDy4MDlA4

Lines 68-71: Variable name rules were incorrectly tagging some of these as PascalClass names.
Lines 75-96: Special highlighting of everything that counts as a NodePath, including strings used as the parameter for get_node(), has_node() and NodePath()

kRNxrrnxSK

Lines 121-128: Type hint syntax was totally broken and overriding definition of dictionary literals. Call to int() was not properly detected as a type constructor.

lGMJsV87ij

Edit: I found at least 2 issues just writing the notes for this post:
Line 27: not highlighting the setget keyword, but only after a type annotation
Line 81 & 82: strings with only punctuation not being detected as NodePaths
I'll fix these tomorrow.

@DaelonSuzuka
Copy link
Collaborator Author

DaelonSuzuka commented Apr 11, 2022

Fixed the two issues from the last comment.

I'm ready to not look at regex for a couple weeks, so I'm done polishing this for now unless somebody finds a mistake.

I might make an issue to collect thoughts for future highlighting improvements.

Screenshots

Code_sJLy1dGGbO

Code_17r7YXU0Vp

Copy link
Member

@Calinou Calinou left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@Calinou Calinou changed the title Overhaul Syntax Highlighting Overhaul syntax highlighting Apr 11, 2022
@Calinou Calinou merged commit 78e37e8 into godotengine:master Apr 11, 2022
@DaelonSuzuka DaelonSuzuka deleted the syntax-highlighting branch May 11, 2022 09:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants