Don't send diagnostic notifiacations by default.
These days, editors seem to mostly use the feature to actively request
diagnostics, so sending them as notification is just creating
unnecessary churn and possibly are shown twice.
Default off now, but can be enabled with `--push_diagnostic_notifications`
Issues #2164 #2376
diff --git a/verible/verilog/tools/ls/BUILD b/verible/verilog/tools/ls/BUILD
index d51f41c..da370de 100644
--- a/verible/verilog/tools/ls/BUILD
+++ b/verible/verilog/tools/ls/BUILD
@@ -269,6 +269,7 @@
deps = [
":verilog-language-server",
"//verible/common/util:init-command-line",
+ "@abseil-cpp//absl/flags:flag",
"@abseil-cpp//absl/status",
],
)
diff --git a/verible/verilog/tools/ls/verible-verilog-ls.cc b/verible/verilog/tools/ls/verible-verilog-ls.cc
index 76242e3..d3099f3 100644
--- a/verible/verilog/tools/ls/verible-verilog-ls.cc
+++ b/verible/verilog/tools/ls/verible-verilog-ls.cc
@@ -16,6 +16,7 @@
#include <iostream>
#include <string_view>
+#include "absl/flags/flag.h"
#include "absl/status/status.h"
#include "verible/common/util/init-command-line.h"
#include "verible/verilog/tools/ls/verilog-language-server.h"
@@ -30,6 +31,11 @@
#define read(fd, buf, size) _read(fd, buf, size)
#endif
+// These days, editors mostly request the diagnostic and don't want them
+// as notification (in fact, they might show it multiple times)
+ABSL_FLAG(bool, push_diagnostic_notifications, false,
+ "Send diagnostic as notifications.");
+
// Since it is hard to see what exactly the editor passes to the language
// server, let's log it, so it can be inspected in the log.
static void LogCommandline(int argc, char **argv) {
@@ -57,11 +63,14 @@
// -- Input and output is stdin and stdout.
// Output: provided write-function is called with entire response messages.
- verilog::VerilogLanguageServer server([](std::string_view reply) {
- // Output formatting as header/body chunk as required by LSP spec to stdout.
- std::cout << "Content-Length: " << reply.size() << "\r\n\r\n";
- std::cout << reply << std::flush;
- });
+ verilog::VerilogLanguageServer server(
+ absl::GetFlag(FLAGS_push_diagnostic_notifications),
+ [](std::string_view reply) {
+ // Output formatting as header/body chunk as required by LSP spec to
+ // stdout.
+ std::cout << "Content-Length: " << reply.size() << "\r\n\r\n";
+ std::cout << reply << std::flush;
+ });
// Input: Messages received from the read function are dispatched and
// processed until shutdown message received.
diff --git a/verible/verilog/tools/ls/verible-verilog-ls_test.sh b/verible/verilog/tools/ls/verible-verilog-ls_test.sh
index 4e082c1..0e05307 100755
--- a/verible/verilog/tools/ls/verible-verilog-ls_test.sh
+++ b/verible/verilog/tools/ls/verible-verilog-ls_test.sh
@@ -115,7 +115,7 @@
]
EOF
-"${LSP_SERVER}" < ${TMP_IN} 2> "${MSG_OUT}" \
+"${LSP_SERVER}" --push_diagnostic_notifications < ${TMP_IN} 2> "${MSG_OUT}" \
| ${JSON_RPC_EXPECT} ${JSON_EXPECTED}
JSON_RPC_EXIT=$?
diff --git a/verible/verilog/tools/ls/verilog-language-server.cc b/verible/verilog/tools/ls/verilog-language-server.cc
index 8bec185..31d3aea 100644
--- a/verible/verilog/tools/ls/verilog-language-server.cc
+++ b/verible/verilog/tools/ls/verilog-language-server.cc
@@ -42,7 +42,8 @@
namespace verilog {
-VerilogLanguageServer::VerilogLanguageServer(const WriteFun &write_fun)
+VerilogLanguageServer::VerilogLanguageServer(bool push_diagnostic_notification,
+ const WriteFun &write_fun)
: dispatcher_(write_fun), text_buffers_(&dispatcher_) {
// All bodies the stream splitter extracts are pushed to the json dispatcher
stream_splitter_.SetMessageProcessor(
@@ -55,11 +56,13 @@
// Whenever there is a new parse result ready, use that as an opportunity
// to send diagnostics to the client.
- parsed_buffers_.AddChangeListener(
- [this](const std::string &uri,
- const verilog::BufferTracker *buffer_tracker) {
- if (buffer_tracker) SendDiagnostics(uri, *buffer_tracker);
- });
+ if (push_diagnostic_notification) {
+ parsed_buffers_.AddChangeListener(
+ [this](const std::string &uri,
+ const verilog::BufferTracker *buffer_tracker) {
+ if (buffer_tracker) SendDiagnostics(uri, *buffer_tracker);
+ });
+ }
SetRequestHandlers();
}
diff --git a/verible/verilog/tools/ls/verilog-language-server.h b/verible/verilog/tools/ls/verilog-language-server.h
index 1e912b5..da7031c 100644
--- a/verible/verilog/tools/ls/verilog-language-server.h
+++ b/verible/verilog/tools/ls/verilog-language-server.h
@@ -38,7 +38,8 @@
using WriteFun = verible::lsp::JsonRpcDispatcher::WriteFun;
// Constructor preparing the callbacks for Language Server requests
- explicit VerilogLanguageServer(const WriteFun &write_fun);
+ explicit VerilogLanguageServer(bool push_diagnostic_notification,
+ const WriteFun &write_fun);
// Reads single request and responds to it (public to mock in tests).
absl::Status Step(const ReadFun &read_fun);
diff --git a/verible/verilog/tools/ls/verilog-language-server_test.cc b/verible/verilog/tools/ls/verilog-language-server_test.cc
index ed53bde..e4e1e1f 100644
--- a/verible/verilog/tools/ls/verilog-language-server_test.cc
+++ b/verible/verilog/tools/ls/verilog-language-server_test.cc
@@ -124,7 +124,9 @@
// sends textDocument/initialize request.
// It stores the response in initialize_response field for further processing
void SetUp() override { // not yet final
+ const bool push_notifications = true;
server_ = std::make_unique<VerilogLanguageServer>(
+ push_notifications,
[this](std::string_view response) { response_stream_ << response; });
absl::Status status = InitializeCommunication();
