[clang-tools-extra] r369884 - [clangd] Send highlighting diff beyond the end of the file.
Haojian Wu via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 26 01:38:45 PDT 2019
Author: hokein
Date: Mon Aug 26 01:38:45 2019
New Revision: 369884
URL: http://llvm.org/viewvc/llvm-project?rev=369884&view=rev
Log:
[clangd] Send highlighting diff beyond the end of the file.
Summary: This would make the client life (tracking the changes) easier.
Reviewers: jvikstrom
Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D66541
Modified:
clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
clang-tools-extra/trunk/clangd/ClangdLSPServer.h
clang-tools-extra/trunk/clangd/ClangdServer.cpp
clang-tools-extra/trunk/clangd/ClangdServer.h
clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
clang-tools-extra/trunk/clangd/SemanticHighlighting.h
clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.cpp Mon Aug 26 01:38:45 2019
@@ -1195,7 +1195,7 @@ bool ClangdLSPServer::shouldRunCompletio
}
void ClangdLSPServer::onHighlightingsReady(
- PathRef File, std::vector<HighlightingToken> Highlightings, int NumLines) {
+ PathRef File, std::vector<HighlightingToken> Highlightings) {
std::vector<HighlightingToken> Old;
std::vector<HighlightingToken> HighlightingsCopy = Highlightings;
{
@@ -1205,8 +1205,7 @@ void ClangdLSPServer::onHighlightingsRea
}
// LSP allows us to send incremental edits of highlightings. Also need to diff
// to remove highlightings from tokens that should no longer have them.
- std::vector<LineHighlightings> Diffed =
- diffHighlightings(Highlightings, Old, NumLines);
+ std::vector<LineHighlightings> Diffed = diffHighlightings(Highlightings, Old);
publishSemanticHighlighting(
{{URIForFile::canonicalize(File, /*TUPath=*/File)},
toSemanticHighlightingInformation(Diffed)});
Modified: clang-tools-extra/trunk/clangd/ClangdLSPServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdLSPServer.h?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdLSPServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdLSPServer.h Mon Aug 26 01:38:45 2019
@@ -55,9 +55,9 @@ private:
// Implement DiagnosticsConsumer.
void onDiagnosticsReady(PathRef File, std::vector<Diag> Diagnostics) override;
void onFileUpdated(PathRef File, const TUStatus &Status) override;
- void onHighlightingsReady(PathRef File,
- std::vector<HighlightingToken> Highlightings,
- int NumLines) override;
+ void
+ onHighlightingsReady(PathRef File,
+ std::vector<HighlightingToken> Highlightings) override;
// LSP methods. Notifications have signature void(const Params&).
// Calls have signature void(const Params&, Callback<Response>).
Modified: clang-tools-extra/trunk/clangd/ClangdServer.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.cpp?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.cpp (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.cpp Mon Aug 26 01:38:45 2019
@@ -73,19 +73,10 @@ struct UpdateIndexCallbacks : public Par
if (SemanticHighlighting)
Highlightings = getSemanticHighlightings(AST);
- // FIXME: We need a better way to send the maximum line number to the
- // differ.
- // The differ needs the information about the max number of lines
- // to not send diffs that are outside the file.
- const SourceManager &SM = AST.getSourceManager();
- FileID MainFileID = SM.getMainFileID();
- int NumLines = SM.getBufferData(MainFileID).count('\n') + 1;
-
Publish([&]() {
DiagConsumer.onDiagnosticsReady(Path, std::move(Diagnostics));
if (SemanticHighlighting)
- DiagConsumer.onHighlightingsReady(Path, std::move(Highlightings),
- NumLines);
+ DiagConsumer.onHighlightingsReady(Path, std::move(Highlightings));
});
}
Modified: clang-tools-extra/trunk/clangd/ClangdServer.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/ClangdServer.h?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/ClangdServer.h (original)
+++ clang-tools-extra/trunk/clangd/ClangdServer.h Mon Aug 26 01:38:45 2019
@@ -56,8 +56,7 @@ public:
/// where generated from.
virtual void
onHighlightingsReady(PathRef File,
- std::vector<HighlightingToken> Highlightings,
- int NumLines) {}
+ std::vector<HighlightingToken> Highlightings) {}
};
/// When set, used by ClangdServer to get clang-tidy options for each particular
Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.cpp Mon Aug 26 01:38:45 2019
@@ -339,9 +339,11 @@ takeLine(ArrayRef<HighlightingToken> All
std::vector<LineHighlightings>
diffHighlightings(ArrayRef<HighlightingToken> New,
- ArrayRef<HighlightingToken> Old, int NewMaxLine) {
- assert(std::is_sorted(New.begin(), New.end()) && "New must be a sorted vector");
- assert(std::is_sorted(Old.begin(), Old.end()) && "Old must be a sorted vector");
+ ArrayRef<HighlightingToken> Old) {
+ assert(std::is_sorted(New.begin(), New.end()) &&
+ "New must be a sorted vector");
+ assert(std::is_sorted(Old.begin(), Old.end()) &&
+ "Old must be a sorted vector");
// FIXME: There's an edge case when tokens span multiple lines. If the first
// token on the line started on a line above the current one and the rest of
@@ -371,9 +373,7 @@ diffHighlightings(ArrayRef<HighlightingT
return std::min(NextNew, NextOld);
};
- // If the New file has fewer lines than the Old file we don't want to send
- // highlightings beyond the end of the file.
- for (int LineNumber = 0; LineNumber < NewMaxLine;
+ for (int LineNumber = 0; NewLine.end() < NewEnd || OldLine.end() < OldEnd;
LineNumber = NextLineNumber()) {
NewLine = takeLine(New, NewLine.end(), LineNumber);
OldLine = takeLine(Old, OldLine.end(), LineNumber);
Modified: clang-tools-extra/trunk/clangd/SemanticHighlighting.h
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/SemanticHighlighting.h?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/SemanticHighlighting.h (original)
+++ clang-tools-extra/trunk/clangd/SemanticHighlighting.h Mon Aug 26 01:38:45 2019
@@ -71,15 +71,15 @@ toSemanticHighlightingInformation(llvm::
/// Return a line-by-line diff between two highlightings.
/// - if the tokens on a line are the same in both hightlightings, this line is
/// omitted.
-/// - if a line exists in New but not in Old the tokens on this line are
+/// - if a line exists in New but not in Old, the tokens on this line are
/// emitted.
-/// - if a line does not exists in New but exists in Old an empty line is
+/// - if a line does not exist in New but exists in Old, an empty line is
/// emitted (to tell client to clear the previous highlightings on this line).
-/// \p NewMaxLine is the maximum line number from the new file.
+///
/// REQUIRED: Old and New are sorted.
std::vector<LineHighlightings>
diffHighlightings(ArrayRef<HighlightingToken> New,
- ArrayRef<HighlightingToken> Old, int NewMaxLine);
+ ArrayRef<HighlightingToken> Old);
} // namespace clangd
} // namespace clang
Modified: clang-tools-extra/trunk/clangd/test/semantic-highlighting.test
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/test/semantic-highlighting.test?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/test/semantic-highlighting.test (original)
+++ clang-tools-extra/trunk/clangd/test/semantic-highlighting.test Mon Aug 26 01:38:45 2019
@@ -92,7 +92,12 @@
{"jsonrpc":"2.0","method":"textDocument/didChange","params":{"textDocument":{"uri":"test:///foo.cpp","version":2},"contentChanges": [{"range":{"start": {"line": 0,"character": 10},"end": {"line": 1,"character": 10}},"rangeLength": 11,"text": ""}]}}
# CHECK: "method": "textDocument/semanticHighlighting",
# CHECK-NEXT: "params": {
-# CHECK-NEXT: "lines": [],
+# CHECK-NEXT: "lines": [
+# CHECK-NEXT: {
+# CHECK-NEXT: "line": 1,
+# CHECK-NEXT: "tokens": ""
+# CHECK-NEXT: }
+# CHECK-NEXT: ],
# CHECK-NEXT: "textDocument": {
# CHECK-NEXT: "uri": "file:///clangd-test/foo.cpp"
# CHECK-NEXT: }
Modified: clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp?rev=369884&r1=369883&r2=369884&view=diff
==============================================================================
--- clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp (original)
+++ clang-tools-extra/trunk/clangd/unittests/SemanticHighlightingTests.cpp Mon Aug 26 01:38:45 2019
@@ -18,6 +18,9 @@ namespace clang {
namespace clangd {
namespace {
+MATCHER_P(LineNumber, L, "") { return arg.Line == L; }
+MATCHER(EmptyHighlightings, "") { return arg.Tokens.empty(); }
+
std::vector<HighlightingToken>
makeHighlightingTokens(llvm::ArrayRef<Range> Ranges, HighlightingKind Kind) {
std::vector<HighlightingToken> Tokens(Ranges.size());
@@ -92,9 +95,10 @@ void checkDiffedHighlights(llvm::StringR
{LineTokens.first, LineTokens.second});
std::vector<LineHighlightings> ActualDiffed =
- diffHighlightings(NewTokens, OldTokens, NewCode.count('\n'));
+ diffHighlightings(NewTokens, OldTokens);
EXPECT_THAT(ActualDiffed,
- testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting));
+ testing::UnorderedElementsAreArray(ExpectedLinePairHighlighting))
+ << OldCode;
}
TEST(SemanticHighlighting, GetsCorrectTokens) {
@@ -463,9 +467,8 @@ TEST(SemanticHighlighting, GeneratesHigh
std::atomic<int> Count = {0};
void onDiagnosticsReady(PathRef, std::vector<Diag>) override {}
- void onHighlightingsReady(PathRef File,
- std::vector<HighlightingToken> Highlightings,
- int NLines) override {
+ void onHighlightingsReady(
+ PathRef File, std::vector<HighlightingToken> Highlightings) override {
++Count;
}
};
@@ -574,17 +577,6 @@ TEST(SemanticHighlighting, HighlightingD
R"(
$Class[[A]]
$Variable[[A]]
- $Class[[A]]
- $Variable[[A]]
- )",
- R"(
- $Class[[A]]
- $Variable[[A]]
- )"},
- {
- R"(
- $Class[[A]]
- $Variable[[A]]
)",
R"(
$Class[[A]]
@@ -608,6 +600,32 @@ TEST(SemanticHighlighting, HighlightingD
checkDiffedHighlights(Test.OldCode, Test.NewCode);
}
+TEST(SemanticHighlighting, DiffBeyondTheEndOfFile) {
+ llvm::StringRef OldCode =
+ R"(
+ $Class[[A]]
+ $Variable[[A]]
+ $Class[[A]]
+ $Variable[[A]]
+ )";
+ llvm::StringRef NewCode =
+ R"(
+ $Class[[A]] // line 1
+ $Variable[[A]] // line 2
+ )";
+
+ Annotations OldTest(OldCode);
+ Annotations NewTest(NewCode);
+ std::vector<HighlightingToken> OldTokens = getExpectedTokens(OldTest);
+ std::vector<HighlightingToken> NewTokens = getExpectedTokens(NewTest);
+
+ auto ActualDiff = diffHighlightings(NewTokens, OldTokens);
+ EXPECT_THAT(ActualDiff,
+ testing::UnorderedElementsAre(
+ testing::AllOf(LineNumber(3), EmptyHighlightings()),
+ testing::AllOf(LineNumber(4), EmptyHighlightings())));
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list