Show the actual available flags in useline

[lugray]

Use common useline notation for available flags:

• [-f foo] for (standard) optional flags
• [-f foo | -b bar] for mutually exclusive flags
• -f foo for required flags
• {-f foo | -b bar} for mutually exclusive flags where one is required
• [-f foo -b bar] for flags required as a group

For a boolean flag f, foo is dropped from the above. The foo/bar comes from flag.UnquoteUsage.

[CLAassistant]

All committers have signed the CLA.

[marckhouzam]

This sounds like a nice improvement @lugray. Thanks! I’ll try to find some time in the coming weeks.

[marckhouzam]

Thanks @lugray! can you share complete help output of complex commands with many arguments using this feature? How long lines are handled?

For example other tools with complex command lines options using this style use multiple lines to make the output nicer.

This example is using manual formatting - hard work but you have complete control on the output: https://gitlab.com/nbdkit/libnbd/-/blob/master/copy/main.c?ref_type=heads#L69

I like the idea behind this improvement. Thanks @lugray !

I do prefer the multi-line format @nirs suggests. Currently for example, here is the change to a helm command help:

# current
helm repo add [NAME] [URL] [flags]

# with this PR
helm repo add [NAME] [URL] [--allow-deprecated-repos] [--burst-limit int] [--ca-file string] [--cert-file string] [--debug] [--force-update] [-h] [--insecure-skip-tls-verify] [--key-file string] [--kube-apiserver string] [--kube-as-group stringArray] [--kube-as-user string] [--kube-ca-file string] [--kube-context string] [--kube-insecure-skip-tls-verify] [--kube-tls-server-name string] [--kube-token string] [--kubeconfig string] [-n string] [--no-update] [--pass-credentials] [--password string] [--password-stdin] [--registry-config string] [--repository-cache string] [--repository-config string] [--username string]

As you can see this is problematic for tools with lots of flags. Could you use multi-line like @nirs suggests?

The above gives me a couple of ideas:

1. should we list global flags or should we limit ourselves to local flags and maybe add [global flags] at the end?
2. it may happen that tools only have a few commands with many flags; in such a case it would be nice to allow to control enabling this feature on a per-command basis also. BUT, with the better formatting @nirs suggests, maybe we won't need this. I'm mentioning it so I don't forget.

[nirs]

Trying helm help with multiple lines (manually formatted):

$ helm repo add -h
add a chart repository

Usage:
  helm repo add [NAME] [URL] [--allow-deprecated-repos] [--burst-limit int] [--ca-file string]
                [--cert-file string] [--debug] [--force-update] [-h]
                [--insecure-skip-tls-verify] [--key-file string] [--kube-apiserver string]
                [--kube-as-group stringArray] [--kube-as-user string] [--kube-ca-file string]
                [--kube-context string] [--kube-insecure-skip-tls-verify]
                [--kube-tls-server-name string] [--kube-token string] [--kubeconfig string]
                [-n string] [--no-update] [--pass-credentials] [--password string]
                [--password-stdin] [--registry-config string] [--repository-cache string]
                [--repository-config string] [--username string]

Flags:
      --allow-deprecated-repos     by default, this command will not allow adding official repos that have been permanently deleted. This disables that behavior
      --ca-file string             verify certificates of HTTPS-enabled servers using this CA bundle
      --cert-file string           identify HTTPS client using this SSL certificate file
      --force-update               replace (overwrite) the repo if it already exists
  -h, --help                       help for add
      --insecure-skip-tls-verify   skip tls certificate checks for the repository
      --key-file string            identify HTTPS client using this SSL key file
      --no-update                  Ignored. Formerly, it would disabled forced updates. It is deprecated by force-update.
      --pass-credentials           pass credentials to all domains
      --password string            chart repository password
      --password-stdin             read chart repository password from stdin
      --username string            chart repository username

Global Flags:
      --burst-limit int                 client-side default throttling limit (default 100)
      --debug                           enable verbose output
      --kube-apiserver string           the address and the port for the Kubernetes API server
      --kube-as-group stringArray       group to impersonate for the operation, this flag can be repeated to specify multiple groups.
      --kube-as-user string             username to impersonate for the operation
      --kube-ca-file string             the certificate authority file for the Kubernetes API server connection
      --kube-context string             name of the kubeconfig context to use
      --kube-insecure-skip-tls-verify   if true, the Kubernetes API server's certificate will not be checked for validity. This will make your HTTPS connections insecure
      --kube-tls-server-name string     server name to use for Kubernetes API server certificate validation. If it is not provided, the hostname used to contact the server is used
      --kube-token string               bearer token used for authentication
      --kubeconfig string               path to the kubeconfig file
  -n, --namespace string                namespace scope for this request
      --registry-config string          path to the registry config file (default "/home/nsoffer/.config/helm/registry/config.json")
      --repository-cache string         path to the file containing cached repository indexes (default "/home/nsoffer/.cache/helm/repository")
      --repository-config string        path to the file containing repository names and URLs (default "/home/nsoffer/.config/helm/repositories.yaml")

Much better than long line but still overwhelming. The current output is better. Finding the right flag in the list of flags is much easier.

$ helm repo add -h
add a chart repository

Usage:
  helm repo add [NAME] [URL] [flags]

Flags:
      --allow-deprecated-repos     by default, this command will not allow adding official repos that have been permanently deleted. This disables that behavior
      --ca-file string             verify certificates of HTTPS-enabled servers using this CA bundle
      --cert-file string           identify HTTPS client using this SSL certificate file
      --force-update               replace (overwrite) the repo if it already exists
  -h, --help                       help for add
      --insecure-skip-tls-verify   skip tls certificate checks for the repository
      --key-file string            identify HTTPS client using this SSL key file
      --no-update                  Ignored. Formerly, it would disabled forced updates. It is deprecated by force-update.
      --pass-credentials           pass credentials to all domains
      --password string            chart repository password
      --password-stdin             read chart repository password from stdin
      --username string            chart repository username

Global Flags:
      --burst-limit int                 client-side default throttling limit (default 100)
      --debug                           enable verbose output
      --kube-apiserver string           the address and the port for the Kubernetes API server
      --kube-as-group stringArray       group to impersonate for the operation, this flag can be repeated to specify multiple groups.
      --kube-as-user string             username to impersonate for the operation
      --kube-ca-file string             the certificate authority file for the Kubernetes API server connection
      --kube-context string             name of the kubeconfig context to use
      --kube-insecure-skip-tls-verify   if true, the Kubernetes API server's certificate will not be checked for validity. This will make your HTTPS connections insecure
      --kube-tls-server-name string     server name to use for Kubernetes API server certificate validation. If it is not provided, the hostname used to contact the server is used
      --kube-token string               bearer token used for authentication
      --kubeconfig string               path to the kubeconfig file
  -n, --namespace string                namespace scope for this request
      --registry-config string          path to the registry config file (default "/home/nsoffer/.config/helm/registry/config.json")
      --repository-cache string         path to the file containing cached repository indexes (default "/home/nsoffer/.cache/helm/repository")
      --repository-config string        path to the file containing repository names and URLs (default "/home/nsoffer/.config/helm/repositories.yaml")

Trying without global flags looks much better:

$ helm repo add -h
add a chart repository

Usage:
  helm repo add [NAME] [URL] [--allow-deprecated-repos] [--ca-file string]
                [--cert-file string] [--force-update] [-h] [--insecure-skip-tls-verify]
                [--key-file string] [--no-update] [--pass-credentials] [--password string]
                [--password-stdin] [--username string]

Flags:
      --allow-deprecated-repos     by default, this command will not allow adding official repos that have been permanently deleted. This disables that behavior
      --ca-file string             verify certificates of HTTPS-enabled servers using this CA bundle
      --cert-file string           identify HTTPS client using this SSL certificate file
      --force-update               replace (overwrite) the repo if it already exists
  -h, --help                       help for add
      --insecure-skip-tls-verify   skip tls certificate checks for the repository
      --key-file string            identify HTTPS client using this SSL key file
      --no-update                  Ignored. Formerly, it would disabled forced updates. It is deprecated by force-update.
      --pass-credentials           pass credentials to all domains
      --password string            chart repository password
      --password-stdin             read chart repository password from stdin
      --username string            chart repository username

Global Flags:
      --burst-limit int                 client-side default throttling limit (default 100)
      --debug                           enable verbose output
      --kube-apiserver string           the address and the port for the Kubernetes API server
      --kube-as-group stringArray       group to impersonate for the operation, this flag can be repeated to specify multiple groups.
      --kube-as-user string             username to impersonate for the operation
      --kube-ca-file string             the certificate authority file for the Kubernetes API server connection
      --kube-context string             name of the kubeconfig context to use
      --kube-insecure-skip-tls-verify   if true, the Kubernetes API server's certificate will not be checked for validity. This will make your HTTPS connections insecure
      --kube-tls-server-name string     server name to use for Kubernetes API server certificate validation. If it is not provided, the hostname used to contact the server is used
      --kube-token string               bearer token used for authentication
      --kubeconfig string               path to the kubeconfig file
  -n, --namespace string                namespace scope for this request
      --registry-config string          path to the registry config file (default "/home/nsoffer/.config/helm/registry/config.json")
      --repository-cache string         path to the file containing cached repository indexes (default "/home/nsoffer/.cache/helm/repository")
      --repository-config string        path to the file containing repository names and URLs (default "/home/nsoffer/.config/helm/repositories.yaml")

Maybe we need 2 options?

• ShowFlagsInUseLine
• ShowPersistentFlagsInUseLine

Another way , add MarkShowFlagInUseLine() so it is possible to include or exclude flags from the use line to show only the most important flags in the use line. This allows making insecure options like [--insecure-skip-tls-verify] less visible, helping users to do the right thing.

[marckhouzam]

Seeing the whole output makes me realize we have a duplication of flag names very close together (in the usage line and listed below it). This makes me wonder if the proposed change brings much value to the end user?

I’m leaning towards keeping things as is to avoid unnecessarily complicating the code base.

But I’m open to hear if someone sees more value in this modified help text.

[marckhouzam]

I'm going to close this as "won't implement". If you still feel this has value, feel free to re-open and explain.