[PATCH] D44673: Make positionToOffset return llvm::Expected

This revision was automatically updated to reflect the committed changes. Closed by commit rCTE328100: Make positionToOffset return llvm::Expectedsize_t (authored by simark, committed by ). Changed prior to commit: https://reviews.llvm.org/D44673?vs=139292=139297#toc Repository: rL LLVM

[PATCH] D44673: Make positionToOffset return llvm::Expected

This revision was automatically updated to reflect the committed changes. Closed by commit rL328100: Make positionToOffset return llvm::Expectedsize_t (authored by simark, committed by ). Herald added a subscriber: llvm-commits. Repository: rL LLVM https://reviews.llvm.org/D44673 Files:

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)}); simark wrote: > ilya-biryukov wrote: > > NIT: unnecessary empty line > In

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark marked 6 inline comments as done. simark added inline comments. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File, {tooling::Range(*Begin, *End - *Begin)}); ilya-biryukov wrote: > NIT: unnecessary empty

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark updated this revision to Diff 139292. simark marked 3 inline comments as done. simark added a comment. Remove spaces, add fixmes Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44673 Files: clangd/ClangdServer.cpp clangd/SourceCode.cpp clangd/SourceCode.h

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov added inline comments. Comment at: clangd/SourceCode.h:41 /// Turn an offset in Code into a [line, column] pair. Position offsetToPosition(llvm::StringRef Code, size_t Offset); We should be consistent for all functions inside this fail. They

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov accepted this revision. ilya-biryukov added a comment. This revision is now accepted and ready to land. LGTM. See the empty line NITS, though. Comment at: clangd/ClangdServer.cpp:199 +return End.takeError(); + + return formatCode(Code, File,

[PATCH] D44673: Make positionToOffset return llvm::Expected

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

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark updated this revision to Diff 139157. simark marked 9 inline comments as done. simark added a comment. Herald added a subscriber: mgorny. Address review comments Repository: rCTE Clang Tools Extra https://reviews.llvm.org/D44673 Files: clangd/ClangdServer.cpp

[PATCH] D44673: Make positionToOffset return llvm::Expected

ilya-biryukov added inline comments. Comment at: clangd/ClangdServer.cpp:196 +return Begin.takeError(); + + llvm::Expected End = positionToOffset(Code, Rng.end); NIT: maybe remove empty lines? they don't seem to add much to readability, since when prev

[PATCH] D44673: Make positionToOffset return llvm::Expected

simark created this revision. Herald added subscribers: cfe-commits, ioeric, jkorous-apple, ilya-biryukov, klimek. simark added a reviewer: ilya-biryukov. To implement incremental document syncing, we want to verify that the ranges provided by the front-end are valid. Currently,