sammccall added a comment.
Herald added a subscriber: jkorous.
Hey Simon,
Sorry this patch got stalled. I hope you're still interested!
I agree there's space for providing some high-signal information about error
conditions to power users/potential developers/administrators, even if it's not
"p
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
simark wrote:
> ilya-biryukov wrote:
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
ilya-biryukov wrote:
> simark wrote:
> > si
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
simark wrote:
> simark wrote:
> > il
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
simark wrote:
> ilya-biryukov wrote:
> > ma
simark added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
ilya-biryukov wrote:
> Could we also `log`
ilya-biryukov added inline comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
malaperle wrote:
> ilya-biryukov wro
malaperle added a comment.
Herald added a subscriber: MaskRay.
Gentle ping! It would be nice to have this so make Clangd less "verbose".
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+
simark updated this revision to Diff 138743.
simark marked 3 inline comments as done.
simark added a comment.
Address review comments
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
Files:
clangd/ClangdLSPServer.cpp
clangd/JSONRPCDispatcher.cpp
clangd/JSONRPCDispatch
ilya-biryukov added a comment.
Sorry for the delay, just a few more comments.
Comment at: clangd/ClangdLSPServer.cpp:412
+llvm::raw_string_ostream OS(Message);
+OS << "method not found (" << Method << ")";
+replyError(ErrorCode::MethodNotFound, OS.str());
--
simark marked 2 inline comments as done.
simark added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:79
+static llvm::cl::opt Verbose("verbose", llvm::cl::desc("Be more
verbose"),
+ llvm::cl::init(false));
ilya-biryuk
simark updated this revision to Diff 137582.
simark added a comment.
Update
- Add vlog method to Logger interface
- Add method name to "method not found" error message
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
Files:
clangd/ClangdLSPServer.cpp
clangd/JSONRPCDisp
ilya-biryukov added inline comments.
Comment at: clangd/Logger.cpp:19
Logger *L = nullptr;
+bool Verbose_ = false;
} // namespace
simark wrote:
> ilya-biryukov wrote:
> > Could we move the flag to implementation of `Logger`?
> > I.e.:
> > ```
> > class Logger {
simark added inline comments.
Comment at: clangd/tool/ClangdMain.cpp:79
+static llvm::cl::opt Verbose("verbose", llvm::cl::desc("Be more
verbose"),
+ llvm::cl::init(false));
ilya-biryukov wrote:
> Maybe just call it `-v`?
I wo
simark marked an inline comment as done.
simark added inline comments.
Comment at: clangd/Logger.cpp:19
Logger *L = nullptr;
+bool Verbose_ = false;
} // namespace
ilya-biryukov wrote:
> Could we move the flag to implementation of `Logger`?
> I.e.:
> ```
> clas
ilya-biryukov added inline comments.
Comment at: clangd/Logger.cpp:19
Logger *L = nullptr;
+bool Verbose_ = false;
} // namespace
Could we move the flag to implementation of `Logger`?
I.e.:
```
class Logger {
virtual log(const llvm::Twine &Message, bool Verbo
simark added a comment.
Now, if the client calls a method that we do not support (), clangd just
outputs:
C/C++: [10:55:16.033] Error -32601: method not found
It would be useful to at least print in this error message the name of the
method. Because the "unknown method" handler shares the t
simark updated this revision to Diff 137573.
simark added a comment.
Update
- Change switch to -verbose
- Add vlog function, do the filtering there
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
Files:
clangd/JSONRPCDispatcher.cpp
clangd/Logger.cpp
clangd/Logger.h
ilya-biryukov added a comment.
In https://reviews.llvm.org/D44226#1030639, @simark wrote:
> In https://reviews.llvm.org/D44226#1030625, @ilya-biryukov wrote:
>
> > I would vouch for adding a log level instead. It's a very well understood
> > concept that certainly covers this use-case and can be
simark added a comment.
In https://reviews.llvm.org/D44226#1030625, @ilya-biryukov wrote:
> I would vouch for adding a log level instead. It's a very well understood
> concept that certainly covers this use-case and can be useful in other places.
> WDYT?
I agree. How would you prefer the fla
ilya-biryukov added a comment.
I would vouch for adding a log level instead. It's a very well understood
concept that certainly covers this use-case and can be useful in other places.
WDYT?
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
simark updated this revision to Diff 137478.
simark added a comment.
Changed -log-to-stderr to -log-lsp-to-stderr
The first version disabled a bit too much, this version removes the LSP
communication logging in a more fine grained way.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.
22 matches
Mail list logo
