[PATCH] D116385: [clangd] Code action for creating an ObjC initializer

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.

Thanks, and sorry for the long delay.



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:282
+       Anchor::Above}};
+  llvm::SmallVector<std::pair<std::string, Edit>> Edits;
+
----------------
Not sure I see the point of  using a SmallVector here instead of writing directly into Effect.ApplyEdits?


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:295
+
+  if (Impl) {
+    auto ImplReplacement = insertDecl(
----------------
Maybe a comment like:
```
// If we see the class implementation, add the initializer there too.
// FIXME: merging the edits is awkward, do this elsewhere
```

(I don't think we should fix it in this patch, but we should at some point)


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp:301
+
+    if (!SM.isWrittenInSameFile(Interface->getLocation(),
+                                Impl->getLocation())) {
----------------
nit: I'd invert the condition here, to avoid the "else" block being a double-negative


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D116385/new/

https://reviews.llvm.org/D116385