ioeric 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",
----------------
ilya-biryukov wrote:
> 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 :-)
That sounds good to me. Thanks!
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
