[Autocomplete] Avoid prepending literals when the character has already been typed #107872
Conversation
|
This is my first time working in Godot's codebase! Any and all feedback on this approach to refine it further is appreciated. Thank you in advance! I'm especially wondering about the most appropriate type for the script language extension is, since default ints aren't supported, and if there are any other cases where it might benefit to have this context so the editor can be cleverer about autocompletion. EDIT: Thanks for the feedback! Now we don't have to worry about the ints being passed around and will instead rely on a less brute-force method via the parsed literal type. |
Thought-Weaver
changed the title
There was a problem hiding this comment.
I don't think that doing additional checks on the code is a good idea. Autocompletion should work with the parse tree instead. In the cases were we do not want the additional path char there has to exist a literal node already so the check should just be based on completion_context.current_node being a node path or string literal.
Edit: Why is this better:
- the node gives us a view into the code that is far more stable than relying on own analysis of the line
- the node gives us positions of the literal, this is not important at the moment, but it will be in the future once we transition the LSP to calculate the insertion diff completely on the server side
...es/gdscript/tests/scripts/completion/argument_options/string_literals/add_node_path_tween.gd
Outdated
Show resolved
Hide resolved
...ipt/tests/scripts/completion/argument_options/string_literals/add_string_name_input_event.gd
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Seems to work correctly based on my testing. Code looks good to me.
@Thought-Weaver you will need to squash your commits.
Additional cases where prepending doubles up Moved code to helper function Added unit tests Lookup caret character instead of passing position Switched to using the parsed type Adding safety checks and various cleanup
Thought-Weaver
force-pushed
the
users/loganapple/editor-caret-fix
branch
from
952d3d0 to
216c462
Compare
When trying to autocomplete node paths and string literals in a script editor when the relevant editor settings were enabled, if you had already typed ^"" or &"", autocomplete would insert another character to make it ^^"..." or &&"...", causing a syntax error.
The autocomplete functions weren't previously checking the type of the literal that the user had entered, so there wasn't a way to validate whether the character was already prepended or not.
Caret.Fix.mp4
Fixes: #107758
Bugsquad edit:
𖥺