Do not render empty code choosers on generated index page #2822
Conversation
…ite code chooser. Remove source replacement for all conversion errors.
…that section if so.
… for all possible combinations
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2822 +/- ##
==========================================
+ Coverage 68.65% 68.69% +0.04%
==========================================
Files 325 325
Lines 41537 41621 +84
==========================================
+ Hits 28516 28593 +77
- Misses 11418 11422 +4
- Partials 1603 1606 +3 ☔ View full report in Codecov by Sentry. |
pkg/tfgen/installation_docs.go
Outdated
|
|
||
| err := ast.Walk(astNode, func(n ast.Node, entering bool) (ast.WalkStatus, error) { | ||
| if section, ok := n.(*section.Section); ok && entering { | ||
| sectionText := section.Text([]byte(contentStr)) |
There was a problem hiding this comment.
Instead of converting to text and then checking if nothing came out, can we do this with a more structured check:
A section is empty if it has one child (the title). We can then get the content of the title and validate that.
| ## Example Usage | ||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
I thought a goal of this PR was to remove empty ## Example Usage sections like this.
There was a problem hiding this comment.
Yes! This test is only for the code conversion. We would still have an empty Examples section after code conversion.
guineveresaenger
force-pushed
the
guin/fix-empty-code-choosers
branch
from
05fe998 to
eb4af28
Compare
pkg/tfgen/installation_docs.go
Outdated
| err := ast.Walk(astNode, func(n ast.Node, entering bool) (ast.WalkStatus, error) { | ||
| if section, ok := n.(*section.Section); ok && entering { | ||
| sectionText := section.Text(contentBytes) | ||
| // A little confusingly, we check if the section text _only_ contains the title, "Example Usage". |
There was a problem hiding this comment.
| // A little confusingly, we check if the section text _only_ contains the title, "Example Usage". | |
| // A little confusingly, we check if the section text _only_ contains the title. |
| if diags.HasErrors() { | ||
| source = "Example currently unavailable in this language\n" | ||
| err = fmt.Errorf("failed to convert an example: %s", diags.Error()) | ||
| err = fmt.Errorf("conversion errors: %s", diags.Error()) | ||
| } | ||
|
|
||
| if source == "" { | ||
| source = exampleUnavailable | ||
| } |
There was a problem hiding this comment.
I don't understand why this change makes sense:
If there are no errors (!diag.HasErrors()), but source == "", should we really set source = exampleUnavailable.
There was a problem hiding this comment.
Two reasons:
-
It is possible for
diagto have no errors (for example, when the input is empty), which results insourcebeing empty. We can return an empty string, or declare that we currently have no example. -
It is also possible for
diagto have errors but the conversion being successful regardless.
Therefore it makes more sense to check for "what are we looking to display to the user" and if it comes up empty, say that.
There was a problem hiding this comment.
Can you put that in a comment on if source == "" {?
|
This PR has been shipped in release v3.102.0. |
This pull request does the following:
pclExampleobject for empty pulumi YAML or PCL explicitly and before attempting to convert, removing the empty string check from the converter functionsexampleUnavailableplaceholder constant for use when conversion fails on an individual languagesuccessfulConversionflag that flips to True as soon as any one language registers as having content other than theexampleUnavailablemessage. The idea is that if even only one pulumi language converted, we want to display the code chooser.Fixes #2819