[PATCH] D38425: [clangd] Document highlights for clangd

Marc-Andre Laperle via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 30 12:08:56 PST 2017


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





More information about the cfe-commits mailing list