From c6090c290e1eb8b60148a026773c5f8cce6eb435 Mon Sep 17 00:00:00 2001 From: Ed Santiago Date: Wed, 24 Jun 2020 10:16:59 -0600 Subject: Docs: consistency between man / --help New functionality in hack/man-page-checker: start cross- referencing the man page 'Synopsis' line against the output of 'podman foo --help'. This is part 1, flag/option consistency. Part 2 (arg consistency) is too big and will have to wait for later. flag/option consistency means: if 'podman foo --help' includes the string '[flags]' in the Usage message, make sure the man page includes '[*options*]' in its Synopsis line, and vice-versa. This found several inconsistencies, which I've fixed. While doing this I realized that Cobra automatically includes a 'Flags:' subsection in its --help output for all subcommands that have defined flags. This is great - it lets us cross-check against the usage synopsis, and make sure that '[flags]' is present or absent as needed, without fear of human screwups. If a flag-less subcommand ever gets extended with flags, but the developer forgets to add '[flags]' and remove DisableFlagsInUseLine, we now have a test that will catch that. (This, too, caught two instances which I fixed). I don't actually know if the new man-page-checker functionality will work in CI: I vaguely recall that it might run before 'make podman' does; and also vaguely recall that some steps were taken to remedy that. Signed-off-by: Ed Santiago --- hack/man-page-checker | 72 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 67 insertions(+), 5 deletions(-) (limited to 'hack') diff --git a/hack/man-page-checker b/hack/man-page-checker index 17d85d65d..d2cc6c6e1 100755 --- a/hack/man-page-checker +++ b/hack/man-page-checker @@ -3,6 +3,14 @@ # man-page-checker - validate and cross-reference man page names # +verbose= +for i; do + case "$i" in + -v|--verbose) verbose=verbose ;; + esac +done + + die() { echo "$(basename $0): $*" >&2 exit 1 @@ -65,6 +73,61 @@ for md in $(ls -1 *-*.1.md | grep -v remote);do fi done +# Helper function: compares man page synopsis vs --help usage message +function compare_usage() { + local cmd="$1" + local from_man="$2" + + # Sometimes in CI we run before podman gets built. + test -x ../../../bin/podman || return + + # Run 'cmd --help', grab the line immediately after 'Usage:' + local help_output=$(../../../bin/$cmd --help) + local from_help=$(echo "$help_output" | grep -A1 '^Usage:' | tail -1) + + # strip off command name from both + from_man=$(sed -e "s/\*\*$cmd\*\*[[:space:]]*//" <<<"$from_man") + from_help=$(sed -e "s/^[[:space:]]*$cmd[[:space:]]*//" <<<"$from_help") + + # man page lists 'foo [*options*]', help msg shows 'foo [flags]'. + # Make sure if one has it, the other does too. + if expr "$from_man" : "\[\*options\*\]" >/dev/null; then + if expr "$from_help" : "\[flags\]" >/dev/null; then + : + else + echo "WARNING: $cmd: man page shows '[*options*]', help does not show [flags]" + rc=1 + fi + elif expr "$from_help" : "\[flags\]" >/dev/null; then + echo "WARNING: $cmd: --help shows [flags], man page does not show [*options*]" + rc=1 + fi + + # Strip off options and flags; start comparing arguments + from_man=$(sed -e 's/^\[\*options\*\][[:space:]]*//' <<<"$from_man") + from_help=$(sed -e 's/^\[flags\][[:space:]]*//' <<<"$from_help") + + # Args in man page are '*foo*', in --help are 'FOO'. Convert all to + # UPCASE simply because it stands out better to the eye. + from_man=$(sed -e 's/\*\([a-z-]\+\)\*/\U\1/g' <<<"$from_man") + + # FIXME: one of the common patterns is for --help to show 'POD [POD...]' + # but man page show 'pod ...'. This conversion may help one day, but + # not yet: there are too many inconsistencies such as '[pod ...]' + # (brackets) and 'pod...' (no space between). +# from_help=$(sed -e 's/\([A-Z]\+\)[[:space:]]\+\[\1[[:space:]]*\.\.\.\]/\1 .../' <<<"$from_help") + + # Compare man-page and --help usage strings. For now, do so only + # when run with --verbose. + if [[ "$from_man" != "$from_help" ]]; then + if [ -n "$verbose" ]; then + printf "%-25s man='%s' help='%s'\n" "$cmd:" "$from_man" "$from_help" + # Yeah, we're not going to enable this as a blocker any time soon. + # rc=1 + fi + fi +} + # Pass 3: compare synopses. # # Make sure the SYNOPSIS line in podman-foo.1.md reads '**podman foo** ...' @@ -87,9 +150,7 @@ for md in *.1.md;do cmd=$(echo "$synopsis" | sed -e 's/\(.*\)\*\*.*/\1/' | tr -d \*) md_nodash=$(basename "$md" .1.md | tr '-' ' ') if [[ $md_nodash = 'podman auto update' ]]; then - # podman-auto-update.1.md is special cased as it's structure differs - # from that of other man pages where main and sub-commands split by - # dashes. + # special case: the command is "auto-update", with a hyphen md_nodash='podman auto-update' fi if [ "$cmd" != "$md_nodash" -a "$cmd" != "podman-remote" ]; then @@ -111,8 +172,9 @@ for md in *.1.md;do # (for debugging, and getting a sense of standard conventions) #printf " %-32s ------ '%s'\n" $md "$synopsis" - # FIXME: some day: run ./bin/podman "args", extract Usage, - # strip off [flags] and [options], then compare arguments + # If bin/podman is available, run "cmd --help" and compare Usage + # messages. This is complicated, so do it in a helper function. + compare_usage "$md_nodash" "$synopsis" done -- cgit v1.2.3-54-g00ecf