Page MenuHomePhabricator

[clangd] Send highlighting diff beyond the end of the file.
ClosedPublic

Authored by hokein on Aug 21 2019, 9:11 AM.

Diff Detail

Repository
rL LLVM

Event Timeline

hokein created this revision.Aug 21 2019, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 21 2019, 9:11 AM
jvikstrom accepted this revision.Aug 22 2019, 1:42 AM

It feels a bit strange to be sending highlighting (even if they are empty) beyond eof. But I guess the proposal does not specify this and it would make life for the vscode extension (much) simpler.
Theia also does not crash from this and if we apply decorations to vscode outside the file nothing happens as well so I guess it should be fine. (even when vscode finally implements this feature into lsp)

Just had the comment about comparing HighlightigLine objects instead of using matchers but up to you.

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
625 ↗(On Diff #216417)

Maybe create the HighlightingLine objects for Line 3 and 4 and add them directly in the UnorderedElementsAre because then we don't have to create the matchers.

This revision is now accepted and ready to land.Aug 22 2019, 1:42 AM
hokein marked an inline comment as done.Aug 22 2019, 2:53 AM
hokein added inline comments.
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
625 ↗(On Diff #216417)

we prefer to use gtest matchers in the unittest as they are easier to understand, and provide better error messages (when the tests fail).

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 26 2019, 1:37 AM