[PATCH] D36574: [refactor] add clang-refactor tool with initial testing support and local-rename action

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.


================
Comment at: tools/clang-refactor/ClangRefactor.cpp:60
+      : SubCommand(Name, Description), Action(&Action) {
+    Sources = llvm::make_unique<cl::list<std::string>>(
+        cl::Positional, cl::ZeroOrMore, cl::desc("<source0> [... <sourceN>]"),
----------------
arphaman wrote:
> ioeric wrote:
> > I think you would get a conflict of positional args when you have multiple sub-command instances.
> > 
> > Any reason not to use `clang::tooling::CommonOptionsParser` which takes care of sources and compilation options for you? 
> Not from my experience, I've tried multiple actions it seems like the right arguments are parsed for the right subcommand. It looks like the cl option parser looks is smart enough to handle identical options across multiple subcommands.
I agree that using `CommonOptionsParser` would be preferable, but right now it doesn't work well with subcommands. I will create a followup patch that improves subcommand support in `CommonOptionsParser` and uses them in clang-refactor when this patch is accepted.


Repository:
  rL LLVM

https://reviews.llvm.org/D36574