display flag type and default value in a dedicated columns

[crazy-max]

flag type is mixed with flag name and can be hard to read in our documentation. This PR adds a dedicated column for it.

also overrides types that can be confusing for some users docker/buildx#717:

• stringArray: list
• stringSlice: list/strings

wonder if we could also add a dedicated legend to explain some types like docker/buildx#922 (comment)

example: https://github.com/crazy-max/buildx/blob/mkdocs/docs/reference/buildx_build.md

let me know what you think @thaJeztah

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

[codecov-commenter]

Codecov Report

Merging #19 (b3ecfe1) into main (77df2af) will increase coverage by 0.47%.
The diff coverage is 83.33%.

@@            Coverage Diff             @@
##             main      #19      +/-   ##
==========================================
+ Coverage   64.37%   64.84%   +0.47%     
==========================================
  Files           4        4              
  Lines         508      532      +24     
==========================================
+ Hits          327      345      +18     
- Misses        127      129       +2     
- Partials       54       58       +4     

Impacted Files	Coverage Δ
clidocstool_yaml.go	58.30% <82.35%> (+0.73%)	⬆️
clidocstool_md.go	76.36% <84.21%> (-0.11%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77df2af...b3ecfe1. Read the comment docs.

[crazy-max]

@thaJeztah I revert the stringArray, stringSlice type conversion and also adds a Default column for markdown. See https://github.com/crazy-max/docker-cli-docs-tool/blob/flag-type/fixtures/buildx_build.md

[thaJeztah]

also adds a Default column for markdown.

Not really against this, but thinking if it always shows useful information. In quite some cases it may not be;

• we defer the default to the daemon side (client leaves the default empty, so that the daemon can decide what to do (default can be configurable there in some cases)
• in some case, the "default" may be part of the flag's description (which could be explaining that the daemon sets it)
• in some cases, the "default" that's shown on the command-line is dynamically set (given: perhaps not best practice for all). For example, I recall that being the case for --config-file (although ~/.docker/config.json would probably be fine for that);

docker --help | grep 'config files'
      --config string      Location of client config files (default "/Users/sebastiaan/.docker")

[crazy-max]

As discussed yeah we can't dynamically handle that for static generation if defaults are handled daemon side but we could be smart and provide insights about its behavior in the default column. Maybe using an annotation. We could also move defaults not linked to the flag type in this annotation to avoid putting it in the description. This way it would be properly handled both for markdown generation and yaml one that could then be used on docs.docker.com.

I can think of an annotation and see how it could be handled with buildx for example. Do you want that in a follow-up or here?

We also discussed about badges like:

We could add it to markdown generation using http://shield.io/:

https://img.shields.io/badge/api-1.25-blue

[crazy-max]

@thaJeztah I have added an annotation to be able to override the default value. Let me know if it sgty.

[thaJeztah]

LGTM

[gmargaritis]

@crazy-max @thaJeztah Why did we decide to skip the Type and Default columns for boolean types here?

cli-docs-tool/clidocstool_md.go

Lines 243 to 258 in f588677

	var ftype string
	if f.Value.Type() != "bool" {
	ftype = "`" + f.Value.Type() + "`"
	}
	var defval string
	if v, ok := f.Annotations[annotation.DefaultValue]; ok && len(v) > 0 {
	defval = v[0]
	if cd, ok := f.Annotations[annotation.CodeDelimiter]; ok {
	defval = strings.ReplaceAll(defval, cd[0], "`")
	} else if cd, ok := cmd.Annotations[annotation.CodeDelimiter]; ok {
	defval = strings.ReplaceAll(defval, cd, "`")
	}
	} else if f.DefValue != "" && (f.Value.Type() != "bool" && f.DefValue != "true") && f.DefValue != "[]" {
	defval = "`" + f.DefValue + "`"
	}

This seems related to docker/cli#4259 (comment)

Since pretty much all of the optional flags are false by default, we could add the type and default to boolean flags that have true as default. This would make it easier for users to differentiate.

[thaJeztah]

Thanks for looking @gmargaritis - I suspect this was by accident, and the intent was to not show a default for a boolean flag that uses false as default.

The reason for that was to make it more similar to the --help output itself. For example, --help is a "boolean", but while it's technically valid to use --help=false, doing so would be rare, and in the past we showed false as default (on the command-line), which confused users.

However, if the flag is a boolean, but uses true by default, I think we should show that, because that's way less common, and relevant information for the user.