simark added inline comments.
================
Comment at: clangd/SourceCode.h:24
-/// Turn a [line, column] pair into an offset in Code.
-size_t positionToOffset(llvm::StringRef Code, Position P);
+/// Turn a [line, column] pair into an offset in Code according to the LSP
+/// definition of a Position. If the character value is greater than the line
----------------
ilya-biryukov wrote:
> Reverting to line length for larger column numbers seems to be the wrong
> thing to do for content changes, even though LSP does not distinguish this
> case.
> The fundamental difference is that position provided as inputs to features
> are caret or selection positions and it makes sense for them to span
> non-existing columns if the editors allows to put the caret there. Text
> changes, on the other hand, should be generated from proper offsets inside a
> string and it does not make sense for their columns to be out of range.
> I suggest to add a param with default argument to the function to control
> this behavior (`bool AllowColumnsBeyondLineLength = true`).
>
Ok, when `AllowColumnsBeyondLineLength` is false and the character value
exceeds the line length, I now return an error.
================
Comment at: unittests/clangd/SourceCodeTests.cpp:37
+bool positionToOffsetCheck(StringRef Code, Position P, size_t ExpectedOffset)
+{
+ llvm::Expected<size_t> Result = positionToOffset(Code, P);
----------------
ilya-biryukov wrote:
> NIT: opening must be on the previous line
Oops, forgot to run clang-format this time.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D44673
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
