ilya-biryukov accepted this revision.
ilya-biryukov added a comment.
This revision is now accepted and ready to land.
LGTM! Many thanks, the new tests are much simpler to read!
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:31
+ llvm::StringRef Code = Test.code();
+ for (const auto &R : Test.ranges()) {
+ unsigned StartOffset = llvm::cantFail(positionToOffset(Code, R.start));
----------------
NIT: `Test.llvm::Annotations::ranges()` will return ranges with offsets.
Should simplify the code.
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:44
TEST(RenameTest, SingleFile) {
- struct Test {
- const char* Before;
- const char* After;
- } Tests[] = {
+ const char *Tests[] = {
// Rename function.
----------------
NIT: worth documenting that `^` points to the start position of the rename and
ranges point to the identifier that is being renamed.
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:44
TEST(RenameTest, SingleFile) {
- struct Test {
- const char* Before;
- const char* After;
- } Tests[] = {
+ const char *Tests[] = {
// Rename function.
----------------
ilya-biryukov wrote:
> NIT: worth documenting that `^` points to the start position of the rename
> and ranges point to the identifier that is being renamed.
NIT: maybe use `llvm::StringRef Tests[]` instead?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69890/new/
https://reviews.llvm.org/D69890
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
