malaperle requested changes to this revision.
malaperle added a comment.
This revision now requires changes to proceed.
very minor comments
================
Comment at: clangd/ClangdServer.h:288
+ /// Get document highlights for a symbol hovered on.
+ llvm::Expected<Tagged<std::vector<DocumentHighlight>>>
+ findDocumentHighlights(PathRef File, Position Pos);
----------------
"for a symbol hovered on."
It doesn't have to be a symbol and the user doesn't have to hover on it. So
maybe just "for a given position"
================
Comment at: clangd/ClangdUnit.cpp:1088
+ SourceLocation LocStart = ValSourceRange.getBegin();
+ SourceLocation LocEnd = Lexer::getLocForEndOfToken(ValSourceRange.getEnd(),
0,
+ SourceMgr, LangOpts);
----------------
const FileEntry *F =
SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart));
if (!F)
return llvm::None;
================
Comment at: clangd/ClangdUnit.cpp:1098
+ Location L;
+ if (const FileEntry *F =
+ SourceMgr.getFileEntryForID(SourceMgr.getFileID(LocStart))) {
----------------
maybe move the check earlier? see comment above.
================
Comment at: clangd/Protocol.h:601
+
+/**
+ * A document highlight is a range inside a text document which deserves
----------------
Use /// like other structs
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D38425
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
