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

Added GDScript Lexer support for % and $ symbols consistent with Godot Engine #146

Closed
wants to merge 17 commits into from

Conversation

3fTL
Copy link
Contributor

@3fTL 3fTL commented Mar 7, 2023

Fixes #145

Image uses notepad++ with those changes, appears to be consistent with Godot Engine

image

@3fTL 3fTL changed the title Added GDScript Lexer support for symbol "$" consistent with Godot Engine Added GDScript Lexer support for symbol $ consistent with Godot Engine Mar 7, 2023
@nyamatongwe nyamatongwe added the gdscript Caused by the gdscript lexer label Mar 7, 2023
@3fTL
Copy link
Contributor Author

3fTL commented Mar 7, 2023

As easy as adding $ was, I am not really sure how to make % sign work.

How its displayed in godot:
image

@nyamatongwe
Copy link
Member

Changes should include test cases that exercise those changes. For this pull request, an example of SCE_GD_GETNODE should be added to test/examples/gdscript/AllStyles.gd. Then run TestLexers as described in test/README and check the result files for correctness before including them in the changes.

The 3rd column of LexicalClass is a list of tags that allows applications to assign a basic colour scheme or implement features like "search in class (identifier|comment|string)". The tags should be similar to other lexers unless there is a strong need for a new tag. getnode isn't used by other lexers and identifier is more likely to be understood. The last column is an explanation that may appear in a style definition dialog.

For %, maybe only transition to the new style when the next character is alphabetic.

@3fTL
Copy link
Contributor Author

3fTL commented Mar 8, 2023

Should SCE_GD_GETNODE be renamed to SCE_GD_NODE_IDENTIFIER? Should % have a separate SCE_GD_UNIQUENODE_IDENTIFIER?

Transitioning to new style if a character after % is alphabetic doesn't really work because % between 2 identifiers serves as modulo operator.

var a = 2
var b = 1
a%b #this would incorrectly highlight b where % is an operator
a % b

%a % b
a % %b
a.foo(%c) % %b
%a.foo(%c) % b
#if % is preceded with an identifier its treated as an operator

@3fTL
Copy link
Contributor Author

3fTL commented Mar 8, 2023

Added % not sure if i did it properly.

image

@nyamatongwe
Copy link
Member

Should SCE_GD_GETNODE be renamed to SCE_GD_NODE_IDENTIFIER?

I hadn't read the documentation before and it calls the bit after $ 'the name or path of the node'. These are more like access paths into a database. Swift has \ key-path expressions which can specify relative access for actions like sorting items.sorted(by: \.owner.name) although SCE_GD_GETNODE are absolute. It appears that / is allowed in SCE_GD_GETNODE and / could also be a division so may be ambiguous.

SCE_GD_GETNODE has multiple 'words' (possibly identifiers) separated by joining operators like /. It may deserve a new type called something like path if its going to be treated as one thing.

@3fTL
Copy link
Contributor Author

3fTL commented Mar 8, 2023

I hadn't read the documentation before and it calls the bit after $ 'the name or path of the node'. These are more like access paths into a database. Swift has \ key-path expressions which can specify relative access for actions like sorting items.sorted(by: \.owner.name) although SCE_GD_GETNODE are absolute. It appears that / is allowed in SCE_GD_GETNODE and / could also be a division so may be ambiguous.

$ is indeed followed by a path, either absolute or relative. % works pretty much the same but for unique node names. They can be mixed together.

$Node
self.get_node("Node")

$/root/ThisNode/Node
self.get_node("/root/ThisNode/Node")

$MainMenuPanel/%Options
self.get_node("MainMenuPanel/%Options")

%Options
self.get_node("%Options")

$"Node Name"
self.get_node("Node Name")

$'Node Name'
self.get_node("Node Name")

/ ambiguity shouldn't be a problem as SCE_GD_GETNODE takes priority over operator (in Godot Engine at least)

SCE_GD_GETNODE has multiple 'words' (possibly identifiers) separated by joining operators like /. It may deserve a new type called something like path if its going to be treated as one thing.

so far i've renamed SCE_GD_GETNODE to SCE_GD_NODE_IDENTIFIER, should it be renamed to SCE_GD_NODEPATH? $/% do indeed use / to separate identifiers in a path (node names). $A/childOfA/furtherChild

@3fTL 3fTL changed the title Added GDScript Lexer support for symbol $ consistent with Godot Engine Added GDScript Lexer support for % and $ symbols consistent with Godot Engine Mar 8, 2023
@nyamatongwe
Copy link
Member

so far i've renamed SCE_GD_GETNODE to SCE_GD_NODE_IDENTIFIER, should it be renamed to SCE_GD_NODEPATH?

SCE_GD_NODEPATH sounds reasonable to me. The tags entry should just be "path" until other path types are added to work out how they may need to be distinguished.

@3fTL
Copy link
Contributor Author

3fTL commented Mar 9, 2023

I think i am done with it for now, let me know if anything needs changes.

@nyamatongwe
Copy link
Member

To be compatible with all Unicode characters, StyleContext::ch is an int so, to avoid potential truncation bugs, GetGDStringState should take an int argument.

@nyamatongwe nyamatongwe added the committed Issue fixed in repository but not in release label Mar 11, 2023
@nyamatongwe
Copy link
Member

Squashed and committed.

@3fTL 3fTL closed this Mar 11, 2023
@3fTL
Copy link
Contributor Author

3fTL commented Mar 11, 2023

As luck would have it i just rememberd that format strings are a thing and those make use of % operator. Sorry about that. c5eafa1 and 29d98f3

@3fTL 3fTL reopened this Mar 11, 2023
@3fTL
Copy link
Contributor Author

3fTL commented Mar 12, 2023

And now i've made even more changes, to remove all of those percentIsOperator flags.. Once again sorry about that. As single commit https://github.com/3fTL/lexilla/tree/tiny-nodepath-fix

@nyamatongwe
Copy link
Member

Included in version 5.2.4.

@kugel-
Copy link
Contributor

kugel- commented Sep 21, 2023

I'm importing Lexilla 5.2.6 for Geany and I'm a little confused. The NodePath syntax merged here seems to be different from the Godot documentation (which uses ^"/root/Foo ? See https://docs.godotengine.org/en/stable/classes/class_nodepath.html

What's correct now?

@3fTL
Copy link
Contributor Author

3fTL commented Sep 21, 2023

It seems ^"/root/Foo" syntax is used for NodePath objects it functions differently from $/root/Foo and %/root/Foo.
$ / % are shortcuts for self.get_node(NodePath path). String after $ / % doesn't need quotes.
^ is a shortcut for NodePath(string from). String after ^ needs quotes.

In godot editor ^ functions as a prefix for either single or double quote strings. Also Godot 4 uses ^ while Godot 3 used @.

So i guess both are correct but ^ is missing here.

@kugel-
Copy link
Contributor

kugel- commented Sep 22, 2023

I don't know Godot at all so I have to trust you on that. Will you open a follow-up PR?

Regarding @, words staring with that are mapped to annotations (whatever that exactly is). Is that only true for Godot 4? How look annotations in Godot 3 look like?

Do we even support multiple language versions or just 4?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
committed Issue fixed in repository but not in release gdscript Caused by the gdscript lexer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GDScript Lexer does not support $ and % symbols
3 participants