Page MenuHomeFreeBSD
Differential D18977

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

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
View All 81 Files
Subscribers
allanjude
gjb
imp
jonathan

Details

Reviewers
bcr
eadler
Group Reviewers
manpages
docs
Commits
rS357449: units(1): Refactor the manual page and update usage information
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 26620
Build 24998: arc lint + arc unit

Event Timeline

0mp created this revision.Jan 25 2019, 10:06 PM
Herald added a reviewer: manpages. · View Herald TranscriptJan 25 2019, 10:06 PM
Herald added a subscriber: imp. · View Herald Transcript
Harbormaster completed remote builds in B22170: Diff 53235.Jan 25 2019, 10:06 PM
0mp added inline comments.Jan 26 2019, 5:06 AM
usr.bin/units/units.1
41

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.

Herald added a subscriber: jonathan. · View Herald TranscriptJan 30 2019, 2:20 PM
Harbormaster completed remote builds in B22270: Diff 53421.Jan 30 2019, 2:20 PM
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.

allanjude added a subscriber: allanjude.Feb 25 2019, 4:18 PM

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 removed a reviewer: Src Committers.Feb 25 2019, 4:24 PM
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
0mp updated this revision to Diff 62419.Sep 22 2019, 1:32 PM

Sync with the latest head

Harbormaster completed remote builds in B26619: Diff 62419.Sep 22 2019, 1:32 PM
0mp updated this revision to Diff 62420.Sep 22 2019, 1:39 PM
  • Remove --history from synopsis and usage message
Harbormaster completed remote builds in B26620: Diff 62420.Sep 22 2019, 1:39 PM
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 updated this revision to Diff 62421.Sep 22 2019, 1:41 PM
0mp marked an inline comment as done.

Remove whitespace changes in units.c

Harbormaster completed remote builds in B26621: Diff 62421.Sep 22 2019, 1:41 PM
0mp added a comment.Sep 22 2019, 1:42 PM

Mark some comments as resolved.

0mp added inline comments.Sep 22 2019, 1:46 PM
usr.bin/units/units.c
807

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

808

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..

0mp updated this revision to Diff 67611.Feb 1 2020, 11:30 AM

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

Harbormaster completed remote builds in B29090: Diff 67611.Feb 1 2020, 11:30 AM
0mp added inline comments.Feb 1 2020, 11:33 AM
usr.bin/units/units.c
734

I changed -f filename to -f unitsfile.

808

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

0mp updated this revision to Diff 67614.Feb 1 2020, 11:57 AM

Keep -f flag argument consistent across manpage and usage

Harbormaster completed remote builds in B29092: Diff 67614.Feb 1 2020, 11:58 AM
0mp edited the summary of this revision. (Show Details)Feb 3 2020, 10:46 AM
0mp updated this revision to Diff 67688.Feb 3 2020, 10:51 AM

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

Harbormaster completed remote builds in B29123: Diff 67688.Feb 3 2020, 10:51 AM
0mp edited the summary of this revision. (Show Details)Feb 3 2020, 10:54 AM

@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?

bcr accepted this revision as: manpages, bcr.Feb 3 2020, 3:08 PM

Looks good to me, yes.

This revision is now accepted and ready to land.Feb 3 2020, 3:08 PM
Closed by commit rS357449: units(1): Refactor the manual page and update usage information (authored by 0mp). · Explain WhyFeb 3 2020, 3:23 PM
This revision was automatically updated to reflect the committed changes.
0mp added a commit: rS357449: units(1): Refactor the manual page and update usage information.
0mp added a comment.Feb 3 2020, 3:23 PM

@bcr, thanks!

Revision Contents
Changeset List

PathSize
usr.bin/
units/
482 lines
44 lines

Diff 62420

View Options

usr.bin/units/units.1

Loading...
View Options

usr.bin/units/units.c

Loading...