Page MenuHomeFreeBSD

Updated description of default usbdump(8) format
Needs ReviewPublic

Authored by jpb_jimby.name on Oct 18 2024, 4:58 PM.
Tags
None
Referenced Files
F104548803: D47178.diff
Sat, Dec 7, 8:34 PM
Unknown Object (File)
Wed, Dec 4, 10:58 AM
Unknown Object (File)
Tue, Dec 3, 10:54 PM
Unknown Object (File)
Fri, Nov 29, 10:14 PM
Unknown Object (File)
Fri, Nov 22, 10:58 PM
Unknown Object (File)
Tue, Nov 19, 1:31 PM
Unknown Object (File)
Fri, Nov 15, 2:54 AM
Unknown Object (File)
Nov 7 2024, 5:32 PM

Details

Reviewers
concussious.bugzilla_runbox.com
Group Reviewers
manpages
Summary

Note to reviewers:

  1. I have not developed any code using USB, so please review the descriptions of the format elements for the default format.
  1. There is a trailing space at the end of the format line after the Unix continuation character '\' (formatted as '\\\ '). The continuation character does not render correctly without the trailing space.
Test Plan

tested with mantra

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 60183
Build 57067: arc lint + arc unit

Event Timeline

jpb_jimby.name edited the test plan for this revision. (Show Details)
gperciva_tarsnap.com added inline comments.
usr.sbin/usbdump/usbdump.8
141–144

Two issues here:

  1. I think this should switch to .Bl -literal -compact

or possibly

.Bl -literal -offset indent -compact

although the line is long enough that I think the indent isn't worth it.

  1. I can't find the \\\ construct used anywhere else. I couldn't find anything in mdoc(7) about it, but https://mandoc.bsd.lv/mdoc/intro/escaping.html

says "never use the escape sequence \\ in any context". They say to use `\e' to get a backslash.

160

.Sq SUBM
indicates a USB submit.

(And the same thing for DONE)

Ok, will update these shortly. I'll also update the spelling of "isochronous" on line 186 that somehow slipped by me.

Check out mandoc_char for more context on special characters.

Updated description of default usbdump(8) format

Fixed .Bl and .Sq entries. Replaced '\\\' with '\e' per mandoc. Added a .Dq for timestamp entry. Update spelling of isochronous. Added .Sh HISTORY section.

Updated description of default usbdump(8) format

I am not familiar (seems like none of us are?) with the internals of USB. Please CC a SME who is working on USB. I have a suspicion all of these items in this output format list might be Defined Variables?

If SUBM and DONE indicate a USB transfer operation, are they Internal Commands (.Ic)?

Nice job on HISTORY, I really like HISTORY sections.

Looks good, thanks for the quick fixes!

I've reached out to the original submitter of the PR in Bugzilla 264125.

Please test patches before saying they look good.

.Bl is begin list. You're looking for .Bd -literal (begin display block), which requires a .Ed (end display block) after the test block is finished.

This revision now requires changes to proceed.Oct 22 2024, 12:40 AM

Updated description of default usbdump(8) format

Removed .Bl -literal -offset indent -compact

Tested with mantra
Test with mandoc -W error,stop usbdump.8
Tested with igor

This revision now requires changes to proceed.Oct 22 2024, 1:12 AM

Sorry I wasn't clear. The basic premise of Graham's idea about using a display block instead of an indented literal display is appropriate since this spans multiple lines. See my comment above for what I was suggesting, and you can read more about the syntax for manuals at mdoc(7).

Updated description of default usbdump(8) format

Added .Bd -literal with corresponding .Ed

Removed errant space on second line.

Tested with mantra
Test with mandoc -W error,stop usbdump.8
Tested with igor

Am I doing this wrong? Patch doesn't apply to main (git-arc.sh patch -c D47178). That also lets you consider the proposed commit message (git log/git show). You may need to squash your commits and/or rebase. Also, consider testing with the compiler's builtin linter (mandoc -T lint usbdump.8) instead of mandoc -W, I find it more informative and easier.

usbdump.8 was updated on 2024-10-21, a few days after this patch was submitted.

https://github.com/freebsd/freebsd-src/commit/f50d2fced24f4dbffc1cc414da48ea7ef7b84d1e

Yes, the patch needs to be updated, but it's understandable why it's out of date.

I am so sorry if it came off that way. I didn't understand if I was doing it correctly, or if the author didn't realize (like me, often). Author is a volunteer donating time and can take as much time as they need. I have reviews currently open that have been in progress for months, but my favorite thing is when people give me quick feedback with tips that help me, so I'm trying to do the same here.

Updated description of default usbdump(8) format

Trimmed lines to stay below 80 character limit for mandoc -T lint
Removed extraneous .Pp before .Bd -literal
Tested with mantra.

I liked your history section and isonchronous typo correction.

I liked your history section and isonchronous typo correction.

Thank you!

It seems to have gotten lost and is no longer part off this commit?

Alexander,

In the diff section, look for and click "Show All Lines".

All the preceding corrections are there, including "isochronous" and the history section.

The weird looking last set of changes was your suggestion to limit lines to 75 characters or less, so I chopped some lines leading to those diff entries. The content is the same, but the formatting is different.

Best,
Jim B.

Line folding is kinda a whole thing (no change suggested, just fyi):

First, roff asks nothing goes over 80 characters unless there's an emergency (see #4).

Second, in a standard bsd console, the bsd editor nvi uses 8 characters to the left of the screen, so you have 72 characters before wrapping occurs. So personally, I like to limit it there, because then it's a logical extension of self-hosting: our doc is written with our tooling in mind. Of course, people are welcome to use whatever they like, but this is BSD culturally conscious best practice.

Third, the mdoc style guide for authors at mandoc.bsd.lv teaches that when lines are wrapped, they're as wrapped in as close to the middle as possible, for many reasons.

Fourth, keep in mind not breaking up phrases, or breaking them up in a logical way for grep, since grep searches a line at a time.

So, afaik those are all the main considerations when choosing how to wrap lines.