[CI:DOCS] man-page checker: include --format (Go templates) #17443

edsantiago · 2023-02-08T22:35:16Z

Very belated successor to #14046.

I don't know why this is so important to me. Probably because we're doing a halfhearted sloppy job of documenting, and new options get added, and not documented, and that's just wrong.

I've given up on documenting internal structs. This iteration has a $Format_Exceptions table defined at the top of the xref script, enumerating a hardcoded defined set of podman commands and fields that should remain undocumented.

This iteration also forgives completely-undocumented formats. If podman-foo has a --format, but podman-foo.1.md does not list any valid fields, the script warns but does not fail. This at least is better than documenting a random mix of fields.

This version of the xref script is much slower: 10s vs 4. I think we can live with that in a CI-only script.

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

Document a few more --format (Go template) fields

Very belated successor to containers#14046. I don't know why this is so important to me. Probably because we're doing a halfhearted sloppy job of documenting, and new options get added, and not documented, and that's just wrong. I've given up on documenting internal structs. This iteration has a $Format_Exceptions table defined at the top of the xref script, enumerating a hardcoded defined set of podman commands and fields that should remain undocumented. This iteration also forgives completely-undocumented formats. If podman-foo has a --format, but podman-foo.1.md does not list *any* valid fields, the script warns but does not fail. This at least is better than documenting a random mix of fields. This version of the xref script is much slower: 10s vs 4. I think we can live with that in a CI-only script. Signed-off-by: Ed Santiago <[email protected]>

edsantiago · 2023-02-08T23:58:45Z

New warnings, as expected:

hack/xref-helpmsgs-manpages
xref-helpmsgs-manpages: 'podman image inspect': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-image-inspect.1.md
xref-helpmsgs-manpages: 'podman machine info': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-machine-info.1.md
xref-helpmsgs-manpages: 'podman machine inspect': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-machine-inspect.1.md
xref-helpmsgs-manpages: 'podman secret ls': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-secret-ls.1.md
xref-helpmsgs-manpages: 'podman system df': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-system-df.1.md
xref-helpmsgs-manpages: 'podman volume inspect': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-volume-inspect.1.md
xref-helpmsgs-manpages: 'podman volume ls': --format options are available through autocomplete, but are not documented in docs/source/markdown/podman-volume-ls.1.md

vrothberg

LGTM

openshift-ci · 2023-02-09T10:29:11Z

[APPROVALNOTIFIER] This PR is APPROVED

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

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,vrothberg]

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

edsantiago · 2023-02-09T11:50:57Z

hack/xref-helpmsgs-manpages

+# Table of exceptions for documenting fields in '--format {{.Foo}}'
+#
+# Autocomplete is wonderful, and it's even better when we document the
+# existing options. Unfortunately, sometimes internal structures get
+# exposed that are of no use to anyone and cannot be guaranteed. Avoid
+# documenting those. This table lists those exceptions. Format is:
+#
+#      foo       .Bar
+#
+# ...such that "podman foo --format '{{.Bar}}'" will not be documented.
+#
+my $Format_Exceptions = <<'END_EXCEPTIONS';
+# Deep internal structs; pretty sure these are permanent exceptions
+events       .Details
+history      .ImageHistoryLayer
+images       .ImageSummary
+network-ls   .Network
+
+# FIXME: this one, maybe? But someone needs to write the text
+machine-list    .Starting
+
+# No clue what these are. Some are just different-case dups of others.
+pod-ps  .Containers .Id .InfraId .ListPodsReport .Namespace
+ps      .Cgroup .CGROUPNS .IPC .ListContainer .MNT .Namespaces .NET .PIDNS .User .USERNS .UTS
+
+# I think .Destination is an internal struct, but .IsMachine maybe needs doc?
+system-connection-list .Destination .IsMachine
+END_EXCEPTIONS


@Luap99 PTAL, this is the most questionable part of my PR

I think this is a reasonable way of starting this. However I see two problem in this:

Every new unwanted field must be special cases here in the script. Not something most authors will be aware of.

The docs will not match the completion. If we do not want some stuff documented should we maybe not auto complete them as well? I think having a solution in pure go would be better, something that allows us to set a option on a struct field to make it not show up. We could also write some form of generator that will read the go comments from the struct fields and use this as Description so that a) a human must not write them by hand and b) code changes will be reflected in the docs. Of course the quality of the go comments varies but I think most are decent enough to be used as user doc.
That said I know that is not something you can write and I don't know how difficult it would be for me or somebody else.

I am definitely in favour of merging this though. This is much better than nothing.

Every new unwanted field must be special cases here in the script. Not something most authors will be aware of.

True. Good point.

If we do not want some stuff documented should we maybe not auto complete them as well?

Exactly, and this is what has blocked me the two other times I've tried to submit something like this: the document-or-not argument has been unsolvable. See #14386 (comment)

I would love for those tables to be autogenerated!

Luap99

First of all I 100% agree with this. These fields should be documented.

I think it would be useful create an issue/card for the missing commands and assign it to somebody.

Luap99 · 2023-02-09T15:23:28Z

hack/xref-helpmsgs-manpages

+# Table of exceptions for documenting fields in '--format {{.Foo}}'
+#
+# Autocomplete is wonderful, and it's even better when we document the
+# existing options. Unfortunately, sometimes internal structures get
+# exposed that are of no use to anyone and cannot be guaranteed. Avoid
+# documenting those. This table lists those exceptions. Format is:
+#
+#      foo       .Bar
+#
+# ...such that "podman foo --format '{{.Bar}}'" will not be documented.
+#
+my $Format_Exceptions = <<'END_EXCEPTIONS';
+# Deep internal structs; pretty sure these are permanent exceptions
+events       .Details
+history      .ImageHistoryLayer
+images       .ImageSummary
+network-ls   .Network
+
+# FIXME: this one, maybe? But someone needs to write the text
+machine-list    .Starting
+
+# No clue what these are. Some are just different-case dups of others.
+pod-ps  .Containers .Id .InfraId .ListPodsReport .Namespace
+ps      .Cgroup .CGROUPNS .IPC .ListContainer .MNT .Namespaces .NET .PIDNS .User .USERNS .UTS
+
+# I think .Destination is an internal struct, but .IsMachine maybe needs doc?
+system-connection-list .Destination .IsMachine
+END_EXCEPTIONS


I think this is a reasonable way of starting this. However I see two problem in this:

Every new unwanted field must be special cases here in the script. Not something most authors will be aware of.

The docs will not match the completion. If we do not want some stuff documented should we maybe not auto complete them as well? I think having a solution in pure go would be better, something that allows us to set a option on a struct field to make it not show up. We could also write some form of generator that will read the go comments from the struct fields and use this as Description so that a) a human must not write them by hand and b) code changes will be reflected in the docs. Of course the quality of the go comments varies but I think most are decent enough to be used as user doc.
That said I know that is not something you can write and I don't know how difficult it would be for me or somebody else.

I am definitely in favour of merging this though. This is much better than nothing.

rhatdan · 2023-02-09T19:18:36Z

I would love it if the autocompletion did not suggest formats, that we do not want to document. But this is best we have for now.

/lgtm

openshift-ci bot added release-note approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Feb 8, 2023

vrothberg approved these changes Feb 9, 2023

View reviewed changes

edsantiago commented Feb 9, 2023

View reviewed changes

Luap99 reviewed Feb 9, 2023

View reviewed changes

openshift-ci bot assigned rhatdan Feb 9, 2023

openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Feb 9, 2023

openshift-merge-robot merged commit f0d863e into containers:main Feb 9, 2023

Luap99 mentioned this pull request Feb 10, 2023

[RFE]: --format go template fields documentation #17472

Open

edsantiago deleted the xref_format_baby_steps branch February 13, 2023 11:38

edsantiago mentioned this pull request Feb 13, 2023

[WIP] [CI:DOCS] man-page checker: include --format (Go templates) #14046

Closed

github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 10, 2023

github-actions bot locked as resolved and limited conversation to collaborators Sep 10, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CI:DOCS] man-page checker: include --format (Go templates) #17443

[CI:DOCS] man-page checker: include --format (Go templates) #17443

edsantiago commented Feb 8, 2023

edsantiago commented Feb 8, 2023

vrothberg left a comment

openshift-ci bot commented Feb 9, 2023

edsantiago Feb 9, 2023

Luap99 Feb 9, 2023

edsantiago Feb 9, 2023

Luap99 left a comment

Luap99 Feb 9, 2023

rhatdan commented Feb 9, 2023

[CI:DOCS] man-page checker: include --format (Go templates) #17443

[CI:DOCS] man-page checker: include --format (Go templates) #17443

Conversation

edsantiago commented Feb 8, 2023

edsantiago commented Feb 8, 2023

vrothberg left a comment

Choose a reason for hiding this comment

openshift-ci bot commented Feb 9, 2023

edsantiago Feb 9, 2023

Choose a reason for hiding this comment

Luap99 Feb 9, 2023

Choose a reason for hiding this comment

edsantiago Feb 9, 2023

Choose a reason for hiding this comment

Luap99 left a comment

Choose a reason for hiding this comment

Luap99 Feb 9, 2023

Choose a reason for hiding this comment

rhatdan commented Feb 9, 2023