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