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

[edsantiago]

Autocomplete is awesome. It offers us a programmatic way to
determine the set of values for a given --format option.
Use that to make sure the man pages document reality.

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

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: edsantiago

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]

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

[edsantiago]

I don't need to say DO NOT MERGE, because this isn't even going to pass CI. Current set of failures is:

xref-helpmsgs-manpages: 'podman history --format': .CreatedAt in docs/source/markdown/podman-history.1.md, but not --help
xref-helpmsgs-manpages: 'podman history --format': .CreatedSince in docs/source/markdown/podman-history.1.md, but not --help
xref-helpmsgs-manpages: 'podman image history --format': .CreatedAt in docs/source/markdown/podman-history.1.md, but not --help
xref-helpmsgs-manpages: 'podman image history --format': .CreatedSince in docs/source/markdown/podman-history.1.md, but not --help
xref-helpmsgs-manpages: 'podman image list --format': .CreatedAt in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman image list --format': .CreatedSince in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman image list --format': .Repository in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman image list --format': .Tag in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman image search': --format options documented in man page, but not available via autocomplete
xref-helpmsgs-manpages: 'podman images --format': .CreatedAt in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman images --format': .CreatedSince in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman images --format': .Repository in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman images --format': .Tag in docs/source/markdown/podman-images.1.md, but not --help
xref-helpmsgs-manpages: 'podman network inspect': --format options documented in man page, but not available via autocomplete
xref-helpmsgs-manpages: 'podman search': --format options documented in man page, but not available via autocomplete

Those are beyond my ability to fix. And, as will be obvious to anyone who looks at the diffs, I didn't even bother describing any of the options I added in the man pages.

This is just a proof of concept. Worth pursuing? If so, would someone like to tag-team with me on it, pushing fixes to this PR?

[Luap99]

The idea is very nice but there is a significant problem with it. The current autocompletion is best effort and far from perfect.

For example podman inspect --format can display containers, pods, images, networks... There no reason to duplicated all fields in the man page when they are already in podman-container-inspect(1) and so on. I think at the moment podman inspect --format doesn't provide any autocompletion for a struct fields which is wrong and should be fixed.

Also this all depends on the correct use of AutocompleteFormat(). I think this is a good idea but I have to improve AutocompleteFormat() first to actually make this work correctly. I guess it would help to have a test which checks that the suggested field name actually work for the given command.

[edsantiago]

Thank you!

[Luap99]

I think we should still merge some of your detected problem (--rollback ordering for example)

I will take a look at the autocompletion code to see if there is anything I can improve easily

[edsantiago]

@containers/podman-maintainers this is a followup (precursor, actually) to @Luap99's demo this morning. Could each of you PTAL at the diffs and, if you are responsible for any of the modified man pages, take a moment to submit a (separate) PR to update the --format documentation for your given subcommand?

There is, at this time, no automated checking. This PR probable has to linger for weeks or months while we get the man pages in shape. If you're willing to do the man-page updates piecemeal, I'm willing to keep rebasing and tweaking the script logic. (By "tweak" I mean find, with your help, some way to deal with podman stats where the man page and autocomplete are completely and wildly out of sync;

[Luap99]

We should get this change merged independently from this PR, this is obviously correct and should not have to wait for the other stuff to land which will take more time.

[edsantiago]

OK, I can submit a mini-PR for this and other obvious fixes. Before I do so, can you (or anyone) comment on the weird "Print the numeric IDs only" line (search for FIXME FIXME) below?

[Luap99]

I think this must be under the --quiet option.

[edsantiago]

The rest of the man pages can be slowly filled in if/when people like, but this set here bothers me a little: some of the fields are duplicated for no clear reason; some (Duration?) seem misleading to my untrained eye. I wonder if someone familiar with podman stats would mind taking a look and confirming that these fields all do what's desired?

[rhatdan]

@edsantiago Are you intending on merging this at some point?

[edsantiago]

I'm feeling very lost. The PR you just closed was a subset of this one. How can this one be acceptable, but a tiny-subset not be?

Looking back at the discussion on that PR, you objected to some duplicate fields (which are also present in this PR, because the underlying completion code is the same). IIUC the completion code cannot easily be fixed to remove the duplication or to hide a subset of fields. So there doesn't seem to be anything I can do: either I document everything, including useless confusing stuff, or I document nothing (which is what we have now). I can't do a middle ground.

[Luap99]

I think we should do this and document all options. Right now we have zero idea about the stability of these options. Removing or renaming fields in structs that are exposed via a template are breaking changes to users. We are very bad a catching stuff like this at review time.
Having it documented in the man page and check via script will ensure that it is up do date and will make it very clear when there is a breaking change.

If there is a reason to hide some fields from the shell completion then this is a technical problem which can be solved. But I still think we should never expose the internal structs to the fronted, instead use stable (template only) structs and copy the data (and convert where needed) from the internal struct to the template struct.

[mheon]

Using template-exclusive structs would be a big change, but I agree that it's necessary. Tying our internal data representations to Docker's formats is not sensible or sane in the long run.

[rhatdan]

I will not stand in the way of implementing this. My fear is the noice to signal ratio could get out of control. But catching bugs and documenting things like format would be useful.

[github-actions]

A friendly reminder that this PR had no activity for 30 days.

[rhatdan]

@edsantiago Going to move forward on this after the consolidation is done?

[edsantiago]

@rhatdan I would like to proceed with this one day, but it seems like we still need to have deep conversations about what that would look like, and perhaps do some internal work to mark some formats as hidden.

[openshift-merge-robot]

@edsantiago: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

[edsantiago]

Addressed, at least partly, in #17443. Future discussion will take place in #17472.