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

Handle headers in code comments #2285

Merged
merged 10 commits into from
Aug 8, 2024
Merged

Conversation

iwahbe
Copy link
Member

@iwahbe iwahbe commented Aug 6, 2024

Correctly handle "markdown headers" in code blocks. We do this by using goldmark to parse the markdown and find headers of the right type, then use byte offsets to extract from the original text.

This allows us to correctly parse comments like:

## Example
```tf
## Minimal
resource foo "bar" {}
```

Fixes pulumi/pulumi-snowflake#683


I have tested this in aws, gcp, azure, cloudflare and snowflake. Snowflake fixes a great number of resources, gcp is broken due to a bad doc rewrite rule (which I will fix before merging here). The rest are unchanged.

I think we are ready to merge.

@iwahbe iwahbe requested review from guineveresaenger and a team August 6, 2024 15:24
@iwahbe iwahbe self-assigned this Aug 6, 2024
@VenelinMartinov
Copy link
Contributor

Is #2260 also fixed by this?

@iwahbe iwahbe force-pushed the iwahbe/handle-headers-in-code-comments branch from 4bdeccf to 5c24869 Compare August 6, 2024 16:10
Copy link

codecov bot commented Aug 6, 2024

Codecov Report

Attention: Patch coverage is 94.73684% with 5 lines in your changes missing coverage. Please review.

Project coverage is 56.76%. Comparing base (b40324a) to head (dacf7a1).
Report is 9 commits behind head on master.

Files Patch % Lines
pkg/tfgen/docs.go 94.50% 3 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2285      +/-   ##
==========================================
+ Coverage   56.71%   56.76%   +0.05%     
==========================================
  Files         361      361              
  Lines       49547    49745     +198     
==========================================
+ Hits        28099    28238     +139     
- Misses      19906    19954      +48     
- Partials     1542     1553      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@iwahbe
Copy link
Member Author

iwahbe commented Aug 7, 2024

Is #2260 also fixed by this?

I doubt it. This change is only concerned with correctly parsing ## H2 headers.

@iwahbe iwahbe force-pushed the iwahbe/handle-headers-in-code-comments branch from 405b544 to b366ae8 Compare August 7, 2024 11:04
@iwahbe iwahbe force-pushed the iwahbe/handle-headers-in-code-comments branch from b366ae8 to 128cbe1 Compare August 7, 2024 11:36
@iwahbe iwahbe force-pushed the iwahbe/handle-headers-in-code-comments branch from 28ab284 to 1145b5e Compare August 7, 2024 13:20
iwahbe added a commit to pulumi/pulumi-gcp that referenced this pull request Aug 8, 2024
This is a pre-requisite for
pulumi/pulumi-terraform-bridge#2285.

pulumi/pulumi-terraform-bridge#2285 uses a real
markdown parser. This correctly identifies `Collection of resources to
manage IAM policy for Dataproc AutoscalingPolicy` as a H2 header, just
like GH does.

```md
---
# ----------------------------------------------------------------------------
#
#     ***     AUTO GENERATED CODE    ***    Type: MMv1     ***
#
# ----------------------------------------------------------------------------
#
#     This file is automatically generated by Magic Modules and manual
#     changes will be clobbered when the file is regenerated.
#
#     Please read more about how to change this file in
#     .github/CONTRIBUTING.md.
#
# ----------------------------------------------------------------------------
## subcategory: "Dataproc"

description: |-
  Collection of resources to manage IAM policy for Dataproc AutoscalingPolicy
---
```

This PR removes the second front-matter from the copied doc.
Copy link
Contributor

@guineveresaenger guineveresaenger left a comment

Choose a reason for hiding this comment

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

I agree we can merge. I like that we've removed groupLines as well.

pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
pkg/tfgen/docs.go Outdated Show resolved Hide resolved
@iwahbe iwahbe enabled auto-merge (squash) August 8, 2024 11:48
@iwahbe iwahbe disabled auto-merge August 8, 2024 11:48
@iwahbe iwahbe enabled auto-merge (squash) August 8, 2024 11:49
@iwahbe iwahbe merged commit 8503fe8 into master Aug 8, 2024
11 checks passed
@iwahbe iwahbe deleted the iwahbe/handle-headers-in-code-comments branch August 8, 2024 12:07
@mjeffryes mjeffryes added this to the 0.108 milestone Aug 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rendering/Styling issue with API Docs
4 participants