Man pages: tighter documenting of --format fields

[edsantiago]

Initial impetus was #20958 (ps --format .Label abc). This is
a complicated solution to a simple-seeming problem.

The problem: .Label is a cobra function, something I did not
know about nor handle.

Solution: recognize cobra functions. Switch to __complete,
not __completeNoDesc, so we can see the number of arguments
required. Invent new man-page format for documenting functions.
And, finally, start enforcing how functions (and cobra structs)
are documented.

This discovered a never-used completion function, .Recycle(),
in podman-events. Remove it.

Signed-off-by: Ed Santiago [email protected]

None

[edsantiago]

No hurry. I think this PR should sit and simmer until Paul has a chance to look at it. I just wanted to get it submitted because it's been a lot of my work over the past two weeks.

[edsantiago]

This is the new convention for documenting cobra functions. It merits careful scrutiny.

[edsantiago]

This huge PR, just for this....

[Luap99]

should we use <STRING> or *string* as syntax to better display that this is a mandatory argument?

[edsantiago]

That's what I was going for with CAPS, but yours seems more consistent with our conventions.

Angle brackets, probably not, because they'd need to be escaped and that's too hard to enforce. Asterisks (italics) show up only in generated HTML, not in man pages (at least not on my laptop, AFAICT). See attached image; the italics don't show up well in CAPS, but they are prominent in lowercase.

If we can find a way to highlight *italics* in the terminal man page view, I'd like to go with that approach. Otherwise, slight preference for CAPS.

[Luap99]

I am fine with CAPS

[edsantiago]

Followup: go-md2man seems to handle italics-in-tables just fine, and produces what looks like sane *roff:

\&.Recycle \fIpath\fP \fIbool\fP    [unimplemented]

...but man -l is not respecting that.

Italics and bold are possible in tables -- see systemd.unit(5) -- I just can't figure out what special magic is needed, or what the difference is between the table in systemd.unit.5 and the podman one. Giving up for now, waiting for inspiration.

[edsantiago]

I give up. Man page highlights are inconsistent. The following groff:

.SH DESCRIPTION
.PP
Monitor and print events that occur in Podman. Each event includes a timestamp,
a type, a status, name (if applicable), and image (if applicable).  The default logging
mechanism is \fIjournald\fP\&. This can be changed in containers.conf by changing the \fB\fCevents_logger\fR
value to \fB\fCfile\fR\&.  Only \fB\fCfile\fR and \fB\fCjournald\fR are accepted. A \fB\fCnone\fR logger is also
available, but this logging mechanism completely disables events; nothing is reported by
\fB\fCpodman events\fR\&.

.PP
By default, streaming mode is used, printing new events as they occur.  Previous events can be listed via \fB\fC--since\fR and \fB\fC--until\fR\&.

...produces the following display:

Note the proliferation of \f<whatever> in the source. The only ones that actually show up in the terminal view are the ones with -- (the options). This is too obscure for me.

[Luap99]

Overall looks good, although I have not looked at the perl code just the docs and test cases.

[Luap99]

should we use <STRING> or *string* as syntax to better display that this is a mandatory argument?

[edsantiago]

The man page confusion was 100% PEBKAC: I have had am working on tweaking a somewhat complicated set of $MANPAGER and $LESS_TERMCAP_xx settings. For normal people, the highlights should work, so I'm going with 'arg` and added tests to check and enforce.

This is too much to ask for full review. My recommendation is to focus on the man-page changes, see if those look reasonable, and then skip to the new error-message checks and see if those are acceptable. As in, if you submitted a PR and this script blew up at you with one of those messages, would you know what to do or at least have a starting point?

[Luap99]

this look like a bogus function that is never called or used.

Mind removing it with

diff --git a/libpod/events/events.go b/libpod/events/events.go
index bcda07e06..18f531469 100644
--- a/libpod/events/events.go
+++ b/libpod/events/events.go
@@ -54,13 +54,6 @@ func NewEvent(status Status) Event {
        }
 }
 
-// Recycle checks if the event log has reach a limit and if so
-// renames the current log and starts a new one.  The remove bool
-// indicates the old log file should be deleted.
-func (e *Event) Recycle(path string, remove bool) error {
-       return errors.New("not implemented")
-}
-
 // ToJSONString returns the event as a json'ified string
 func (e *Event) ToJSONString() (string, error) {
        b, err := json.Marshal(e)

Otherwise I can submit a PR to remove it because there should really be no point in documenting broken stuff that will never help any user.

[edsantiago]

Done. Required small tweak to regression test. I updated the commit message.

.Recycle() dates back to #2527 in 2019. Tagging @baude - that was your code, it looks like a placeholder that got orphaned. If you see a strong reason to leave that, please let me know.

[edsantiago]

Brent has enough on his hands this week. I'm gonna say if there's ever a need for .Recycle() it can be added (and actually implemented) then. @containers/podman-maintainers PTAL, I consider this ready to merge.

[mheon]

LGTM

[giuseppe]

/lgtm

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago, giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• OWNERS [edsantiago,giuseppe]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment