[PATCH] D56267: [clangd] Interfaces for writing code actions

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Jan 25 08:49:13 PST 2019


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





More information about the cfe-commits mailing list