Switch help messages from using [flags] to [options]

[rhatdan]

Want to have man pages match commands, since we have lots of printed
man pages with using Options, we will change the command line to use
Options in --help.

Signed-off-by: Daniel J Walsh [email protected]

[openshift-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rhatdan

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 [rhatdan]

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

[TomSweeneyRedHat]

Looks like you changed Options to Flags everywhere. As other doc says Options, including RHEL doc and our other projects, we should stick with options.

[rhatdan]

Well then you have a conflict between cmd which calls them flags. I am not sure if that can be changed, easily in podman help

$ podman version --help
Display the Podman Version Information

Description:
  

Usage:
  podman version [flags]

Flags:
  -f, --format string   Change the output format to JSON or a Go template

[jwhonce]

small issues

[jwhonce]

Suggested change

	## FLAGS:
	## FLAGS

Other pages do not have :

[jwhonce]

Suggested change

	## FLAGS:
	## FLAGS

[jwhonce]

Suggested change

	## FLAGS:
	## FLAGS

[jwhonce]

Should this list be expanded to include pods and volumes?

[jwhonce]

Suggested change

	## FLAGS:
	## FLAGS

[jwhonce]

Suggested change

Display the running processes of containers in a pod. The *format-descriptors* are ps (1) compatible AIX format descriptors but extended to print additional information, such as the seccomp mode or the effective capabilities of a given process. The descriptors can either be passed as separated arguments or as a single comma-separated argument. Note that you can also specify options and or options of ps(1); in this case, Podman will fallback to executing ps with the specified arguments and options in the container.

Display the running processes of containers in a pod. The *format-descriptors* are ps (1) compatible AIX format descriptors but extended to print additional information, such as the seccomp mode or the effective capabilities of a given process. The descriptors can either be passed as separated arguments or as a single comma-separated argument. Note that you can specify options and/or additionally options of ps(1); in this case, Podman will fallback to executing ps with the specified arguments and options in the container.

[jwhonce]

Looks like you changed Options to Flags everywhere. As other doc says Options, including RHEL doc and our other projects, we should stick with options.

I suspect we would need to dig into how to override the whole help generation
https://github.com/spf13/cobra/blob/master/command.go#L1240

[rhatdan]

I fixed the help generation to use Options now. so back to original.
sed 's/flag/option/g' ...

I will need help from @edsantiago to get his tests to pass. test/system/015-help.bats still seems broken.

[rhatdan]

BTW we definitely need the tests to run on this. This is not only CI Docs.

[jwhonce]

@rhatdan

At https://github.com/containers/podman/blob/master/cmd/podman/main.go#L69 you may need to add

				c.Command.DisableFlagsInUseLine = true

[edsantiago]

One reason for [flags] is that Cobra automatically adds that string if it is missing from a help message. That is, if your command has flags-options-whatever, and you do not include the string "[flags]" in the Usage message, Cobra will helpfully add it (usually in the wrong place).

I don't really care whether anyone uses flags or options, but I've had an exhausting day and am not going to be able to review this tonight or tomorrow. I'm sorry. I do hope my comment above is helpful at least.

[rhatdan]

@edsantiago I think I have removed all issues with Flags, now your test script is blowing up. Could you take a look at this PR versus your test script?

[edsantiago]

Patch:

diff --git a/cmd/podman/containers/list.go b/cmd/podman/containers/list.go
index daf03a51b..78a15559f 100644
--- a/cmd/podman/containers/list.go
+++ b/cmd/podman/containers/list.go
@@ -10,7 +10,7 @@ import (
 var (
        // podman container _list_
        listCmd = &cobra.Command{
-               Use:     "list",
+               Use:     "list [options]",
                Aliases: []string{"ls"},
                Args:    validate.NoArgs,
                Short:   "List containers",

That doesn't fix everything, though. ./bin/podman --help is showing Flags, not Options. I could track that down but it's probably much faster for you to do so.

[edsantiago]

Next patch (assumes that you fix the Flags: error in podman --help; if you can't fix that, lmk, it's a simple tweak)

diff --git a/hack/xref-helpmsgs-manpages b/hack/xref-helpmsgs-manpages
index 7b617eed7..416d65182 100755
--- a/hack/xref-helpmsgs-manpages
+++ b/hack/xref-helpmsgs-manpages
@@ -228,14 +228,14 @@ sub podman_help {
         #    Usage: ...
         #    Available Commands:
         #       ....
-        #    Flags:
+        #    Options:
         #       ....
         #
         # Start by identifying the section we're in...
         if ($line =~ /^Available\s+(Commands):/) {
             $section = lc $1;
         }
-        elsif ($line =~ /^(Flags):/) {
+        elsif ($line =~ /^(Options):/) {
             $section = lc $1;
         }

@@ -248,7 +248,7 @@ sub podman_help {
                     unless $subcommand eq 'help';       # 'help' not in man
             }
         }
-        elsif ($section eq 'flags') {
+        elsif ($section eq 'options') {
             # Handle '--foo' or '-f, --foo'
             if ($line =~ /^\s{1,10}(--\S+)\s/) {
                 print "> podman @_ $1\n"                        if $debug;
@@ -320,7 +320,7 @@ sub podman_man {
             }
         }

-        # Flags should always be of the form '**-f**' or '**--flag**',
+        # Options should always be of the form '**-f**' or '**--flag**',
         # possibly separated by comma-space.
         elsif ($section eq 'flags') {
             # e.g. 'podman run --ip6', documented in man page, but nonexistent

[edsantiago]

Here's one possible fix for setting Flags: -> Options: in podman --help

diff --git a/cmd/podman/root.go b/cmd/podman/root.go
index eed3968e5..d23230763 100644
--- a/cmd/podman/root.go
+++ b/cmd/podman/root.go
@@ -81,6 +81,7 @@ func init() {
        )

        rootFlags(rootCmd, registry.PodmanConfig())
+       rootCmd.SetUsageTemplate(usageTemplate)
 }

 func Execute() {

Note that you do not want to set helpTemplate here: it will add an undesired Description heading.

[edsantiago]

try this

diff --git a/docs/remote-docs.sh b/docs/remote-docs.sh
index a9fda4696..67c731e75 100755
--- a/docs/remote-docs.sh
+++ b/docs/remote-docs.sh
@@ -78,7 +78,7 @@ function html_fn() {
 # the command name but not its description.
 function podman_commands() {
     $PODMAN help "$@" |\
-        awk '/^Available Commands:/{ok=1;next}/^Flags:/{ok=0}ok { print $1 }' |\
+        awk '/^Available Commands:/{ok=1;next}/^Options:/{ok=0}ok { print $1 }' |\
         grep .
 }

[edsantiago]

WARNING: podman: man page shows '[*options*]', help does not show [options]
WARNING: podman container exists: man page shows '[*options*]', help does not show [options]

The second is easy, just an oversight:

diff --git a/cmd/podman/containers/exists.go b/cmd/podman/containers/exists.go
index 1d79b684d..70b8af159 100644
--- a/cmd/podman/containers/exists.go
+++ b/cmd/podman/containers/exists.go
@@ -12,7 +12,7 @@ var (
        containerExistsDescription = `If the named container exists in local storage, podman container exists exits with 0, otherwise the exit code will be 1.`

        existsCommand = &cobra.Command{
-               Use:   "exists [flags] CONTAINER",
+               Use:   "exists [options] CONTAINER",
                Short: "Check if a container exists in local storage",
                Long:  containerExistsDescription,
                Example: `podman container exists --external containerID

The first is harder. Here is a side-by-side of old vs new:

$ /usr/bin/podman --help                  | $ ./bin/podman --help
Manage pods, containers and images        | Manage pods, containers and images
                                          |
Usage:                                    | Usage:
  podman [flags]                          | podman [command]
  podman [command]                        |

IOW, the podman [flags] string is gone.

Best case would be to find a way to make that line podman [options] command, since command is not optional. Is there a way to do that?

[edsantiago]

This seems to have the desired (by me) effect:

diff --git a/cmd/podman/root.go b/cmd/podman/root.go
index d23230763..54c168bd2 100644
--- a/cmd/podman/root.go
+++ b/cmd/podman/root.go
@@ -38,7 +38,7 @@ Description:
 // command should not use this.
 const usageTemplate = `Usage:{{if (and .Runnable (not .HasAvailableSubCommands))}}
   {{.UseLine}}{{end}}{{if .HasAvailableSubCommands}}
-  {{.CommandPath}} [command]{{end}}{{if gt (len .Aliases) 0}}
+  {{.CommandPath}}{{if .HasAvailableLocalFlags}} [options]{{end}} command{{end}}{{if gt (len .Aliases) 0}}

 Aliases:
   {{.NameAndAliases}}{{end}}{{if .HasExample}}
diff --git a/test/system/015-help.bats b/test/system/015-help.bats
index 22db8be8a..d6a8a80c9 100644
--- a/test/system/015-help.bats
+++ b/test/system/015-help.bats
@@ -3,7 +3,7 @@
 # Tests based on 'podman help'
 #
 # Find all commands listed by 'podman --help'. Run each one, make sure it
-# provides its own --help output. If the usage message ends in '[command]',
+# provides its own --help output. If the usage message ends in 'command',
 # treat it as a subcommand, and recurse into its own list of sub-subcommands.
 #
 # Any usage message that ends in '[options]' is interpreted as a command
@@ -45,8 +45,8 @@ function check_help() {
         # has no ' [options]'
         is "$usage " "  $command_string .*" "Usage string matches command"

-        # If usage ends in '[command]', recurse into subcommands
-        if expr "$usage" : '.*\[command\]$' >/dev/null; then
+        # If usage ends in 'command', recurse into subcommands
+        if expr "$usage" : '.*command$' >/dev/null; then
             found[subcommands]=1
             check_help "$@" $cmd
             continue

I don't know if others will find that 'command' (no brackets) desirable; nor do I know what other side effects it might have.

Also, I think you lost one of the changes I specified above. You'll need to apply this to get the man-page xref to pass:

diff --git a/hack/xref-helpmsgs-manpages b/hack/xref-helpmsgs-manpages
index a7063259f..416d65182 100755
--- a/hack/xref-helpmsgs-manpages
+++ b/hack/xref-helpmsgs-manpages
@@ -248,7 +248,7 @@ sub podman_help {
                     unless $subcommand eq 'help';       # 'help' not in man
             }
         }
-        elsif ($section eq 'flags') {
+        elsif ($section eq 'options') {
             # Handle '--foo' or '-f, --foo'
             if ($line =~ /^\s{1,10}(--\S+)\s/) {
                 print "> podman @_ $1\n"                        if $debug;

[edsantiago]

I think you missed this one #8034 (comment)

[edsantiago]

While you're at it would you mind

diff --git a/completions/zsh/_podman b/completions/zsh/_podman
index 067eebbbb..80dc5125d 100644
--- a/completions/zsh/_podman
+++ b/completions/zsh/_podman
@@ -206,7 +206,7 @@ _set_up_podman_args() {

     typeset -ga _podman_args=()
     # E.g. 'podman exec [flags] CONTAINER [...' -> 'CONTAINER [....'
-    local usage_rhs=$(expr "$_podman_usage" : ".*\[flags\] \+\(.*\)")
+    local usage_rhs=$(expr "$_podman_usage" : ".*\[options\] \+\(.*\)")

     # e.g. podman pod ps which takes no further args
     if [ -z "$usage_rhs" ]; then

[edsantiago]

Error was:

Trying to pull quay.io/libpod/fedora-minimal:latest...
[...]
Timed out after 90.000s.

Treating as a flake. Restarted.

[edsantiago]

/lgtm
/hold

@containers/podman-maintainers PTAL, let's get this in before some breaking change merges and causes conflicts

[mheon]

LGTM
/hold cancel