sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang-tools-extra/clangd/Protocol.cpp:425
+const llvm::StringLiteral ExecuteCommandParams::CLANGD_APPLY_TWEAK =
+ "clangd.applyCodeAction";
+
----------------
tweak or applyTweak
================
Comment at: clang-tools-extra/clangd/Protocol.h:634
+struct TweakArgs {
+ URIForFile file;
----------------
comment here?
================
Comment at: clang-tools-extra/clangd/SourceCode.h:59
+/// care to avoid comparing the result with expansion locations.
+llvm::Expected<SourceLocation> sourceLocationInMainFile(const SourceManager
&SM,
+ Position P);
----------------
(can we add a unit test for this function?)
================
Comment at: clang-tools-extra/clangd/refactor/Tweak.cpp:27
+std::vector<std::unique_ptr<Tweak>> prepareTweaks(const Tweak::Selection &S) {
+#ifndef NDEBUG
+ {
----------------
ilya-biryukov wrote:
> Please note I added these assertions here.
>
> It feels weird to traverse twice on every call to `prepareTweaks`, but that's
> the simplest option I could come up with.
Looks fine - you could consider extracting to a separate function
`validateRegistry` or so, but up to you
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56267/new/
https://reviews.llvm.org/D56267
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
