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

Fix incorrect rendering of example and import files for providers with no templates or fallback templates #300

Merged
merged 11 commits into from
Dec 6, 2023

Conversation

SBGoods
Copy link
Contributor

@SBGoods SBGoods commented Nov 21, 2023

Fixes: #9
Fixes: #191
Fixes: #193
Supersedes: #185

The renderMissingResourceDoc() and renderMissingProviderDoc() methods determine if there are any existing templates for a given resource/datasource and if not, generates new templates either by using the default templates or by generating templates from generic "fallback" templates if they exist (i.e. templates/resources.tmpl.md). When new templates are generated in these methods, they are rendered to .md files. After these methods are called, the renderStaticWebsite() method is called to render the template files to .md files and copy them into the rendered website directory. This means that templates generated during the renderMissingResourceDoc() and renderMissingProviderDoc() methods are rendered to markdown twice leading to confusing and inconsistent behavior around the printf function which should print a string literal but would instead render the string as actual markdown.

The default templates use printf to show example syntax of how to call the codefile and tffile functions:

## Import

Import is supported using the following syntax:

{{ printf "{{codefile \"shell\" %q}}" .ImportFile }}
{{- end }}

but because of this bug, they ended up rendering as actual markdown:

Import is supported using the following syntax:

```shell
terraform import scaffolding_example.example
```

instead of the expected string literal:

Import is supported using the following syntax:

{{codefile "shell" .ImportFile }}

The changes in cmd/tfplugindocs/testdata/scripts/schema-json/generate/framework_provider_success_generic_templates.txtar demonstrate this difference.

This PR fixes the bug by removing template rendering code from renderMissingResourceDoc() and renderMissingProviderDoc() methods and refactoring these methods to make their purposes clearer. New acceptance tests have been added to test the behavior of static files that skip new template generation.

Because there may be developers using this tool who have relied on the default templates to generate docs with rendered examples, I choose to change the default templates to accommodate this after fixing the bug.

@SBGoods SBGoods requested a review from a team as a code owner November 21, 2023 19:52
@SBGoods SBGoods added this to the v0.17.0 milestone Nov 21, 2023
Copy link
Contributor

@bendbennett bendbennett left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🚀

Only question I had was whether there are any instances in which these changes will result in different behaviour, and if so, whether this should be documented?

@SBGoods SBGoods merged commit ba0074f into main Dec 6, 2023
6 checks passed
@SBGoods SBGoods deleted the SBGoods/fix-printf-rendering branch December 6, 2023 19:22
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants