LSP: Calculate simple string insertions on the server-side#117710
LSP: Calculate simple string insertions on the server-side#117710HolonProduction wants to merge 2 commits intogodotengine:masterfrom
Conversation
41345fb to
fcb24a6
Compare
725e292 to
30013c6
Compare
|
Should be ready for review now. Note however, that this PR is stacked on #114598 so this one needs to be reviewed first. |
30013c6 to
e05c855
Compare
|
Tested for some time with VS Code. It works well, and makes working with string auto-completions much smoother. The only cases I've found where it's still tripping me up is when trying to auto-completing an unterminated string literal. Example 1 (| represents caret):if Input.is_action_just_pressed("ui_|):Selecting a completion option here results in: if Input.is_action_just_pressed(""ui_accept"):Example 2:var input_dir := Input.get_vector("move_|, "move_right", "move_forward", "move_back")Selecting a completion option here results in: var input_dir := Input.get_vector("move_left"move_right", "move_forward", "move_back")when I wanted; var input_dir := Input.get_vector("move_left", "move_right", "move_forward", "move_back")Example 2 I suppose is technically correct, because it's just replacing the string literal the caret is currently inside, but it's not what I want in that scenario. The behaviour in example 1 doesn't really seem right at all. I ended up in these scenarios by typing or inserting a string, realizing I made a mistake, backspacing once and then triggering auto-complete again. I imagine if you have Auto Closing Quotes disabled in the editor that would cause similar issues. It's an edge case, maybe not an important drawback, but Godot's builtin script editor actually does handle both of these examples the way I expect. I'm not sure how exactly the behaviour is specified -- seems to be something like replace the text from the opening quote up to the first word boundary at or after the caret? (in example 2, putting the caret after the underscore instead gives a different result in the built-in editor). |
For simplicity this PR uses the GDScript parsers string literal parsing. To solve this issue we would need to roll some custom island parser for string literals that is more resilient to errors. Might be worth it in the future, but I'd not add it to this initial implementation. |
e05c855 to
82f308d
Compare
|
I've compiled and tested this with the Zed GDScript extension. It's working well for me, I don't have any issues to report besides the limitation mentioned above. It improves the autocomplete experience. In Zed the default behavior for automatic autocomplete suggestions in this context is constrained; it mainly triggers after inputting double quotes, e.g. when the text is looking like this: To trigger this case, Thanks much! |
Requires and includes #114598
Supersedes #87508
Fixes #86488
Fixes godotengine/godot-vscode-plugin#961
Fixes godotengine/godot-vscode-plugin#885
Fixes GDQuest/zed-gdscript#31
Does NOT fix - godotengine/godot-vscode-plugin#644 -> can be solved using the same approach but will require an extra PR if this one goes through
If the language server only provides an
insertTextit is up to the client how this text is matched with existing code when inserting. The builtin editor does match quoted options with existing quotes, however external editors don't seem to generally do this.For such cases the LSP allows servers to calculate the insertion on the server side. They only need to provide the client with the desired range where the text should be placed and clients have to follow this range without doing any matching.
This PR makes GDScript calculate the insertion of strings on the server side. It is not possible (or much more complex) to do this only in the LSP code, since it has very limited access to the AST. So instead this PR introduces the
TextEditconcept into our own completion API and implements it directly ingdscript_editor.cpp.TODO:
_calculate_string_insertionin all places that insert stringsInsertReplaceEditis optional)ScriptLanguage::CodeCompletionOption