-
Notifications
You must be signed in to change notification settings - Fork 23.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Ensure ansible-docs display boolean values using true/false #78557
Conversation
Adopts JSON/YAML 1.2 standard booleans names. Related: ansible-community/community-topics#116
if isinstance(obj, bool): | ||
# Return JSON/YAML 1.2 standardized true/false values | ||
return "true" if obj else "false" | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely the wrong place for this to happen. This function should not concern itself with YAML serialization. This is basically just Python conversion to text type, not special case modifications to non python types.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sivel given your deep knowledge and experience with ansible, could you take a few minutes to get me headed in the right direction? A couple pointers into the code would be great. I really want to get this right.
As part of mentioned change, we want to ensure that we only display booleans as true
, false
to the user, including output of ansible-doc <module>
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should already be the case except where 'python stringification' takes place, ... which affects fields like Default but not most others. Removing the python stringification would make it match.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You will likely have to make modifications in every location that outputs the value.
In the case of ansible-doc
, the change would be required in lib/ansible/cli/doc.py::DocCLI.add_fields
. A new helper function might be required to avoid duplication. The change likely needs to happen for the default value, as well as choices.
Something like:
default = "[Default: %s" % to_text(_coerce_bool_to_yaml(opt.pop('default', '(null)'))) + "]"
The test
The test
|
Closing in favor of #78668 |
Adopts JSON/YAML 1.2 standard booleans names.
Related: ansible-community/community-topics#116
SUMMARY
ISSUE TYPE
COMPONENT NAME
ADDITIONAL INFORMATION