jkorous updated this revision to Diff 175230.
jkorous added a comment.
Removed empty line noise and fixed doxygen annotation.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54799/new/
https://reviews.llvm.org/D54799
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/Cl
jkorous updated this revision to Diff 175229.
jkorous marked an inline comment as done.
jkorous added a comment.
Addressed comments from the reivew.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54799/new/
https://reviews.llvm.org/D54799
Files:
clangd/CMakeLists.txt
clangd/ClangdLS
jkorous marked 8 inline comments as done.
jkorous added a comment.
Thank you very much for the review Sam!
I am going to write proper unit tests and then just wait for Alex and Ben to
take a look.
Comment at: clangd/XRefs.cpp:785
+}
+Results.emplace_back(std::move(New
sammccall added a comment.
Thanks, this looks good! Just nits, and please do port most of the test cases
to unit tests.
Comment at: clangd/ClangdServer.cpp:529
+
+ WorkScheduler.runWithAST("CursorInfo", File, Bind(Action, std::move(CB)));
+}
nit: SymbolInfo
jkorous updated this revision to Diff 175137.
jkorous added a comment.
Multiple symbols per location.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D54799/new/
https://reviews.llvm.org/D54799
Files:
clangd/CMakeLists.txt
clangd/ClangdLSPServer.cpp
clangd/ClangdLSPServer.h
clangd
jkorous updated this revision to Diff 175115.
jkorous marked 2 inline comments as done.
jkorous added a comment.
Herald added a subscriber: mgorny.
Couple minor changes based on discussion.
- Move `SymbolID` to `index/SymbolID.h`.
- Rename in `ClangdServer` - drop the verb from method name.
- Rem
jkorous marked 7 inline comments as done.
jkorous added a comment.
In https://reviews.llvm.org/D54799#1306585, @sammccall wrote:
> So I think both SymbolID and USR are optional.
No problem.
I am just wondering if it make sense to include any symbol with empty name,
empty USR and no ID in LSP r
hokein added inline comments.
Comment at: XRefs.cpp:95
Preprocessor &PP;
+ const bool StopOnFirstDeclFound;
sammccall wrote:
> jkorous wrote:
> > sammccall wrote:
> > > Please don't do this - it's inconsistent with the other XRefs features.
> > > (If for so
sammccall added a subscriber: hokein.
sammccall added a comment.
In https://reviews.llvm.org/D54799#1306488, @jkorous wrote:
> >> - conditional return in `getCursorInfo` - Should we return for example
> >> data with empty `USR`?
> >
> > Please return a symbol unless it has no SymbolID (we don't
jkorous updated this revision to Diff 175069.
jkorous marked 2 inline comments as done.
jkorous added a comment.
Changes based on review.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D54799
Files:
ClangdLSPServer.cpp
ClangdLSPServer.h
ClangdServer.cpp
ClangdServer.h
jkorous marked 7 inline comments as done.
jkorous added a comment.
Hi Sam.
In https://reviews.llvm.org/D54799#1305470, @sammccall wrote:
>
>> I'd like to ask for early feedback - what's still missing is relevant client
>> capability.
>
> I'd suggest leaving it out unless others feel strong
sammccall added a comment.
Thanks for sending this! Broadly looks good, a few details to work out. I think
the biggest one is multiple symbols which you've flagged.
> I'd like to ask for early feedback - what's still missing is relevant client
> capability.
I'm actually not 100% sure that's ne
jkorous created this revision.
jkorous added reviewers: sammccall, arphaman, benlangmuir.
Herald added subscribers: cfe-commits, kadircet, dexonsmith, MaskRay, ioeric,
ilya-biryukov.
Hi,
I implemented `textDocument/cursorInfo` method based on consensus in
https://reviews.llvm.org/D54529.
I'd li
13 matches
Mail list logo
