Page MenuHomeFreeBSD

units(1): Refactor the manual and update usage information
Needs ReviewPublic

Authored by 0mp on Jan 25 2019, 10:06 PM.

Details

Reviewers
bcr
eadler
Group Reviewers
manpages
docs
Summary

units.1:

  • Change the description to a more descriptive "conversion calculator".
  • Sort options.
  • Split the description into sections to make it easier to navigate the manual page.
  • Improve the description of various options.
  • Document the default value of the output format.
  • Use more mdoc macros for better readability.
  • Document the behavior of the PATH environmental variable.
  • Improve examples.
  • Add sections: EXIT STATUS, DIAGNOSTICS, and HISTORY.
  • Document that units(1) cannot convert negative values and it handles long unit lists poorly.

units.c:

  • Remove trailing whitespace.
  • Sort longopts elements.
  • Print usage instead of the path to the units file as documented when -V is specified.
Test Plan
$ igor ./units.1
$ mandoc -Tlint ./units.1
$ man ./units.1
$ mandoc -Thtml ./units.1 > /tmp/a.hmtl && firefox /tmp/a.html

Diff Detail

Repository
rS FreeBSD src repository
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 22270
Build 21470: arc lint + arc unit

Event Timeline

0mp created this revision.Jan 25 2019, 10:06 PM
0mp added inline comments.Jan 26 2019, 5:06 AM
usr.bin/units/units.1
42

Also, I found out in the commit log that this option can be supplied arbitrary number of times to load many files. Maybe we should include this information in the manual.

0mp updated this revision to Diff 53421.Jan 30 2019, 2:20 PM

I'll update the main comment with the description of the change.

0mp edited the summary of this revision. (Show Details)Jan 30 2019, 2:31 PM
0mp added reviewers: Src Committers, docs.
0mp added a subscriber: gjb.Jan 30 2019, 2:34 PM
0mp edited the test plan for this revision. (Show Details)Jan 30 2019, 2:48 PM
bcr added a comment.Jan 30 2019, 2:54 PM

I'd be OK with the manual page changes. At least I'm not seeing anything wrong with them. I guess we'll have to wait until a src committer has had a closer look at it.

I'd also like to see someone else's opinion on making whitespace changes.

usr.bin/units/units.c
734

I think having the 'type' of file in the usage statement was more helpful. Without it, I have to go refer to the man page to remember which file I specify with -f vs -H.

Although, since -H is ignores, I think it is safe to remove from usage(), and then the change to -f is less of an issue.

807

Not having an actual version number here seems strange. Maybe we should give it one?

808

You have accidentally changed the behaviour here.

Previously, if you ran units -V, it returned:
FreeBSD units
/usr/share/misc/definitions.units

Which I admit is running -U without having asked for it, but it is what the units command does today.
With your change, it will spit out usage().

So, at a minimum, I'd change this from fallthrough to exit();, but I am a little concerned about changing the behaviour here.

avg added a subscriber: avg.

I consider using the whole "Src committers" group as a reviewer to be inappropriate.
Especially for a change in a minor utility.

avg removed a subscriber: avg.Feb 25 2019, 4:25 PM