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
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
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:
> >
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:
> >
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:
> >
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
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
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));
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
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
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.:
> ```
>
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 , bool Verbose);
};
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
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
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
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
22 matches
Mail list logo
