[PATCH] D82923: introducing llvm-libtool-darwin

[James Henderson via Phabricator via llvm-commits]

jhenderson added inline comments.


================
Comment at: llvm/test/tools/llvm-libtool-darwin/hide-unrelated-options.test:1
+## This test checks that unrelated options are hidden in help text.
+
----------------
This is probably fine, but an alternative that might be more consistent is to expand help-message.test to show that the option categories that should be supported are printed (by checking the e.g. "Color options:" text), and that the unrelated options aren't (by similarly checking that the header for them isn't included). There are some examples of this for some tools like llvm-size. You might also want to add a --help-list test case as in llvm-size's help.test.


================
Comment at: llvm/tools/llvm-libtool-darwin/llvm-libtool-darwin.cpp:18
+
+static cl::opt<std::string> OutputFile("output",
+                                       cl::desc("Specify output filename"),
----------------
smeenai wrote:
> sameerarora101 wrote:
> > smeenai wrote:
> > > As far as I can see, cctools libtool doesn't support the `-output` spelling, only `-o`. Is there any reason for us to support it?
> > Yup, that is true. I was just looking at other llvm tools and they have `-output` in addition to `-o`. So I thought of adding both. I can remove `-output` if you guys prefer that?
> I'd prefer to remove it, to mimic cctools libtool's interface as closely as possible.
No issues removing -output from me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82923