Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
- Build Status
Buildable 36486 Build 36485: arc lint + arc unit
Event Timeline
Thanks for working on this! You're missing a number of aliases from the list. Please make sure they are included in the doc. Also, please use the text from the llvm-objcopy doc where the switch is common to the two tools.
llvm/docs/CommandGuide/llvm-strip.rst | ||
---|---|---|
3 | Delete the trailing '=' here. | |
10 | llvm-strip's usage is different to llvm-objcopy's. This follows llvm-objcopy's style. It should be something like :program:'llvm-strip' [*options*] *inputs...* (with back-ticks around llvm-strip instead of apostrophes, used due to not knowing how to escape them in Phabricator). | |
15 | This description should mention what happens by default, what the meaning of '-' is for input file names, and what happens for archive inputs (see the llvm-objcopy doc). | |
28 | This, and many of the other options use a more verbose/clearer description in the llvm-objcopy documentation for the same switch. I'd recommend you copy the text from llvm-objcopy's doc in each case, assuming it makes sense. | |
34–36 | This should be in the "ELF-specific" section. | |
41 | What about -h/--help? | |
48 | This should say something about what happens if there are multiple input files. | |
50–52 | Whilst not really ELF-specific, this is implemented only for ELF currently, so should be down in that section. | |
64 | This needs extra mark-up. See llvm-objcopy's documentation. | |
72 | This text isn't right. The switch has nothing to do with symbols. As with the other switches, use the text from llvm-objcopy. | |
82 | This should be grouped with --enable-deterministic-archives, like -D for --disable-deterministic archives. | |
86 | Also -V, right? | |
121 | Also -K? | |
llvm/tools/llvm-objcopy/StripOpts.td | ||
35–36 ↗ | (On Diff #212130) | Do this in a separate change. |
Happy to help :). Will do.
Also, please use the text from the llvm-objcopy doc where the switch is common to the two tools.
Yes, good idea. In fact, I'm going to move the common switches into a new file, say CommonOpts.td, and include "CommonOpts.td" at the top of ObjcopyOpts.td and StripOpts.td. That way we don't have the same switch in two different places each with different HelpText.
Incorporate feedback from @jhenderson.
Move common help options from ObjcopyOpts.td and StripOpts.td
into CommonOpts.td and CommonAliases.td which are included
in the former files.
Add HelpText to aliases which did not already have it.
The tabelgen option changes should be in their own review. I haven't tried to review them here.
llvm/docs/CommandGuide/llvm-strip.rst | ||
---|---|---|
16 | Perhaps rephrase this to make the behaviour clearer. E.g. "If no other stripping or remove options are specified, --strip-all will be enabled by default." | |
88 | Change this to "remove all debug sections". Specifying other switches could mean that the "only" is invalid. |
CC'ing @wolfgangp, due to D65787 adding a new switch. One of the two patches will need updating once the other lands.
Incorporate feedback from @jhenderson.
Add new switch --strip-sections. Thanks, James.
Delete the trailing '=' here.