[PATCH] D137817: [clangd] Improve action `RemoveUsingNamespace` for user-defined literals

[Tom Praschan via Phabricator via cfe-commits]

tom-anders added a comment.

This looks really cool :)



================
Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:133
+  std::string Directives;
+  for (auto &Using : UsingToAdd)
+    Directives += llvm::formatv("using namespace {0};\n", Using);
----------------
nit: `const auto`


================
Comment at: clang-tools-extra/clangd/refactor/tweaks/RemoveUsingNamespace.cpp:249
 
+  llvm::sort(UsingToAdd);
+  UsingToAdd.erase(std::unique(UsingToAdd.begin(), UsingToAdd.end()),
----------------
If you're sorting and removing duplicates in the end anyway, maybe [[ https://llvm.org/doxygen/classllvm_1_1StringSet.html | llvm::StringSet ]] would be a better fit instead of `vector<string>`?


================
Comment at: clang-tools-extra/clangd/unittests/tweaks/RemoveUsingNamespaceTests.cpp:286
+      using namespace ns1::ns2;
       int main() { 1.5_w; }
     )cpp"}};
----------------
Your code also handles cases where there are multiple inline namespaces involved, right? If so, it would be a good idea also test this here (i.e. add ns3 with another user-defined literal)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137817