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` (i.e. not `vlog`) names of the methods that were handled
> successfully? To have some context when something crashes and we only have
> non-verbose logs.
That would be against the purpose of this patch, which is to output nothing if
everything goes right (so it's easier to notice when something goes wrong).
Just outputting the names of the methods that are handled successfully would
still be very verbose I think.
================
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:
> malaperle wrote:
> > simark wrote:
> > > ilya-biryukov wrote:
> > > > Could we also `log` (i.e. not `vlog`) names of the methods that were
> > > > handled successfully? To have some context when something crashes and
> > > > we only have non-verbose logs.
> > > That would be against the purpose of this patch, which is to output
> > > nothing if everything goes right (so it's easier to notice when something
> > > goes wrong). Just outputting the names of the methods that are handled
> > > successfully would still be very verbose I think.
> > Wouldn't that mean pretty much logging everything coming in? The idea of
> > this patch it to make it that by default Clangd is not talkative unless
> > there is an error and optionally make it verbose, for troubleshooting.
> Logs are also useful to diagnose problems in case something crashes or works
> incorrectly.
> Clang will probably log something when processing any request anyway, logging
> the method name first will merely give some more context on which request
> other log messages correspond to.
>
> > The idea of this patch it to make it that by default Clangd is not
> > talkative unless there is an error.
> I don't think we use logging this way in clangd. Logs give us a way to make
> sense of what's happening inside clangd even when there's no error. `vlog`
> lets us turn off some extremely noisy output that is not useful most of the
> time.
> We have a whole bunch of log statements that don't correspond to errors (e.g.
> "reusing preamble", "building file with compile command").
> Logs are also useful to diagnose problems in case something crashes or works
> incorrectly.
Clang will probably log something when processing any request anyway, logging
the method name first will merely give some more context on which request other
log messages correspond to.
I think it's fine if clangd logs warning/errors by default, that a user might
want to look at and address. But logging things that happen recurrently and
are part of clangd's normal operation just flood the terminal (and makes it
harder to spot actual errors).
I agree that having some context helps to make sense of an error. A
reasonnable way would be to include the method name only when printing an
error. For example, `While handling method 'workspace/symbol': Could not open
file foo.`. That would require us to carry around some more context
information, but I think it would be better than printing all the handled
methods.
> I don't think we use logging this way in clangd. Logs give us a way to make
> sense of what's happening inside clangd even when there's no error.
In that case you turn on a more verbose log level.
> vlog lets us turn off some extremely noisy output that is not useful most of
> the time. We have a whole bunch of log statements that don't correspond to
> errors (e.g. "reusing preamble", "building file with compile command").
I would consider everything that happens at each json-rpc request to be
"noisy". When using clangd from an IDE, there's quite a lot of requests being
done. I was also thinking of modyfing the patch to also use vlog for the
"reusing preamble" and "building file with compile command" messages.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44226
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
