Merge pull request #1817 from hzeller/20230316-comment-updates

Comment updates; remove unused function.

common/lsp/BUILD

@@ -105,6 +105,7 @@ cc_library(
         ":json-rpc-dispatcher",
         ":lsp-protocol",
         "//common/strings:utf8",
+        "@com_google_absl//absl/container:flat_hash_map",
         "@com_google_absl//absl/strings",
     ],
 )

common/lsp/dummy-ls.cc

@@ -61,23 +61,25 @@ InitializeResult InitializeServer(const nlohmann::json &params) {
 
 int main(int argc, char *argv[]) {
 #ifdef _WIN32
+  // Windows messes with newlines by default. Fix this here.
   _setmode(_fileno(stdin), _O_BINARY);
+  _setmode(_fileno(stdout), _O_BINARY);
 #endif
 
   std::cerr << "Note: this dummy-ls is for testing." << std::endl;
 
   // Input and output is stdin and stdout
-  static constexpr int in_fd = 0;  // STDIN_FILENO
+  constexpr int kInputFD = 0;  // STDIN_FILENO, but Win does not have that macro
   JsonRpcDispatcher::WriteFun write_fun = [](absl::string_view reply) {
     // Output formatting as header/body chunk as required by LSP spec.
     std::cout << "Content-Length: " << reply.size() << "\r\n\r\n";
     std::cout << reply << std::flush;
   };
 
-  MessageStreamSplitter stream_splitter;
   JsonRpcDispatcher dispatcher(write_fun);
 
   // All bodies the stream splitter extracts are pushed to the json dispatcher
+  MessageStreamSplitter stream_splitter;
   stream_splitter.SetMessageProcessor(
       [&dispatcher](absl::string_view /*header*/, absl::string_view body) {
         return dispatcher.DispatchMessage(body);
@@ -101,7 +103,7 @@ int main(int argc, char *argv[]) {
   absl::Status status = absl::OkStatus();
   while (status.ok() && !shutdown_requested) {
     status = stream_splitter.PullFrom([](char *buf, int size) -> int {  //
-      return read(in_fd, buf, size);
+      return read(kInputFD, buf, size);
     });
   }
 

common/lsp/lsp-text-buffer.h

@@ -19,7 +19,7 @@
 #include <memory>
 #include <vector>
 
-//
+#include "absl/container/flat_hash_map.h"
 #include "absl/strings/str_cat.h"
 #include "absl/strings/str_split.h"
 #include "absl/strings/string_view.h"
@@ -83,7 +83,7 @@ class EditTextBuffer {
 };
 
 // A buffer collection keeps track of various open text buffers on the
-// client side. Registers new ExitTextBuffers by subscribing to events
+// client side. Registers new EditTextBuffers by subscribing to events
 // coming from the client.
 class BufferCollection {
  public:
@@ -101,11 +101,6 @@ class BufferCollection {
   // Handle textDocument/didClose event. Forget about buffer.
   void didCloseEvent(const DidCloseTextDocumentParams &o);
 
-  const EditTextBuffer *findBufferByUri(const std::string &uri) const {
-    auto found = buffers_.find(uri);
-    return found == buffers_.end() ? nullptr : found->second.get();
-  }
-
   // Edits done on all buffers from all time. Allows to compare a single
   // number if there is any change since last time. Good to remember to get
   // only changed buffers when calling MapBuffersChangedSince()
@@ -127,12 +122,13 @@ class BufferCollection {
     change_listener_ = listener;
   }
 
-  size_t documents_open() const { return buffers_.size(); }
+  // Number of open documents.
+  size_t size() const { return buffers_.size(); }
 
  private:
   int64_t global_version_ = 0;
   UriBufferCallback change_listener_ = nullptr;
-  std::unordered_map<std::string, std::unique_ptr<EditTextBuffer>> buffers_;
+  absl::flat_hash_map<std::string, std::unique_ptr<EditTextBuffer>> buffers_;
 };
 }  // namespace lsp
 }  // namespace verible

common/lsp/lsp-text-buffer_test.cc

@@ -344,7 +344,7 @@ TEST(BufferCollection, SimulateDocumentLifecycleThroughRPC) {
   JsonRpcDispatcher rpc_dispatcher([](absl::string_view) {});
   BufferCollection collection(&rpc_dispatcher);
 
-  EXPECT_EQ(collection.documents_open(), 0);
+  EXPECT_EQ(collection.size(), 0);
 
   int64_t last_global_version = 0;
 
@@ -375,7 +375,7 @@ TEST(BufferCollection, SimulateDocumentLifecycleThroughRPC) {
     }})");
 
   // We now expect one document to be open.
-  EXPECT_EQ(collection.documents_open(), 1);
+  EXPECT_EQ(collection.size(), 1);
   EXPECT_EQ(change_callback_called, 1);
 
   EXPECT_GT(collection.global_version(), last_global_version);
@@ -416,7 +416,7 @@ TEST(BufferCollection, SimulateDocumentLifecycleThroughRPC) {
      }})");
 
   // No document open anymore
-  EXPECT_EQ(collection.documents_open(), 0);
+  EXPECT_EQ(collection.size(), 0);
   EXPECT_EQ(change_callback_called, 3);
 }
 

verilog/tools/ls/verilog-language-server.cc

@@ -27,16 +27,18 @@
 namespace verilog {
 
 VerilogLanguageServer::VerilogLanguageServer(const WriteFun &write_fun)
-    : dispatcher_(write_fun), buffers_(&dispatcher_) {
+    : dispatcher_(write_fun), text_buffers_(&dispatcher_) {
   // All bodies the stream splitter extracts are pushed to the json dispatcher
   stream_splitter_.SetMessageProcessor(
       [this](absl::string_view header, absl::string_view body) {
         return dispatcher_.DispatchMessage(body);
       });
 
+  // Whenever the text changes in the editor, reparse affected code.
+  text_buffers_.SetChangeListener(parsed_buffers_.GetSubscriptionCallback());
+
   // Whenever there is a new parse result ready, use that as an opportunity
   // to send diagnostics to the client.
-  buffers_.SetChangeListener(parsed_buffers_.GetSubscriptionCallback());
   parsed_buffers_.AddChangeListener(
       [this](const std::string &uri,
              const verilog::BufferTracker *buffer_tracker) {

verilog/tools/ls/verilog-language-server.h

@@ -37,13 +37,13 @@ class VerilogLanguageServer {
   // Constructor preparing the callbacks for Language Server requests
   explicit VerilogLanguageServer(const WriteFun &write_fun);
 
-  // Reads single request and responds to it
+  // Reads single request and responds to it (public to mock in tests).
   absl::Status Step(const ReadFun &read_fun);
 
-  // Runs the Language Server
+  // Runs the Language Server, calling "read_fun" until we receive shutdown.
   absl::Status Run(const ReadFun &read_fun);
 
-  // Prints statistics of the current Language Server session
+  // Prints statistics of the current Language Server session.
   void PrintStatistics() const;
 
  private:
@@ -79,7 +79,7 @@ class VerilogLanguageServer {
   verible::lsp::JsonRpcDispatcher dispatcher_;
 
   // Object for keeping track of updates in opened buffers on client's side
-  verible::lsp::BufferCollection buffers_;
+  verible::lsp::BufferCollection text_buffers_;
 
   // Tracks changes in buffers from BufferCollection and parses their contents
   verilog::BufferTrackerContainer parsed_buffers_;

verilog/tools/ls/verilog_ls.cc

@@ -14,7 +14,6 @@
 //
 
 #include <functional>
-#include <iomanip>  // Only needed for json debugging right now
 #include <iostream>
 
 #include "common/util/init_command_line.h"
@@ -30,12 +29,6 @@
 #define read(fd, buf, size) _read(fd, buf, size)
 #endif
 
-static void FormatHeaderBodyReply(absl::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;
-}
-
 int main(int argc, char *argv[]) {
   verible::InitCommandLine(argv[0], &argc, &argv);
 
@@ -48,11 +41,18 @@ int main(int argc, char *argv[]) {
   std::cerr << "Verible Verilog Language Server built at "
             << verible::GetRepositoryVersion() << "\n";
 
-  // Input and output is stdin and stdout
-  constexpr int kInputFD = 0;  // STDIN_FILENO, but Win does not have that macro
+  // -- Input and output is stdin and stdout.
 
-  verilog::VerilogLanguageServer server(FormatHeaderBodyReply);
+  // Output: provided write-function is called with entire response messages.
+  verilog::VerilogLanguageServer server([](absl::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.
+  constexpr int kInputFD = 0;  // STDIN_FILENO, but Win does not have that macro
   absl::Status status = server.Run([](char *buf, int size) -> int {  //
     return read(kInputFD, buf, size);
   });