[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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.

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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:

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Haojian Wu via Phabricator via cfe-commits
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:

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Sam McCall via Phabricator via cfe-commits
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

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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 __

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-06 Thread Eric Liu via Phabricator via cfe-commits
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?

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-05 Thread Sam McCall via Phabricator via cfe-commits
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 `

[PATCH] D42915: [clangd] Use URIs in index symbols.

2018-02-05 Thread Eric Liu via Phabricator via cfe-commits
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