This is an archive of the discontinued LLVM Phabricator instance.

[llvm-ar] Correct help text
ClosedPublic

Authored by bd1976llvm on Jun 25 2018, 6:27 PM.

Details

Summary

Corrected and simplified the help text.
It was clearly too difficult to maintain before (see e.g. @227296) making it simpler and more consistent it should help people keep it up to date.

  • I took the description from https://llvm.org/docs/CommandGuide/llvm-ar.html.
  • I removed the "count" positional argument only used by removed "N" modifier.
  • It is pretty clear from the modifier description which modifiers apply to which operations, so I removed the (currently incorrect) list of modifiers from each operation.
  • Removed the distinction between generic and specific modifiers. This is artificial anyway - for example the "c" modifier is currently listed as generic but only applies some operations.
  • Used "archive" rather than "library" - more correct (library is a subset of archive).
  • Used "files" rather than "members" - for consistency with the rest of the help text.
  • "s" is both an operation key letter and a modifier.
  • Added all the accepted modifiers to the list.
  • Used a consistent rule that if something is in square brackets in the usage text then it is written with square brackets in the rest of the help text.

Diff Detail

Repository
rL LLVM

Event Timeline

bd1976llvm created this revision.Jun 25 2018, 6:27 PM
bd1976llvm edited the summary of this revision. (Show Details)
bd1976llvm added subscribers: jhenderson, edd, andrewng and 2 others.

Having thought about this I have decided to try to simplify the help text as much as possible to aid maintenance.

Updated the diff and the summary.

ruiu added inline comments.Jun 28 2018, 4:35 AM
tools/llvm-ar/llvm-ar.cpp
68 ↗(On Diff #152879)

llvm-ar is not only similar but is actually an ar command, no?

72 ↗(On Diff #152879)

Half-open <.

100 ↗(On Diff #152879)

Only this line starts with an uppercase letter.

updated diff to address review comments

bd1976llvm marked 2 inline comments as done.Jun 28 2018, 5:21 AM
bd1976llvm added inline comments.
tools/llvm-ar/llvm-ar.cpp
68 ↗(On Diff #152879)

I removed the description entirely.

72 ↗(On Diff #152879)

This is a convention used in the gnu tools help text. It is mean't to convey that if invoked with just -M then the program enters an interactive mode where you enter the script line by line but if you pipe in a script then the program reads the script non-interactively. See: https://sourceware.org/binutils/docs/binutils/ar-scripts.html#ar-scripts. Use of MRI scripts should be rare and they are only supported for compatibility with a long defunct toolchain.

100 ↗(On Diff #152879)

Thanks!

bd1976llvm marked 2 inline comments as done.Jun 28 2018, 6:11 AM
MaskRay accepted this revision.Jul 26 2018, 11:06 PM
This revision is now accepted and ready to land.Jul 26 2018, 11:06 PM
This revision was automatically updated to reflect the committed changes.

This wasn't committed in time for the 7.0 release but I wonder if rL338703 + rL338709 should be put on the release branch? The current output is slightly embarrassing.

hans added a comment.Aug 3 2018, 3:12 AM

This wasn't committed in time for the 7.0 release but I wonder if rL338703 + rL338709 should be put on the release branch? The current output is slightly embarrassing.

Merged both in r338840, thanks.