[PATCH] D81907: [llvm-objcopy] Fix help text

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added inline comments.


================
Comment at: llvm/tools/llvm-objcopy/CopyConfig.cpp:403
 static void printHelp(const opt::OptTable &OptTable, raw_ostream &OS,
-                      StringRef ToolName) {
-  OptTable.PrintHelp(OS, (ToolName + " input [output]").str().c_str(),
+                      StringRef ToolName, ToolType ToolEnum) {
+  StringRef HelpText;
----------------
sameerarora101 wrote:
> alexshap wrote:
> > sameerarora101 wrote:
> > > alexshap wrote:
> > > > jhenderson wrote:
> > > > > alexshap wrote:
> > > > > > nit: ToolType ToolEnum -> ToolType Tool
> > > > > > 
> > > > > > ToolName can be derived from ToolType, we probably don't need to pass it as an argument.
> > > > > > 
> > > > > > 
> > > > > How would you and others feel about passing in the real executable name and printing that instead? It was an idea I had, but didn't suggest it before.
> > > > I would be fine with that
> > > @jhenderson What do you mean by "real executable name"? So what should I be doing here now? We keep the ToolName? Sorry for not understanding what you guys meant exactly 😔 
> > > 
> > > Currently I have made the change ToolType ToolEnum -> ToolType Tool. Not sure about ToolName yet?
> > @sameerarora101, sorry about the confusion, if I understand correctly @jhenderson is referring to the global variable ToolName (initialized in llvm-objcopy.cpp) or its basename. My first thought was that it might be useful but now i think that there are some undesirable consequences of this approach - e.g. some unexpected changes to the help message if the binary is moved around. It's already the case to some extent (because that's how we differentiate objcopy vs strip vs install-name-tool), but now it'll happen more frequently + it would create a dependency on the global variable from another file. I don't insist on any particular approach here, it feels like it's simpler to get rid of the duplication which I mentioned above and keep the old behavior.
> oh ok I think I understand now. Thanks a lot for clarifying @alexshap . I'll update the diff to remove duplication.
Thanks for the update. I was suggesting something like the following output:

```
my-renamed-objcopy-11.exe [options] input [output]
```

etc, i.e. whatever the real file name is, but I'm not sure about it myself either way, so I'm happy not to bother.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D81907