From 36a0b90bcde26d3eab1b446acb4023ded470460d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Julian=20T=C3=B6lle?= Date: Sun, 1 Sep 2024 14:19:13 +0200 Subject: [PATCH] refactor: releasepr markdown handling (#42) --- internal/markdown/extensions/ast/section.go | 7 +- internal/markdown/extensions/section.go | 14 +- internal/markdown/goldmark.go | 83 +++++++ internal/markdown/goldmark_test.go | 252 ++++++++++++++++++++ internal/releasepr/releasepr.go | 83 +------ internal/releasepr/releasepr_test.go | 110 +++++++++ 6 files changed, 466 insertions(+), 83 deletions(-) create mode 100644 internal/markdown/goldmark_test.go diff --git a/internal/markdown/extensions/ast/section.go b/internal/markdown/extensions/ast/section.go index 43937c3..b336f9d 100644 --- a/internal/markdown/extensions/ast/section.go +++ b/internal/markdown/extensions/ast/section.go @@ -7,7 +7,8 @@ import ( // A Section struct represents a section of elements. type Section struct { gast.BaseBlock - Name string + Name string + Hidden bool } // Dump implements Node.Dump. @@ -26,6 +27,10 @@ func (n *Section) Kind() gast.NodeKind { return KindSection } +func (n *Section) HideInOutput() { + n.Hidden = true +} + // NewSection returns a new Section node. func NewSection(name string) *Section { return &Section{Name: name} diff --git a/internal/markdown/extensions/section.go b/internal/markdown/extensions/section.go index c8fdcb7..31e0b4d 100644 --- a/internal/markdown/extensions/section.go +++ b/internal/markdown/extensions/section.go @@ -21,8 +21,8 @@ var ( const ( sectionTrigger = "" - SectionEndFormat = "" + SectionStartFormat = "\n" + SectionEndFormat = "\n" ) type sectionParser struct{} @@ -91,6 +91,10 @@ func (s SectionMarkdownRenderer) RegisterFuncs(reg renderer.NodeRendererFuncRegi func (s SectionMarkdownRenderer) renderSection(w util.BufWriter, _ []byte, node gast.Node, enter bool) (gast.WalkStatus, error) { n := node.(*ast.Section) + if n.Hidden { + return gast.WalkContinue, nil + } + if enter { // Add blank previous line if applicable if node.PreviousSibling() != nil && node.HasBlankPreviousLines() { @@ -107,12 +111,10 @@ func (s SectionMarkdownRenderer) renderSection(w util.BufWriter, _ []byte, node return gast.WalkStop, fmt.Errorf(": %w", err) } - if _, err := w.WriteRune('\n'); err != nil { - return gast.WalkStop, err - } } - return gast.WalkContinue, nil + // Somehow the goldmark-markdown renderer does not flush this properly on its own + return gast.WalkContinue, w.Flush() } type section struct{} diff --git a/internal/markdown/goldmark.go b/internal/markdown/goldmark.go index 456fc55..16c7ce4 100644 --- a/internal/markdown/goldmark.go +++ b/internal/markdown/goldmark.go @@ -2,13 +2,17 @@ package markdown import ( "bytes" + "strings" markdown "github.com/teekennedy/goldmark-markdown" "github.com/yuin/goldmark" + gast "github.com/yuin/goldmark/ast" "github.com/yuin/goldmark/parser" + "github.com/yuin/goldmark/text" "github.com/yuin/goldmark/util" "github.com/apricote/releaser-pleaser/internal/markdown/extensions" + "github.com/apricote/releaser-pleaser/internal/markdown/extensions/ast" ) func New() goldmark.Markdown { @@ -34,3 +38,82 @@ func Format(input string) (string, error) { return buf.String(), nil } + +func GetCodeBlockText(source []byte, language string, output *string) gast.Walker { + return func(n gast.Node, entering bool) (gast.WalkStatus, error) { + if !entering { + return gast.WalkContinue, nil + } + + if n.Kind() != gast.KindFencedCodeBlock { + return gast.WalkContinue, nil + } + + codeBlock := n.(*gast.FencedCodeBlock) + + if string(codeBlock.Language(source)) != language { + return gast.WalkContinue, nil + } + + *output = textFromLines(source, codeBlock) + // Stop looking after we find the first result + return gast.WalkStop, nil + } +} + +func GetSectionText(source []byte, name string, output *string) gast.Walker { + return func(n gast.Node, entering bool) (gast.WalkStatus, error) { + if !entering { + return gast.WalkContinue, nil + } + + if n.Kind() != ast.KindSection { + return gast.WalkContinue, nil + } + + section := n.(*ast.Section) + + if section.Name != name { + return gast.WalkContinue, nil + } + + // Do not show section markings in output, we only care about the content + section.HideInOutput() + + // Found the right section + outputBuffer := new(bytes.Buffer) + err := New().Renderer().Render(outputBuffer, source, section) + if err != nil { + return gast.WalkStop, err + } + + *output = outputBuffer.String() + // Stop looking after we find the first result + return gast.WalkStop, nil + } +} + +func textFromLines(source []byte, n gast.Node) string { + content := make([]byte, 0) + + l := n.Lines().Len() + for i := 0; i < l; i++ { + line := n.Lines().At(i) + content = append(content, line.Value(source)...) + } + + return strings.TrimSpace(string(content)) +} + +func WalkAST(source []byte, walkers ...gast.Walker) (err error) { + doc := New().Parser().Parse(text.NewReader(source)) + + for _, walker := range walkers { + err = gast.Walk(doc, walker) + if err != nil { + return err + } + } + + return nil +} diff --git a/internal/markdown/goldmark_test.go b/internal/markdown/goldmark_test.go new file mode 100644 index 0000000..3002994 --- /dev/null +++ b/internal/markdown/goldmark_test.go @@ -0,0 +1,252 @@ +package markdown + +import ( + "errors" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/yuin/goldmark/ast" +) + +func TestFormat(t *testing.T) { + tests := []struct { + name string + input string + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "heading spacing", + input: "# Foo\n## Bar\n### Baz", + want: "# Foo\n\n## Bar\n\n### Baz\n", + wantErr: assert.NoError, + }, + { + name: "no empty lines for list items", + input: "# Foo\n- 1\n- 2\n", + want: "# Foo\n\n- 1\n- 2\n", + wantErr: assert.NoError, + }, + { + name: "sections", + input: "# Foo\n\n- 1\n- 2\n\n", + want: "# Foo\n\n\n- 1\n- 2\n\n\n", + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := Format(tt.input) + if !tt.wantErr(t, err) { + return + } + assert.Equal(t, tt.want, got) + }) + } +} + +func TestGetCodeBlockText(t *testing.T) { + type args struct { + source []byte + language string + } + tests := []struct { + name string + args args + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "no code block", + args: args{ + source: []byte("# Foo"), + language: "missing", + }, + want: "", + wantErr: assert.NoError, + }, + { + name: "code block", + args: args{ + source: []byte("```test\nContent\n```"), + language: "test", + }, + want: "Content", + wantErr: assert.NoError, + }, + { + name: "code block with other language", + args: args{ + source: []byte("```unknown\nContent\n```"), + language: "test", + }, + want: "", + wantErr: assert.NoError, + }, + { + name: "multiple code blocks with different languages", + args: args{ + source: []byte("```unknown\nContent\n```\n\n```test\n1337\n```"), + language: "test", + }, + want: "1337", + wantErr: assert.NoError, + }, + { + name: "multiple code blocks with same language returns first one", + args: args{ + source: []byte("```test\nContent\n```\n\n```test\n1337\n```"), + language: "test", + }, + want: "Content", + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got string + + err := WalkAST(tt.args.source, + GetCodeBlockText(tt.args.source, tt.args.language, &got), + ) + if !tt.wantErr(t, err) { + return + } + + assert.Equal(t, tt.want, got) + }) + } +} + +func TestGetSectionText(t *testing.T) { + type args struct { + source []byte + name string + } + tests := []struct { + name string + args args + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "no section", + args: args{ + source: []byte("# Foo"), + name: "missing", + }, + want: "", + wantErr: assert.NoError, + }, + { + name: "section", + args: args{ + source: []byte("\nContent\n"), + name: "test", + }, + want: "Content\n", + wantErr: assert.NoError, + }, + { + name: "section with other name", + args: args{ + source: []byte("\nContent\n"), + name: "test", + }, + want: "", + wantErr: assert.NoError, + }, + { + name: "multiple sections with different names", + args: args{ + source: []byte("\nContent\n\n\n\n1337\n"), + name: "test", + }, + want: "1337\n", + wantErr: assert.NoError, + }, + { + name: "multiple sections with same name returns first one", + args: args{ + source: []byte("\nContent\n\n\n\n1337\n"), + name: "test", + }, + want: "Content\n", + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var got string + + err := WalkAST(tt.args.source, + GetSectionText(tt.args.source, tt.args.name, &got), + ) + if !tt.wantErr(t, err) { + return + } + + assert.Equal(t, tt.want, got) + }) + } +} + +func TestWalkAST(t *testing.T) { + type args struct { + source []byte + walkers []ast.Walker + } + tests := []struct { + name string + args args + wantErr assert.ErrorAssertionFunc + }{ + { + name: "empty walker", + args: args{ + source: []byte("# Foo"), + walkers: []ast.Walker{ + func(_ ast.Node, _ bool) (ast.WalkStatus, error) { + return ast.WalkStop, nil + }, + }, + }, + wantErr: assert.NoError, + }, + { + name: "returns walker error", + args: args{ + source: []byte("# Foo"), + walkers: []ast.Walker{ + func(_ ast.Node, _ bool) (ast.WalkStatus, error) { + return ast.WalkStop, errors.New("test") + }, + }, + }, + wantErr: assert.Error, + }, + { + name: "runs all walkers", + args: args{ + source: []byte("# Foo"), + walkers: []ast.Walker{ + func(_ ast.Node, _ bool) (ast.WalkStatus, error) { + return ast.WalkStop, nil + }, + func(_ ast.Node, _ bool) (ast.WalkStatus, error) { + return ast.WalkStop, errors.New("test") + }, + }, + }, + wantErr: assert.Error, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := WalkAST(tt.args.source, tt.args.walkers...) + if !tt.wantErr(t, err) { + return + } + }) + } +} diff --git a/internal/releasepr/releasepr.go b/internal/releasepr/releasepr.go index 5bf3649..c19451e 100644 --- a/internal/releasepr/releasepr.go +++ b/internal/releasepr/releasepr.go @@ -6,15 +6,10 @@ import ( "fmt" "log" "regexp" - "strings" "text/template" - "github.com/yuin/goldmark/ast" - "github.com/yuin/goldmark/text" - "github.com/apricote/releaser-pleaser/internal/git" "github.com/apricote/releaser-pleaser/internal/markdown" - ast2 "github.com/apricote/releaser-pleaser/internal/markdown/extensions/ast" "github.com/apricote/releaser-pleaser/internal/versioning" ) @@ -140,31 +135,11 @@ func (pr *ReleasePullRequest) parseVersioningFlags(overrides ReleaseOverrides) R func (pr *ReleasePullRequest) parseDescription(overrides ReleaseOverrides) (ReleaseOverrides, error) { source := []byte(pr.Description) - descriptionAST := markdown.New().Parser().Parse(text.NewReader(source)) - err := ast.Walk(descriptionAST, func(n ast.Node, entering bool) (ast.WalkStatus, error) { - if !entering { - return ast.WalkContinue, nil - } - - if n.Type() != ast.TypeBlock || n.Kind() != ast.KindFencedCodeBlock { - return ast.WalkContinue, nil - } - - codeBlock, ok := n.(*ast.FencedCodeBlock) - if !ok { - return ast.WalkStop, fmt.Errorf("node has unexpected type: %T", n) - } - - switch string(codeBlock.Language(source)) { - case DescriptionLanguagePrefix: - overrides.Prefix = textFromLines(source, codeBlock) - case DescriptionLanguageSuffix: - overrides.Suffix = textFromLines(source, codeBlock) - } - - return ast.WalkContinue, nil - }) + err := markdown.WalkAST(source, + markdown.GetCodeBlockText(source, DescriptionLanguagePrefix, &overrides.Prefix), + markdown.GetCodeBlockText(source, DescriptionLanguageSuffix, &overrides.Suffix), + ) if err != nil { return ReleaseOverrides{}, err } @@ -174,59 +149,15 @@ func (pr *ReleasePullRequest) parseDescription(overrides ReleaseOverrides) (Rele func (pr *ReleasePullRequest) ChangelogText() (string, error) { source := []byte(pr.Description) - gm := markdown.New() - descriptionAST := gm.Parser().Parse(text.NewReader(source)) - var section *ast2.Section - - err := ast.Walk(descriptionAST, func(n ast.Node, entering bool) (ast.WalkStatus, error) { - if !entering { - return ast.WalkContinue, nil - } - - if n.Type() != ast.TypeBlock || n.Kind() != ast2.KindSection { - return ast.WalkContinue, nil - } - - anySection, ok := n.(*ast2.Section) - if !ok { - return ast.WalkStop, fmt.Errorf("node has unexpected type: %T", n) - } - - if anySection.Name != MarkdownSectionChangelog { - return ast.WalkContinue, nil - } - - section = anySection - return ast.WalkStop, nil - }) + var sectionText string + err := markdown.WalkAST(source, markdown.GetSectionText(source, MarkdownSectionChangelog, §ionText)) if err != nil { return "", err } - if section == nil { - return "", nil - } + return sectionText, nil - outputBuffer := new(bytes.Buffer) - err = gm.Renderer().Render(outputBuffer, source, section) - if err != nil { - return "", err - } - - return outputBuffer.String(), nil -} - -func textFromLines(source []byte, n ast.Node) string { - content := make([]byte, 0) - - l := n.Lines().Len() - for i := 0; i < l; i++ { - line := n.Lines().At(i) - content = append(content, line.Value(source)...) - } - - return strings.TrimSpace(string(content)) } func (pr *ReleasePullRequest) SetTitle(branch, version string) { diff --git a/internal/releasepr/releasepr_test.go b/internal/releasepr/releasepr_test.go index 92f338d..64295de 100644 --- a/internal/releasepr/releasepr_test.go +++ b/internal/releasepr/releasepr_test.go @@ -1,11 +1,121 @@ package releasepr import ( + "fmt" "testing" "github.com/stretchr/testify/assert" + + "github.com/apricote/releaser-pleaser/internal/versioning" ) +func TestReleasePullRequest_GetOverrides(t *testing.T) { + tests := []struct { + name string + pr ReleasePullRequest + want ReleaseOverrides + wantErr assert.ErrorAssertionFunc + }{ + { + name: "empty", + pr: ReleasePullRequest{}, + want: ReleaseOverrides{}, + wantErr: assert.NoError, + }, + { + // TODO: Test for multiple version flags + name: "single version flag", + pr: ReleasePullRequest{ + Labels: []Label{LabelNextVersionTypeAlpha}, + }, + want: ReleaseOverrides{ + NextVersionType: versioning.NextVersionTypeAlpha, + }, + wantErr: assert.NoError, + }, + { + name: "prefix in description", + pr: ReleasePullRequest{ + Description: "```rp-prefix\n## Foo\n\n- Cool thing\n```", + }, + want: ReleaseOverrides{Prefix: "## Foo\n\n- Cool thing"}, + wantErr: assert.NoError, + }, + { + name: "suffix in description", + pr: ReleasePullRequest{ + Description: "```rp-suffix\n## Compatibility\n\nNo compatibility guarantees.\n```", + }, + want: ReleaseOverrides{Suffix: "## Compatibility\n\nNo compatibility guarantees."}, + wantErr: assert.NoError, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := tt.pr.GetOverrides() + if !tt.wantErr(t, err, fmt.Sprintf("GetOverrides()")) { + return + } + assert.Equalf(t, tt.want, got, "GetOverrides()") + }) + } +} + +func TestReleasePullRequest_ChangelogText(t *testing.T) { + tests := []struct { + name string + description string + want string + wantErr assert.ErrorAssertionFunc + }{ + { + name: "no section", + description: "# Foo\n", + want: "", + wantErr: assert.NoError, + }, + { + name: "with section", + description: `# Foobar + + +This is the changelog + +## Awesome + +### New + +#### Changes + + +Suffix Things +`, + want: `This is the changelog + +## Awesome + +### New + +#### Changes +`, + wantErr: assert.NoError, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + pr := &ReleasePullRequest{ + Description: tt.description, + } + got, err := pr.ChangelogText() + if !tt.wantErr(t, err, fmt.Sprintf("ChangelogText()")) { + return + } + assert.Equalf(t, tt.want, got, "ChangelogText()") + }) + } +} + func TestReleasePullRequest_SetTitle(t *testing.T) { type args struct { branch string