Skip to content

Commit

Permalink
Add option for LSP diagnostics on change.
Browse files Browse the repository at this point in the history
Fixes godotengine#87410.

Currently the LSP sends diagnostics on every change.
This means that if you're typing `var thread := Thread.new()`,
the language server sends a diagnostic after every keypress:

```
"Expected variable name after \"var\"."
"Expected variable name after \"var\"."
"(UNUSED_VARIABLE): The local variable \"t\" is declared but never used in the block. If this is intended
"(UNUSED_VARIABLE): The local variable \"th\" is declared but never used in the block. If this is intended
"(UNUSED_VARIABLE): The local variable \"thr\" is declared but never used in the block. If this is intended
"(UNUSED_VARIABLE): The local variable \"thre\" is declared but never used in the block. If this is intended
"(UNUSED_VARIABLE): The local variable \"threa\" is declared but never used in the block. If this is intended
"(UNUSED_VARIABLE): The local variable \"thread\" is declared but never used in the block. If this is intended
"(UNUSED_VARIABLE): The local variable \"thread\" is declared but never used in the block. If this is intended
"Expected type after \":\""
"Expected expression for variable initial value after \"=\"."
"Expected expression for variable initial value after \"=\"."
```

This causes the LSP to be slow and the diagnositcs aren't that helpful.

This patch adds a "Diagnostics on Change" option, which defaults on to maintain existing behavior.
When toggled off, diagnostics are only sent on open/save, and not on every change.
  • Loading branch information
rcorre committed Jan 23, 2024
1 parent 0bcc0e9 commit 4ebe61a
Show file tree
Hide file tree
Showing 5 changed files with 17 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ GDScriptLanguageServer::GDScriptLanguageServer() {
_EDITOR_DEF("network/language_server/enable_smart_resolve", true);
_EDITOR_DEF("network/language_server/show_native_symbols_in_editor", false);
_EDITOR_DEF("network/language_server/use_thread", use_thread);
_EDITOR_DEF("network/language_server/diagnostics_on_change", true);
}

void GDScriptLanguageServer::_notification(int p_what) {
Expand Down
12 changes: 7 additions & 5 deletions modules/gdscript/language_server/gdscript_text_document.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ void GDScriptTextDocument::_bind_methods() {

void GDScriptTextDocument::didOpen(const Variant &p_param) {
lsp::TextDocumentItem doc = load_document_item(p_param);
sync_script_content(doc.uri, doc.text);
sync_script_content(doc.uri, doc.text, true);
}

void GDScriptTextDocument::didClose(const Variant &p_param) {
Expand All @@ -82,7 +82,9 @@ void GDScriptTextDocument::didChange(const Variant &p_param) {
evt.load(contentChanges[i]);
doc.text = evt.text;
}
sync_script_content(doc.uri, doc.text);

bool diagnostics_on_save = _EDITOR_GET("network/language_server/diagnostics_on_change");
sync_script_content(doc.uri, doc.text, diagnostics_on_save);
}

void GDScriptTextDocument::willSaveWaitUntil(const Variant &p_param) {
Expand All @@ -100,7 +102,7 @@ void GDScriptTextDocument::didSave(const Variant &p_param) {
Dictionary dict = p_param;
String text = dict["text"];

sync_script_content(doc.uri, text);
sync_script_content(doc.uri, text, true);

String path = GDScriptLanguageProtocol::get_singleton()->get_workspace()->get_file_path(doc.uri);
Ref<GDScript> scr = ResourceLoader::load(path);
Expand Down Expand Up @@ -482,9 +484,9 @@ GDScriptTextDocument::GDScriptTextDocument() {
file_checker = FileAccess::create(FileAccess::ACCESS_RESOURCES);
}

void GDScriptTextDocument::sync_script_content(const String &p_path, const String &p_content) {
void GDScriptTextDocument::sync_script_content(const String &p_path, const String &p_content, bool p_publish_diagnostics) {
String path = GDScriptLanguageProtocol::get_singleton()->get_workspace()->get_file_path(p_path);
GDScriptLanguageProtocol::get_singleton()->get_workspace()->parse_script(path, p_content);
GDScriptLanguageProtocol::get_singleton()->get_workspace()->parse_script(path, p_content, p_publish_diagnostics);

EditorFileSystem::get_singleton()->update_file(path);
}
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/language_server/gdscript_text_document.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class GDScriptTextDocument : public RefCounted {
void willSaveWaitUntil(const Variant &p_param);
void didSave(const Variant &p_param);

void sync_script_content(const String &p_path, const String &p_content);
void sync_script_content(const String &p_path, const String &p_content, bool p_publish_diagnostics);
void show_native_symbol_in_editor(const String &p_symbol_id);

Array native_member_completions;
Expand Down
12 changes: 7 additions & 5 deletions modules/gdscript/language_server/gdscript_workspace.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ void GDScriptWorkspace::did_delete_files(const Dictionary &p_params) {
Dictionary file = files[i];
String uri = file["uri"];
String path = get_file_path(uri);
parse_script(path, "");
parse_script(path, "", true);
}
}

Expand Down Expand Up @@ -217,7 +217,7 @@ void GDScriptWorkspace::reload_all_workspace_scripts() {
Error err;
String content = FileAccess::get_file_as_string(path, &err);
ERR_CONTINUE(err != OK);
err = parse_script(path, content);
err = parse_script(path, content, true);

if (err != OK) {
HashMap<String, ExtendGDScriptParser *>::Iterator S = parse_results.find(path);
Expand Down Expand Up @@ -413,7 +413,7 @@ Error GDScriptWorkspace::initialize() {
return OK;
}

Error GDScriptWorkspace::parse_script(const String &p_path, const String &p_content) {
Error GDScriptWorkspace::parse_script(const String &p_path, const String &p_content, bool p_publish_diagnostics) {
ExtendGDScriptParser *parser = memnew(ExtendGDScriptParser);
Error err = parser->parse(p_content, p_path);
HashMap<String, ExtendGDScriptParser *>::Iterator last_parser = parse_results.find(p_path);
Expand All @@ -431,7 +431,9 @@ Error GDScriptWorkspace::parse_script(const String &p_path, const String &p_cont
parse_results[p_path] = parser;
}

publish_diagnostics(p_path);
if (p_publish_diagnostics) {
publish_diagnostics(p_path);
}

return err;
}
Expand Down Expand Up @@ -546,7 +548,7 @@ Error GDScriptWorkspace::parse_local_script(const String &p_path) {
Error err;
String content = FileAccess::get_file_as_string(p_path, &err);
if (err == OK) {
err = parse_script(p_path, content);
err = parse_script(p_path, content, true);
}
return err;
}
Expand Down
2 changes: 1 addition & 1 deletion modules/gdscript/language_server/gdscript_workspace.h
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ class GDScriptWorkspace : public RefCounted {
public:
Error initialize();

Error parse_script(const String &p_path, const String &p_content);
Error parse_script(const String &p_path, const String &p_content, bool p_publish_diagnostics);
Error parse_local_script(const String &p_path);

String get_file_path(const String &p_uri) const;
Expand Down

0 comments on commit 4ebe61a

Please sign in to comment.