Skip to content
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

Use code formatting for all values (choice entries, defaults, samples) #38

Closed
felixfontein opened this issue Sep 22, 2022 · 9 comments · Fixed by #42
Closed

Use code formatting for all values (choice entries, defaults, samples) #38

felixfontein opened this issue Sep 22, 2022 · 9 comments · Fixed by #42
Labels
enhancement New feature or request

Comments

@felixfontein
Copy link
Collaborator

Right now, while values in the description that are (correctly) formtated with C(...) have code / teletype formatting, other auto-generated values such as choice list entries, defaults, and samples, are formatted as regular text (only with different colors).

@briantist suggested (#36 (comment)) to format choice list entries also as code, and I extended that suggestion to format also defaults and samples values as code.

What do you think?

@felixfontein
Copy link
Collaborator Author

Output right now: https://docs.ansible.com/ansible/devel/collections/ansible/builtin/apt_module.html#parameter-purge As you can see, values inside descriptions use <code>, while values in choice lists and defaults do not. The same is true for sample values: https://docs.ansible.com/ansible/devel/collections/ansible/builtin/apt_module.html#return-values

@tremble
Copy link

tremble commented Sep 22, 2022

Would this mean dropping the 'bold' for the default value?, or would you have a slightly tweaked CSS for the "default value" code block?

@felixfontein
Copy link
Collaborator Author

I would keep the other formatting. (The sphinx extension will need to provide more roles, but we already collected some experience with this in the semantic markup PR :) )

@felixfontein
Copy link
Collaborator Author

I've created a prototype implementation, currently based on the semantic markup branch. I'll convert this to something based on main if there are some more approvals :)

Using code style:
Code style

Right now it looks like this:
Current style

@samccann
Copy link
Contributor

Consistency is a good thing :-) +1 for making it consistent.

Since we are past core feature freeze, I would suggest we hold off on implementing until/unless we have to implement some other must-have fix into antsibull-docs for stable-2.14. Even then, we are kind of 'stretching' the limits of what we should be doing. Being good Ansible citizens would suggest the only thing we put into antsibull-doc for stable-2.14 are bugfixes, and even that, we are running out of time as beta happens on Monday and RC I think the week after? So we likely need to code-freeze antsibull-doc before the end of Sept.

@felixfontein
Copy link
Collaborator Author

Since we are past core feature freeze, I would suggest we hold off on implementing until/unless we have to implement some other must-have fix into antsibull-docs for stable-2.14.

It would be a lot easier if ansible-core would not start using new constructs in docs that require new features that aren't available in antsibull-docs after feature freeze. Then we could have finished this stuff already weeks ago. This change discussed here is mainly for improving the rendering of the combine filter docs in https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/combine.yml#L26-L32, that with the currently released antsibull-docs version (that stable-2.14 and devel use) results in a rendering error (https://docs.ansible.com/ansible/devel/collections/ansible/builtin/combine_filter.html).

@briantist
Copy link
Contributor

well, biased I guess since I suggested this, but to my eyes the code formatting is way better for choices and defaults.

I would've suggested removing the quotes from string values, but I suppose that is useful to distinguish between strings and non-strings, ("5" vs 5, "true" vs. true), sort of a repr() view.

@felixfontein
Copy link
Collaborator Author

@briantist the quotes are there because these values are formatted as JSON. I guess we could also use YAML (one-line) serialization, but that is another discussion I guess ;)

@felixfontein
Copy link
Collaborator Author

I created a PR for this: #42

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants