[PATCH] D81907: [llvm-objcopy] Fix help text for install-name-tool
James Henderson via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 16 01:04:25 PDT 2020
jhenderson added a comment.
llvm-strip doesn't allow a positional argument output either, as it can take multiple input files. This looks like a regression in the help text since an older version I have kicking around. Could you fix it for both (and update the patch description), please? You should also expand the help message test for both llvm-strip and llvm-objcopy to test that the full string has been matched.
================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:398
StringRef ToolName) {
- OptTable.PrintHelp(OS, (ToolName + " input [output]").str().c_str(),
+ StringRef Output = ToolName == "llvm-install-name-tool" ? "" : " [output]";
+ OptTable.PrintHelp(OS, (ToolName + " input" + Output).str().c_str(),
----------------
Rather than relying on a string for the name, I wonder if this code would be more robust with an enum that represents the three different tools, given that there's always potential for more front ends to be added. You would probably still want to pass in the tool name, but that could just be whatever the executable is called, possibly. For example, if the tool was llvm_install_name_tool, it would print that as the name in the help text message, rather than the current llvm-install-name-tool.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D81907/new/
https://reviews.llvm.org/D81907
More information about the llvm-commits
mailing list