[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

llvm-svn: 369884
This commit is contained in:
Haojian Wu 2019-08-26 08:38:45 +00:00
parent 72ca5f3694
commit 0a6000f2cb
8 changed files with 57 additions and 45 deletions

View File

@ -1195,7 +1195,7 @@ bool ClangdLSPServer::shouldRunCompletion(
}
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::onHighlightingsReady(
}
// 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)});

View File

@ -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>).

View File

@ -73,19 +73,10 @@ struct UpdateIndexCallbacks : public ParsingCallbacks {
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));
});
}

View File

@ -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

View File

@ -339,9 +339,11 @@ takeLine(ArrayRef<HighlightingToken> AllTokens,
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<HighlightingToken> New,
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);

View File

@ -71,15 +71,15 @@ toSemanticHighlightingInformation(llvm::ArrayRef<LineHighlightings> Tokens);
/// 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

View File

@ -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: }

View File

@ -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::StringRef OldCode, llvm::StringRef NewCode) {
{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, GeneratesHighlightsWhenFileChange) {
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;
}
};
@ -569,17 +572,6 @@ TEST(SemanticHighlighting, HighlightingDiffer) {
$Class[[A]]
^$Variable[[AA]]
$Variable[[A]]
)"},
{
R"(
$Class[[A]]
$Variable[[A]]
$Class[[A]]
$Variable[[A]]
)",
R"(
$Class[[A]]
$Variable[[A]]
)"},
{
R"(
@ -608,6 +600,32 @@ TEST(SemanticHighlighting, HighlightingDiffer) {
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