Multiple highlighting improvements #506

DaelonSuzuka · 2023-10-12T06:59:18Z

binary int notation
_ spacers in numeric literals
fixed several instances of incorrect case insensitivity
@"a", &"b", ^"c", and r"d" style string literals

AlfishSoftware · 2023-10-20T02:50:56Z

Will this fix the lambda_declaration rule?
Because I was getting support.function.any-method.gdscript (method color) for func in func(): ... instead of storage.type.function.gdscript (keyword color).

AlfishSoftware · 2023-10-20T03:07:27Z

Is r"..." actually valid syntax in GDScript?
I tested in Godot 3 and 4 script editors and neither is accepting it.
It's also not listed here.

AlfishSoftware · 2023-10-20T03:18:26Z

syntaxes/GDScript.tmLanguage.json

+                    "name": "constant.character.escape"
+                }
+            ]
+        },
        "builtin_get_node_shorthand_quoted": {
            "begin": "(\\$)([\"'])",


Godot 4 also accepts %"..." and %'...' quoted syntax.

Good catch.

DaelonSuzuka · 2023-10-20T03:31:21Z

Is r"..." actually valid syntax in GDScript?

godotengine/godot#74995

AlfishSoftware · 2023-10-20T03:32:21Z

syntaxes/GDScript.tmLanguage.json

                    "name": "constant.numeric.float.gdscript"
                },
                {
-                    "match": "\\b\\d+\\b",
+                    "match": "\\b[0-9_]+\\b",


It's not right to allow _ to intermix with numbers unconditionally. Cases like _0 are valid variable identifiers, and must not be colored like number literals. This regex need to be carefully thought out and tested, because adding the _ separator feature is not that simple. You need to ensure:

_ cannot appear at the beginning, or right after e in floats or x in hex literals

_ should not appear at the end, but it seems Godot allows it; better confirm this properly to know how to go about this

consecutive __ is not allowed

~~On this particular line it would probably be enough to use "\\b\\d[0-9_]*\\b" if you won't forbid trailing _. On float and hex literals it'd probably be similar.~~

EDIT: Nevermind, forgot about the consecutive __. Yeah, this needs to be thought out properly.

I'm not interested in completely validating numeric literals with the grammar. Invalid cases should get marked as syntax errors by the LSP.

That's fair for invalid cases that aren't valid for other rules with less priority. But I believe valid variable names like _0, _0e0, _0x0 will be taken over by this rule and be incorrectly colored as numbers. At the bare minimum, these regexes gotta exclude the initial _ by using something like \d[0-9_]* instead of [0-9_]+.

But if you don't mind me completely validating it, the ideal would be:
int: "\\b\\d+(?:_\\d+)*\\b"
hex: "\\b0x[0-9A-Fa-f]+(?:_[0-9A-Fa-f]+)*\\b"
bin: "\\b0b[01]+(?:_[01]+)*\\b"
float1: "\\b\\d+(?:_\\d+)*\\.(?:\\d+(?:_\\d+)*)?(?:[eE][-+]?\\d+(?:_\\d+)*)?\\b"
float2: "\\.\\d+(?:_\\d+)*(?:[eE][-+]?\\d+(?:_\\d+)*)?\\b"
float3: "\\b\\d+(?:_\\d+)*[eE][-+]?\\d+(?:_\\d+)*\\b"

This should also fix:

[eE] exponent which can be uppercase too

a typo where the \\ part of \\d+ on line 338->368 wasn't removed

float2 starts with . (non-word character) so it must not be preceded by a \b word boundary

Sorry for writing this in a comment instead of making a PR or something.

... other rules with less priority. But I believe valid variable names like _0, _0e0, _0x0 will be taken over by this rule ...

Ah, I'm with you now; I misunderstood the problem.

I'll add some new test cases and then pop in your rules. Thanks for writing them up.

Sorry for writing this in a comment instead of making a PR or something.

No worries at all. Your second set of eyes has always been a big help.

AlfishSoftware · 2023-10-20T03:56:28Z

Is r"..." actually valid syntax in GDScript?

godotengine/godot#74995

Ah I see, so it's a new feature.

"escaping" in r-strings exists, but the sequences are escaped into themselves

I've never used this r-string before, but if I understood correctly, it seems some care needs to be taken about the internal patterns, because I believe nothing inside should be colored like an escape character. It would probably need a separate rule(s). Otherwise it can be misleading and people won't understand the difference in the syntax.

For example, in r"\"" both the \ and the " are part of the string, even though " won't close it when preceded by \.
So it should look like the image in this comment in those tests:

DaelonSuzuka · 2023-10-22T00:02:55Z

r-string stuff

Makes sense. I was just making sure they were highlighted at all.

screenshot

Wait a second, are triple single quotes valid in Godot 4? IIRC they're illegal in Godot 3 and I have them marked as such.

DaelonSuzuka · 2023-10-22T01:36:14Z

Will this fix the lambda_declaration rule? Because I was getting support.function.any-method.gdscript (method color) for func in func(): ... instead of storage.type.function.gdscript (keyword color).

This is what I'm currently seeing:

AlfishSoftware · 2023-11-17T20:13:56Z

How strange ... this is what I'm getting (both when Godot is connected or disconnected).

I'm on geequlim.godot-tools v1.3.1 on VSCodium v1.84.2
I've also disabled all other godot-related extensions (to make sure mine wasn't interfering somehow), same result.

DaelonSuzuka · 2023-11-18T23:26:24Z

I think I took those screenshots with this PR, not 1.3.1. Would you mind checking with master to make sure that this is working correctly for you?

(both when Godot is connected or disconnected)

Why would this matter?

atirut-w · 2023-11-19T03:13:37Z

Why would this matter?

I think LSP can affect some highlighting behaviour IIRC

DaelonSuzuka · 2023-11-19T04:15:44Z

It could, if the Godot LSP supported semantic tokenization, but it doesn't.

Calinou · 2023-11-19T13:57:00Z

Wait a second, are triple single quotes valid in Godot 4? IIRC they're illegal in Godot 3 and I have them marked as such.

Triple single quotes for multiline strings have always been allowed, even in Godot 1.0.

DaelonSuzuka · 2023-11-19T15:30:59Z

Wait a second, are triple single quotes valid in Godot 4? IIRC they're illegal in Godot 3 and I have them marked as such.

Triple single quotes for multiline strings have always been allowed, even in Godot 1.0.

For some reason, 2 years ago I thought it wasn't. Maybe I wrote a python-style docstring, got an error, and assumed, who knows?

Either way I've fixed it now.

Multiple highlighting improvements

96762b2

DaelonSuzuka mentioned this pull request Oct 12, 2023

Missing keywords in GDScript grammar #379

Closed

DaelonSuzuka merged commit 5bfe853 into godotengine:master Oct 13, 2023
1 check passed

DaelonSuzuka deleted the highlighting_improvements branch October 13, 2023 05:06

AlfishSoftware reviewed Oct 20, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Multiple highlighting improvements #506

Multiple highlighting improvements #506

DaelonSuzuka commented Oct 12, 2023 •

edited

Loading

AlfishSoftware commented Oct 20, 2023

AlfishSoftware commented Oct 20, 2023

AlfishSoftware Oct 20, 2023

DaelonSuzuka Oct 22, 2023

DaelonSuzuka commented Oct 20, 2023

AlfishSoftware Oct 20, 2023

AlfishSoftware Oct 20, 2023 •

edited

Loading

DaelonSuzuka Oct 20, 2023 •

edited

Loading

AlfishSoftware Oct 20, 2023 •

edited

Loading

DaelonSuzuka Oct 21, 2023 •

edited

Loading

AlfishSoftware commented Oct 20, 2023 •

edited

Loading

DaelonSuzuka commented Oct 22, 2023

DaelonSuzuka commented Oct 22, 2023 •

edited

Loading

AlfishSoftware commented Nov 17, 2023 •

edited

Loading

DaelonSuzuka commented Nov 18, 2023 •

edited

Loading

atirut-w commented Nov 19, 2023

DaelonSuzuka commented Nov 19, 2023

Calinou commented Nov 19, 2023 •

edited

Loading

DaelonSuzuka commented Nov 19, 2023

Multiple highlighting improvements #506

Multiple highlighting improvements #506

Conversation

DaelonSuzuka commented Oct 12, 2023 • edited Loading

AlfishSoftware commented Oct 20, 2023

AlfishSoftware commented Oct 20, 2023

AlfishSoftware Oct 20, 2023

Choose a reason for hiding this comment

DaelonSuzuka Oct 22, 2023

Choose a reason for hiding this comment

DaelonSuzuka commented Oct 20, 2023

AlfishSoftware Oct 20, 2023

Choose a reason for hiding this comment

AlfishSoftware Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

DaelonSuzuka Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

AlfishSoftware Oct 20, 2023 • edited Loading

Choose a reason for hiding this comment

DaelonSuzuka Oct 21, 2023 • edited Loading

Choose a reason for hiding this comment

AlfishSoftware commented Oct 20, 2023 • edited Loading

DaelonSuzuka commented Oct 22, 2023

DaelonSuzuka commented Oct 22, 2023 • edited Loading

AlfishSoftware commented Nov 17, 2023 • edited Loading

DaelonSuzuka commented Nov 18, 2023 • edited Loading

atirut-w commented Nov 19, 2023

DaelonSuzuka commented Nov 19, 2023

Calinou commented Nov 19, 2023 • edited Loading

DaelonSuzuka commented Nov 19, 2023

DaelonSuzuka commented Oct 12, 2023 •

edited

Loading

AlfishSoftware Oct 20, 2023 •

edited

Loading

DaelonSuzuka Oct 20, 2023 •

edited

Loading

AlfishSoftware Oct 20, 2023 •

edited

Loading

DaelonSuzuka Oct 21, 2023 •

edited

Loading

AlfishSoftware commented Oct 20, 2023 •

edited

Loading

DaelonSuzuka commented Oct 22, 2023 •

edited

Loading

AlfishSoftware commented Nov 17, 2023 •

edited

Loading

DaelonSuzuka commented Nov 18, 2023 •

edited

Loading

Calinou commented Nov 19, 2023 •

edited

Loading