tom-anders wrote: Thanks for the feedback!
> Thanks, the approach in this patch looks pretty good to me. > > My only feedback is to encapsulate the "hard coding" into a function like: > > ``` > Option<CodeActionResult::Rename> TryConvertToRename(const Diag *D, const Fix > *F) > ``` > > because I can imagine in the future coming across other diagnostics that are > conceptually renames that we may want to handle this way. :heavy_check_mark: > Regarding testing: I find lit-tests pretty hard to read and maintain; a nicer > alternative might be a gtest in `ClangdLSPServerTests` (e.g. see [this > one](https://searchfox.org/llvm/rev/23b233c8adad5b81e185e50d04356fab64c2f870/clang-tools-extra/clangd/unittests/ClangdLSPServerTests.cpp#198) > for testing call hierarchy incoming calls). Perfect, thanks for the hint! I had to adapt `LSPClient` a bit, it usually would flag every server->client call as an error, but in this case we actually expect a `workspace/applyEdit` after we trigger the renaming. https://github.com/llvm/llvm-project/pull/78454 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
