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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
