ilya-biryukov added inline comments.
================
Comment at: clang-tools-extra/clangd/tool/ClangdMain.cpp:269
+ cat(Features),
+ desc("Enable cross-file rename feature."),
+ init(false),
----------------
Could you document that the feature is highly experimental and may lead to
broken code or incomplete renames for the time being?
Also, maybe make it hidden for now?
At least until we have basic validation of ranges from the index. In the
current state we can easily produce edits to unrelated parts of the file...
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:277
+ // - file on VFS for bar.cc;
+ Annotations MainCode("class [[Fo^o]] {};");
+ auto MainFilePath = testPath("main.cc");
----------------
Thinking whether we can encode these transformations in a nicer way?
If we could convert the contents dirty buffer and replacements to something
like:
```
class [[Foo |-> newName]] {};
```
Just a proposal, maybe there's a better syntax here.
We can easily match strings instead of matching ranges in the tests. This has
the advantage of having textual diffs in case something goes wrong - much more
pleasant than looking at the offsets in the ranges.
WDYT?
================
Comment at: clang-tools-extra/clangd/unittests/RenameTests.cpp:302
+ testing::UnorderedElementsAre(
+ PairMatcher(Eq(FooPath),
+ EqualFileEdit(FooDirtyBuffer, FooCode.ranges())),
----------------
Instead of defining custom matchers, could we convert to standard types and use
existing gtest matchers?
Something like `std::vector<std::pair</*Path*/std::string, Edit>>` should work
just fine.
Huge advantage we'll get is better output in case of errors.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D69263/new/
https://reviews.llvm.org/D69263
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
