This revision was automatically updated to reflect the committed changes.
Closed by commit rL324358: [clangd] Use URIs in index symbols. (authored by
ioeric, committed by ).
Herald added a subscriber: llvm-commits.
Repository:
rL LLVM
https://reviews.llvm.org/D42915
Files:
clang-tools-extra
ioeric updated this revision to Diff 133009.
ioeric added a comment.
- removed leftover logs.
- Merge branch 'master' of http://llvm.org/git/clang-tools-extra into uri
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42915
Files:
clangd/index/Index.cpp
clangd/index/Index.h
sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
Comment at: unittests/clangd/SymbolCollectorTests.cpp:280
runSymbolCollector(Header.code(), /*Main=*/"");
- EXPECT_THAT(Symbols,
- UnorderedElement
ioeric updated this revision to Diff 133000.
ioeric added a comment.
- s/Uri/URI/
- Address review comments. Support multiple schemes.
- Merged with origin/master
- Merge branch 'master' of http://llvm.org/git/clang-tools-extra into uri
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.
ioeric added inline comments.
Comment at: clangd/index/Index.h:27
+ // The URI of the source file where a symbol occurs.
+ llvm::StringRef FileUri;
// The 0-based offset to the first character of the symbol from the beginning
hokein wrote:
> sammccall wrote:
hokein added inline comments.
Comment at: clangd/index/Index.h:27
+ // The URI of the source file where a symbol occurs.
+ llvm::StringRef FileUri;
// The 0-based offset to the first character of the symbol from the beginning
sammccall wrote:
> ioeric wrote:
sammccall added a comment.
Great, this all makes sense. I think we can/should make the scheme selection a
bit more robust (we shouldn't crash if we get unexpected filenames).
And... Uri or URI (I really think this is a usability issue - i had a scarring
experience with a codebase that couldn't d
ioeric added a comment.
I was thinking about leaving URI scheme customization to the postprocessing
phase, but you are right, it would be better to make the URI scheme extendable
in SymbolCollector.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42915
__
ioeric updated this revision to Diff 132958.
ioeric marked 2 inline comments as done.
ioeric added a comment.
- Make URIScheme customizable in SymbolCollector.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42915
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/i
ioeric added inline comments.
Comment at: clangd/index/Index.h:27
+ // The URI of the source file where a symbol occurs.
+ llvm::StringRef FileUri;
// The 0-based offset to the first character of the symbol from the beginning
sammccall wrote:
> nit: FileURI?
sammccall added a comment.
This looks OK so far, where is it going? It doesn't make sense to put URIs in
if we only ever use `file:`.
I guess others will be produced by some kind of extension point on
SymbolCollector. That will specify URI schemes we should try, allow you to
replace the whole `
ioeric created this revision.
ioeric added reviewers: hokein, sammccall.
Herald added subscribers: cfe-commits, jkorous-apple, ilya-biryukov, klimek.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42915
Files:
clangd/index/Index.cpp
clangd/index/Index.h
clangd/index/Merge.c
12 matches
Mail list logo