sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Thanks for the fix! Just comment nits.
================
Comment at: clangd/DraftStore.cpp:85
+
+ // The difference between EndIndex and StartIndex gives the range length in
+ // bytes. However, the LSP range length is specified in UTF-16 code units
----------------
can we summarize this a bit?, e.g.
`// EndIndex and StartIndex are in bytes, but rangeLength is in UTF-16 units`
no need to explain exactly what the code is doing, just need to say why.
================
Comment at: clangd/SourceCode.cpp:70
-// Counts the number of UTF-16 code units needed to represent a string.
-// Like most strings in clangd, the input is UTF-8 encoded.
-static size_t utf16Len(StringRef U8) {
+// Counts the number of UTF-16 code units needed to represent a string (LSP
+// specifies string lengths in UTF-16 code units). Like most strings in clangd,
----------------
actually just moving this comment to the header would be great!
================
Comment at: clangd/SourceCode.cpp:73
+// the input is UTF-8 encoded.
+size_t lspLength(StringRef Code) {
// A codepoint takes two UTF-16 code unit if it's astral (outside BMP).
----------------
could you few simple assertions for this function (maybe ascii, BMP, astral) to
unittests/clangd/SourceCodeTests.cpp?
================
Comment at: clangd/SourceCode.h:26
+/// Calculate the length of Code according to the LSP specification.
+size_t lspLength(StringRef Code);
----------------
I think it would be helpful to mention here (only in the comment!) that this is
UTF-16 code units.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D53527
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
