ilya-biryukov added inline comments.
================
Comment at: clangd/CodeComplete.cpp:755
+ });
+ log("SigHelp: requested docs for {0} symbols from the index, got {1} "
+ "symbols with non-empty docs in the response",
----------------
ioeric wrote:
> hokein wrote:
> > ioeric wrote:
> > > ilya-biryukov wrote:
> > > > ioeric wrote:
> > > > > drive by: I think this should be `vlog` or `dlog`.
> > > > Code completion also logs the number of results from sema, index, etc.
> > > > using the `log()` call.
> > > > The added log message looks similar, so trying to be consistent with
> > > > the rest of the code in this file.
> > > >
> > > > Maybe we should turn all of them into `vlog` or `dlog`, but I'd rather
> > > > leave it to a separate patch.
> > > I'm not sure which level code completion log messages should be in, but I
> > > think we should follow the guidelines in the logger documentation instead
> > > of the existing uses.
> > >
> > > Quote from Logger.h
> > > ```
> > > // log() is used for information important to understanding a clangd
> > > session.
> > > // e.g. the names of LSP messages sent are logged at this level.
> > > // This level could be enabled in production builds to allow later
> > > inspection.
> > >
> > > // vlog() is used for details often needed for debugging clangd sessions.
> > > // This level would typically be enabled for clangd developers.
> > > ```
> > My recent experience of debugging some weird GotoDef issues tells me that
> > log of index should be on production (since it is a non-trivial part in a
> > clangd session), it would be really helpful to understand what is going on.
> I agree that they are useful for debugging, but they might not be interesting
> to end-users. And I think that's why there is `vlog`. Clangd developers could
> use a different log level with `--log` flag.
I agree that `vlog` is probably a better fit here, but still think we should
change it across `CodeComplete.cpp` in a single commit, rather than partially
apply the guidelines to different parts of the file.
However, if everyone agrees we should use `vlog` here, happy to use `vlog` too
and also send a patch to update all the rest of the usages.
The situation I'm trying to avoid is:
1. We commit `vlog` here.
2. Someone argues that using `log` is actually better and we decide to not
change other usages to `log`.
3. We end up with inconsistent choices across this file: `vlog` for signature
help and `log` for code completion.
But if there's an agreement that we should use `vlog` everywhere, more than
happy to change it :-)
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D50727
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
