Page MenuHomeFreeBSD

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

Authored by 0mp on Jan 25 2019, 10:06 PM.
Tags
None
Referenced Files
F105968298: D18977.diff
Mon, Dec 23, 6:10 AM
Unknown Object (File)
Oct 30 2024, 6:39 PM
Unknown Object (File)
Oct 6 2024, 10:51 AM
Unknown Object (File)
Sep 26 2024, 12:32 PM
Unknown Object (File)
Sep 24 2024, 12:57 PM
Unknown Object (File)
Sep 24 2024, 6:26 AM
Unknown Object (File)
Sep 21 2024, 2:29 PM
Unknown Object (File)
Sep 17 2024, 6:03 PM

Details

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.
  • Update the documentation of the -V flag to match the implementation. units(1) prints its version and the units data file instead of its version and usage information.

units.c:

  • Remove trailing whitespace.
  • Sort longopts elements.
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 - subversion
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 22170
Build 21385: arc lint + arc unit

Event Timeline

usr.bin/units/units.1
44

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.

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

0mp added reviewers: Src Committers, docs.

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.

810

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

811

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.

Sync with the latest head

  • Remove --history from synopsis and usage message
0mp marked an inline comment as done.Sep 22 2019, 1:39 PM
0mp added inline comments.
usr.bin/units/units.c
734

I removed -H from usage and synopsis.

0mp marked an inline comment as done.

Remove whitespace changes in units.c

Mark some comments as resolved.

usr.bin/units/units.c
810

Sure. Any ideas? I am not sure what we can show here.

811

I am aware of this change. I did it on purpose because that is how -V is documented in the manual.

I am not sure whether the manual has to be corrected or the program..

Change behaviour of --version to just print the version and exit.

usr.bin/units/units.c
734

I changed -f filename to -f unitsfile.

811

I spoke with @bcr and we think that returning just the version is the best idea and is the least confusing.

Keep -f flag argument consistent across manpage and usage

Do not change --version behavior. Just update the manual to match the current implementation.

@bcr, I've removed the controversial bits from the units.c file that were changing the program behavior. Now this patch is only changing the documentation.

Could I ask for your approval to commit it now?

Looks good to me, yes.

This revision is now accepted and ready to land.Feb 3 2020, 3:08 PM