Uh oh!
There was an error while loading. Please reload this page.
Improve boolean flag documentation generation - #48
Conversation
gmargaritis
commented
Feb 11, 2024
@thaJeztah Let me know if we want to also display the Type of boolean flags regardless of their default value |
crazy-max
commented
Feb 12, 2024
I think we want the same for yaml generation? |
crazy-max
commented
Feb 12, 2024
Can we also have a test for this case? |
thaJeztah
commented
Feb 12, 2024
@crazy-max I think we already unconditionally set the default value for YAML; at least looking at the fixture here, I see cli-docs-tool/fixtures/docker_buildx_build.yaml Lines 74 to 78 in f588677 |
crazy-max
commented
Feb 12, 2024
Ah ok looks good then 👍 |
That's correct! Do we want to make any changes here? cli-docs-tool/clidocstool_yaml.go Lines 271 to 290 in f588677 |
thaJeztah
left a comment
There was a problem hiding this comment.
LGTM
we should probably look at some additional test-cases in a follow-up
| defval = strings.ReplaceAll(defval, cd, "`") | ||
| } | ||
| } else if f.DefValue != "" && (f.Value.Type() != "bool" && f.DefValue != "true") && f.DefValue != "[]" { | ||
| } else if f.DefValue != "" && ((f.Value.Type() != "bool" && f.DefValue != "true") || (f.Value.Type() == "bool" && f.DefValue == "true")) && f.DefValue != "[]" { |
There was a problem hiding this comment.
For a follow-up, we should probably look at splitting some of these up. Things start to become too complicated IMO (too many boolean conditions here, which makes it easy to introduce bugs).
thaJeztah
commented
Feb 12, 2024
@crazy-max LGTY? |
crazy-max
commented
Feb 12, 2024
Can we have a test in https://github.com/docker/cli-docs-tool/blob/main/clidocstool_test.go#L38? Just a dummy new flag like |
Update the documentation generation logic to include values in Type and Default columns for boolean flags that have true as a default value. This change addresses the need for users to differentiate between boolean flags that require explicit negation. Signed-off-by: George Margaritis <gmargaritis@protonmail.com>
4c6e97f to
e795250Comparegmargaritis
commented
Feb 12, 2024
@crazy-max Added the test case, we should be good to go! |
Resolves#47
Updated the documentation generation logic to include values in Type and Default columns for boolean flags that have true as a default value. This change addresses the need for users to differentiate between boolean flags that require explicit negation.