Skip to content

Commit

Permalink
Add table renderer to render tables as Markdown. Add tests. (#2603)
Browse files Browse the repository at this point in the history
The markdown renderer we use does [not support rendering out
tables](teekennedy/goldmark-markdown#19). This
pull request adds a table renderer, since the raw HTML rendering is
lossy and does not handle inline code blocks well.

With this change, a table will properly render out inline code blocks
inside table cells, and correctly handle tables as separate elements.

Example from NewRelic's `docs/_index.md` page with this change applied:

<img width="898" alt="Screenshot 2024-11-07 at 5 07 42 PM"
src="https://github.com/user-attachments/assets/01392204-814e-4469-ac94-c0c03832a9c6">
  • Loading branch information
guineveresaenger authored Nov 8, 2024
1 parent 52a2e41 commit af9ccb5
Show file tree
Hide file tree
Showing 10 changed files with 268 additions and 1 deletion.
1 change: 1 addition & 0 deletions dynamic/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ require (
github.com/hashicorp/logutils v1.0.0 // indirect
github.com/hashicorp/terraform-plugin-sdk/v2 v2.33.0 // indirect
github.com/nxadm/tail v1.4.11 // indirect
github.com/olekukonko/tablewriter v0.0.5 // indirect
github.com/pulumi/terraform-diff-reader v0.0.2 // indirect
github.com/teekennedy/goldmark-markdown v0.3.0 // indirect
github.com/vmihailenco/msgpack v4.0.4+incompatible // indirect
Expand Down
3 changes: 3 additions & 0 deletions dynamic/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1787,6 +1787,7 @@ github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D
github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4=
github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88=
github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
github.com/mattn/go-runewidth v0.0.15 h1:UNAjwbU9l54TA3KzvqLGxwWjHmMgBUVhBiTjelZgg3U=
github.com/mattn/go-runewidth v0.0.15/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
Expand Down Expand Up @@ -1850,6 +1851,8 @@ github.com/nxadm/tail v1.4.11/go.mod h1:OTaG3NK980DZzxbRq6lEuzgU+mug70nY11sMd4JX
github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU=
github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=
github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
Expand Down
3 changes: 2 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ module github.com/pulumi/pulumi-terraform-bridge/v3

go 1.22.0

toolchain go1.23.1
toolchain go1.23.2

require (
github.com/apparentlymart/go-cidr v1.1.0
Expand Down Expand Up @@ -39,6 +39,7 @@ require (
github.com/mitchellh/hashstructure v1.0.0
github.com/mitchellh/mapstructure v1.5.0
github.com/mitchellh/reflectwalk v1.0.2
github.com/olekukonko/tablewriter v0.0.5
github.com/pkg/errors v0.9.1
github.com/pulumi/inflector v0.1.1
github.com/pulumi/providertest v0.1.3
Expand Down
3 changes: 3 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1790,6 +1790,7 @@ github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D
github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4=
github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88=
github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
github.com/mattn/go-runewidth v0.0.15 h1:UNAjwbU9l54TA3KzvqLGxwWjHmMgBUVhBiTjelZgg3U=
github.com/mattn/go-runewidth v0.0.15/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
Expand Down Expand Up @@ -1853,6 +1854,8 @@ github.com/nxadm/tail v1.4.11/go.mod h1:OTaG3NK980DZzxbRq6lEuzgU+mug70nY11sMd4JX
github.com/oklog/run v1.0.0/go.mod h1:dlhp/R75TPv97u0XWUtDeV/lRKWPKSdTuV0TZvrmrQA=
github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU=
github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=
github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
github.com/onsi/ginkgo v1.6.0/go.mod h1:lLunBs/Ym6LB5Z9jYTR76FiuTmxDTDusOGeTQH+WWjE=
github.com/onsi/ginkgo v1.12.1/go.mod h1:zj2OWP4+oCPe1qIXoGWkgMRwljMUYCdkwsT2108oapk=
github.com/onsi/ginkgo v1.16.4/go.mod h1:dX+/inL/fNMqNlz0e9LfyB9TswhZpCVdJM/Z6Vvnwo0=
Expand Down
1 change: 1 addition & 0 deletions pf/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ require (
github.com/muesli/cancelreader v0.2.2 // indirect
github.com/natefinch/atomic v1.0.1 // indirect
github.com/oklog/run v1.1.0 // indirect
github.com/olekukonko/tablewriter v0.0.5 // indirect
github.com/opentracing/basictracer-go v1.1.0 // indirect
github.com/opentracing/opentracing-go v1.2.0 // indirect
github.com/pgavlin/goldmark v1.1.33-0.20200616210433-b5eb04559386 // indirect
Expand Down
3 changes: 3 additions & 0 deletions pf/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -636,6 +636,7 @@ github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D
github.com/mattn/go-localereader v0.0.1 h1:ygSAOl7ZXTx4RdPYinUpg6W99U8jWvWi9Ye2JC/oIi4=
github.com/mattn/go-localereader v0.0.1/go.mod h1:8fBrzywKY7BI3czFoHkuzRoWE9C+EiG4R1k4Cjx5p88=
github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU=
github.com/mattn/go-runewidth v0.0.9/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m2gUSrubnMI=
github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
github.com/mattn/go-runewidth v0.0.15 h1:UNAjwbU9l54TA3KzvqLGxwWjHmMgBUVhBiTjelZgg3U=
github.com/mattn/go-runewidth v0.0.15/go.mod h1:Jdepj2loyihRzMpdS35Xk/zdY8IAYHsh153qUoGf23w=
Expand Down Expand Up @@ -679,6 +680,8 @@ github.com/nightlyone/lockfile v1.0.0 h1:RHep2cFKK4PonZJDdEl4GmkabuhbsRMgk/k3uAm
github.com/nightlyone/lockfile v1.0.0/go.mod h1:rywoIealpdNse2r832aiD9jRk8ErCatROs6LzC841CI=
github.com/oklog/run v1.1.0 h1:GEenZ1cK0+q0+wsJew9qUg/DyD8k3JzYsZAi5gYi2mA=
github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DVU=
github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=
github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI=
github.com/onsi/gomega v1.27.10/go.mod h1:RsS8tutOdbdgzbPtzzATp12yT7kM5I5aElG3evPbQ0M=
github.com/opentracing/basictracer-go v1.1.0 h1:Oa1fTSBvAl8pa3U+IJYqrKm0NALwH9OsgwOqDv4xJW0=
Expand Down
132 changes: 132 additions & 0 deletions pkg/tfgen/parse/extension.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,18 @@ package parse

import (
"bytes"
"fmt"
"strings"

"github.com/olekukonko/tablewriter"
"github.com/pulumi/pulumi/sdk/v3/go/common/util/contract"
markdown "github.com/teekennedy/goldmark-markdown"
"github.com/yuin/goldmark"
"github.com/yuin/goldmark/ast"
"github.com/yuin/goldmark/extension"
extensionast "github.com/yuin/goldmark/extension/ast"
"github.com/yuin/goldmark/parser"
"github.com/yuin/goldmark/renderer"
"github.com/yuin/goldmark/text"
"github.com/yuin/goldmark/util"

Expand All @@ -37,12 +43,42 @@ var TFRegistryExtension goldmark.Extender = tfRegistryExtension{}
type tfRegistryExtension struct{}

func (s tfRegistryExtension) Extend(md goldmark.Markdown) {
md.SetRenderer(markdown.NewRenderer())
extension.GFM.Extend(md)
section.Extension.Extend(md)
md.Parser().AddOptions(
parser.WithASTTransformers(
util.Prioritized(recognizeHeaderAfterHTML{}, 902),
))
md.Renderer().AddOptions(renderer.WithNodeRenderers(
// The markdown renderer we use does not support rendering out tables.[^1].
//
// Without intervention, tables are rendered out as HTML. This works fine
// for the registry (since it renders HTML). It works poorly for SDK docs,
// since the HTML content is shown as-is.
//
// [^1]: https://github.com/teekennedy/goldmark-markdown/issues/19
util.Prioritized(tableRenderer{md.Renderer()}, 499),
// The markdown renderer we use does not support rendering raw
// [ast.String] nodes.[^2] We just render them out as-is.
//
//nolint:lll
//
// [^2]: https://github.com/teekennedy/goldmark-markdown/blob/0cdef017688474073914d6db7e293a028150c0cb/renderer.go#L95-L96
util.Prioritized(renderType{
kind: ast.KindString,
f: func(
writer util.BufWriter,
_ []byte, n ast.Node, entering bool,
) (ast.WalkStatus, error) {
if !entering {
return ast.WalkContinue, nil
}
_, err := writer.Write(n.(*ast.String).Value)
return ast.WalkContinue, err
},
}, 100),
))
}

// recognizeHeaderAfterHTML allows us to work around a difference in how TF's registry parses
Expand Down Expand Up @@ -85,3 +121,99 @@ func WalkNode[T ast.Node](node ast.Node, f func(T)) {
})
contract.AssertNoErrorf(err, "impossible: ast.Walk never returns an error")
}

type renderType struct {
kind ast.NodeKind
f renderer.NodeRendererFunc
}

func (renderType renderType) RegisterFuncs(r renderer.NodeRendererFuncRegisterer) {
r.Register(renderType.kind, renderType.f)
}

func panicOnRender(writer util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) {
contract.Failf("The renderer for %s should not have been called", n.Kind())
return ast.WalkStop, nil
}

var _ renderer.NodeRenderer = (*tableRenderer)(nil)

type tableRenderer struct {
renderer.Renderer
}

func (tableRenderer tableRenderer) RegisterFuncs(r renderer.NodeRendererFuncRegisterer) {
r.Register(extensionast.KindTable, tableRenderer.render)
r.Register(extensionast.KindTableHeader, panicOnRender)
r.Register(extensionast.KindTableRow, panicOnRender)
r.Register(extensionast.KindTableCell,
func(writer util.BufWriter, source []byte, n ast.Node, entering bool) (ast.WalkStatus, error) {
return ast.WalkContinue, nil
})
}

func (tableRenderer tableRenderer) render(
writer util.BufWriter, source []byte, n ast.Node, entering bool,
) (ast.WalkStatus, error) {
if !entering {
return ast.WalkSkipChildren, nil
}
_, err := writer.WriteRune('\n') // this is so that we have a newline between markdown elements.
contract.AssertNoErrorf(err, "impossible")
var inHeader bool
header := make([]string, 0, len(n.(*extensionast.Table).Alignments))
var rows [][]string
err = ast.Walk(n, func(n ast.Node, entering bool) (ast.WalkStatus, error) {
switch n := n.(type) {
case *extensionast.Table:
return ast.WalkContinue, nil
case *extensionast.TableHeader:
inHeader = entering
return ast.WalkContinue, nil
case *extensionast.TableRow:
if entering {
rows = append(rows, make([]string, 0, len(n.Alignments)))
}
return ast.WalkContinue, nil
case *extensionast.TableCell:
if entering {
var cell bytes.Buffer
textBlock := ast.NewTextBlock()
child := n.FirstChild()
for {
if child == nil {
break
}
next := child.NextSibling()
textBlock.AppendChild(textBlock, child)
child = next
}
err := tableRenderer.Render(&cell, source, textBlock)
if err != nil {
return ast.WalkStop, err
}
content := strings.TrimSpace(cell.String())
if inHeader {
header = append(header, content)
} else {
rows[len(rows)-1] = append(rows[len(rows)-1], content)
}
}
return ast.WalkSkipChildren, nil
default:
return ast.WalkStop, fmt.Errorf("unexpected node in a table: %s", n.Kind())
}
})
table := tablewriter.NewWriter(writer)
table.SetHeader(header)
table.SetBorders(tablewriter.Border{Left: true, Top: false, Right: true, Bottom: false})
table.SetCenterSeparator("|")
table.SetAutoFormatHeaders(false)
table.SetAutoMergeCells(false)
table.SetAutoWrapText(false)
table.SetReflowDuringAutoWrap(false)
table.AppendBulk(rows)
table.Render()

return ast.WalkSkipChildren, err
}
106 changes: 106 additions & 0 deletions pkg/tfgen/parse/extension_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// Copyright 2016-2024, Pulumi Corporation.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

package parse

import (
"bytes"
"os"
"runtime"
"testing"

"github.com/hexops/autogold/v2"
"github.com/stretchr/testify/require"
"github.com/yuin/goldmark"
)

func TestRenderTable(t *testing.T) {
t.Parallel()
if runtime.GOOS == "windows" {
t.Skipf("Skipping on windows to avoid failing on incorrect newline handling")
}

tests := []struct {
name string
input string
expected autogold.Value
}{
{
name: "basic",
input: `# hi
| t1 | t2 |
|---|---|
| r1c1 | r1c2 |
| r2c1 | r2c2 |
`,
expected: autogold.Expect(`# hi
| t1 | t2 |
|------|------|
| r1c1 | r1c2 |
| r2c1 | r2c2 |
`),
},
{
name: "with-in table effects",
input: `
| t1 | *t2* |
|------|------|
| __r1c1__ | r1c2 |
| r2c1 | r2c2 |
`,
expected: autogold.Expect(`
| t1 | *t2* |
|----------|------|
| **r1c1** | r1c2 |
| r2c1 | r2c2 |
`),
},
{
name: "with-codeblock-after-text",
input: "| t1 | *t2* |\n" +
"|------|-------------------|\n" +
"| r2c1 | r1c2 `code_block` |\n",
expected: autogold.Expect("\n" +
"| t1 | *t2* |\n" +
"|------|-------------------|\n" +
"| r2c1 | r1c2 `code_block` |\n"),
},
{
name: "from-newrelic",
input: readfile(t, "../test_data/table-rendering/input.md"),
expected: autogold.Expect(readfile(t, "../test_data/table-rendering/expected.md")),
},
}

for _, tt := range tests {
tt := tt
t.Run(tt.name, func(t *testing.T) {
var out bytes.Buffer
err := goldmark.New(
goldmark.WithExtensions(TFRegistryExtension),
).Convert([]byte(tt.input), &out)
require.NoError(t, err)
tt.expected.Equal(t, out.String())
})
}
}

func readfile(t *testing.T, file string) string {
t.Helper()
bytes, err := os.ReadFile(file)
require.NoError(t, err)
return string(bytes)
}
9 changes: 9 additions & 0 deletions pkg/tfgen/test_data/table-rendering/expected.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

| Argument | Required? | Description |
|------------------------|-----------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `account_id` | Required | Your New Relic account ID. The `NEW_RELIC_ACCOUNT_ID` environment variable can also be used. |
| `api_key` | Required | Your New Relic Personal API key (usually prefixed with `NRAK`). The `NEW_RELIC_API_KEY` environment variable can also be used. |
| `region` | Optional | The region for the data center for which your New Relic account is configured. The `NEW_RELIC_REGION` environment variable can also be used. Valid values are `US` or `EU`. Default value is `US`. |
| `insecure_skip_verify` | Optional | Trust self-signed SSL certificates. If omitted, the `NEW_RELIC_API_SKIP_VERIFY` environment variable is used. |
| `insights_insert_key` | Optional | Your Insights insert key used when inserting Insights events via the `newrelic_insights_event` resource. Can also use `NEW_RELIC_INSIGHTS_INSERT_KEY` environment variable. |
| `cacert_file` | Optional | A path to a PEM-encoded certificate authority used to verify the remote agent's certificate. The `NEW_RELIC_API_CACERT` environment variable can also be used. |
8 changes: 8 additions & 0 deletions pkg/tfgen/test_data/table-rendering/input.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
| Argument | Required? | Description |
| ---------------------- | --------- |----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
| `account_id` | Required | Your New Relic account ID. The `NEW_RELIC_ACCOUNT_ID` environment variable can also be used. |
| `api_key` | Required | Your New Relic Personal API key (usually prefixed with `NRAK`). The `NEW_RELIC_API_KEY` environment variable can also be used. |
| `region` | Optional | The region for the data center for which your New Relic account is configured. The `NEW_RELIC_REGION` environment variable can also be used. Valid values are `US` or `EU`. Default value is `US`. |
| `insecure_skip_verify` | Optional | Trust self-signed SSL certificates. If omitted, the `NEW_RELIC_API_SKIP_VERIFY` environment variable is used. |
| `insights_insert_key` | Optional | Your Insights insert key used when inserting Insights events via the `newrelic_insights_event` resource. Can also use `NEW_RELIC_INSIGHTS_INSERT_KEY` environment variable. |
| `cacert_file` | Optional | A path to a PEM-encoded certificate authority used to verify the remote agent's certificate. The `NEW_RELIC_API_CACERT` environment variable can also be used. |

0 comments on commit af9ccb5

Please sign in to comment.