simark created this revision.
Herald added subscribers: cfe-commits, ioeric, JDevlieghere, jkorous-apple, 
ilya-biryukov, klimek.

Currently, clangd prints all the LSP communication on stderr.  While
this is good for developping, I don't think it's good as a general
default.  For example, we are integrating clangd (and other language
servers) in an IDE, and we print everything the language servers send on
their stderr on our own stderr.  Printing the whole LSP communication
adds a lot of noise.  If there are actual error messages to be printed,
then it makes sense to output them on stderr.

Also, an IDE or tool that integrates many language servers will likely
have its own way of logging LSP communication in a consistent way, so
it's better to leave it to that tool to log the LSP communication if it
wants to.

An alternative would be to introduce log levels (debug, info, warning,
error), and output LSP communications only at the debug log level.

Signed-off-by: Simon Marchi <simon.mar...@ericsson.com>


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D44226

Files:
  clangd/JSONRPCDispatcher.cpp
  clangd/JSONRPCDispatcher.h
  clangd/tool/ClangdMain.cpp
  test/clangd/protocol.test
  test/clangd/too_large.test

Index: test/clangd/too_large.test
===================================================================
--- test/clangd/too_large.test
+++ test/clangd/too_large.test
@@ -1,4 +1,4 @@
-# RUN: not clangd -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# RUN: not clangd -run-synchronously -log-to-stderr < %s 2>&1 | FileCheck -check-prefix=STDERR %s
 # vim: fileformat=dos
 # It is absolutely vital that this file has CRLF line endings.
 #
Index: test/clangd/protocol.test
===================================================================
--- test/clangd/protocol.test
+++ test/clangd/protocol.test
@@ -1,5 +1,5 @@
-# RUN: not clangd -pretty -run-synchronously < %s | FileCheck -strict-whitespace %s
-# RUN: not clangd -pretty -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
+# RUN: not clangd -pretty -log-to-stderr -run-synchronously < %s | FileCheck -strict-whitespace %s
+# RUN: not clangd -pretty -log-to-stderr -run-synchronously < %s 2>&1 | FileCheck -check-prefix=STDERR %s
 # vim: fileformat=dos
 # It is absolutely vital that this file has CRLF line endings.
 #
Index: clangd/tool/ClangdMain.cpp
===================================================================
--- clangd/tool/ClangdMain.cpp
+++ clangd/tool/ClangdMain.cpp
@@ -77,15 +77,20 @@
     llvm::cl::init(JSONStreamStyle::Standard));
 
 static llvm::cl::opt<bool>
+    LogToStderr("log-to-stderr",
+                llvm::cl::desc("Output LSP communication on stderr"),
+                llvm::cl::init(false));
+
+static llvm::cl::opt<bool>
     PrettyPrint("pretty", llvm::cl::desc("Pretty-print JSON output"),
                 llvm::cl::init(false));
 
-static llvm::cl::opt<bool> Test(
-    "lit-test",
-    llvm::cl::desc(
-        "Abbreviation for -input-style=delimited -pretty -run-synchronously. "
-        "Intended to simplify lit tests."),
-    llvm::cl::init(false), llvm::cl::Hidden);
+static llvm::cl::opt<bool>
+    Test("lit-test",
+         llvm::cl::desc(
+             "Abbreviation for -input-style=delimited -pretty -log-to-stderr "
+             "-run-synchronously. Intended to simplify lit tests."),
+         llvm::cl::init(false), llvm::cl::Hidden);
 
 static llvm::cl::opt<PCHStorageFlag> PCHStorage(
     "pch-storage",
@@ -140,6 +145,7 @@
   if (Test) {
     RunSynchronously = true;
     InputStyle = JSONStreamStyle::Delimited;
+    LogToStderr = true;
     PrettyPrint = true;
   }
 
@@ -190,7 +196,7 @@
 
   JSONOutput Out(llvm::outs(), llvm::errs(),
                  InputMirrorStream ? InputMirrorStream.getPointer() : nullptr,
-                 PrettyPrint);
+                 LogToStderr, PrettyPrint);
 
   clangd::LoggingSession LoggingSession(Out);
 
Index: clangd/JSONRPCDispatcher.h
===================================================================
--- clangd/JSONRPCDispatcher.h
+++ clangd/JSONRPCDispatcher.h
@@ -30,8 +30,10 @@
   // JSONOutput now that we pass Context everywhere.
 public:
   JSONOutput(llvm::raw_ostream &Outs, llvm::raw_ostream &Logs,
-             llvm::raw_ostream *InputMirror = nullptr, bool Pretty = false)
-      : Pretty(Pretty), Outs(Outs), Logs(Logs), InputMirror(InputMirror) {}
+             llvm::raw_ostream *InputMirror = nullptr, bool LogToStderr = false,
+             bool Pretty = false)
+      : LogToStderr(LogToStderr), Pretty(Pretty), Outs(Outs), Logs(Logs),
+        InputMirror(InputMirror) {}
 
   /// Emit a JSONRPC message.
   void writeMessage(const json::Expr &Result);
@@ -44,6 +46,9 @@
   /// Unlike other methods of JSONOutput, mirrorInput is not thread-safe.
   void mirrorInput(const Twine &Message);
 
+  // Whether to output all LSP communication to stderr.
+  const bool LogToStderr;
+
   // Whether output should be pretty-printed.
   const bool Pretty;
 
Index: clangd/JSONRPCDispatcher.cpp
===================================================================
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -70,8 +70,12 @@
 }
 
 void JSONOutput::log(const Twine &Message) {
-  llvm::sys::TimePoint<> Timestamp = std::chrono::system_clock::now();
   trace::log(Message);
+
+  if (!LogToStderr)
+    return;
+
+  llvm::sys::TimePoint<> Timestamp = std::chrono::system_clock::now();
   std::lock_guard<std::mutex> Guard(StreamMutex);
   Logs << llvm::formatv("[{0:%H:%M:%S.%L}] {1}\n", Timestamp, Message);
   Logs.flush();
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to