Skip to content

Commit ad55639

Browse files
committed
pkg/md: Add more tests against reflowing.
Also fix a bug where the reflow can result in lines one unit wider than the budget.
1 parent 03beb02 commit ad55639

File tree

6 files changed

+155
-30
lines changed

6 files changed

+155
-30
lines changed

pkg/md/fmt.go

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -662,15 +662,39 @@ func (c *FmtCodec) writeSegmentsParagraphReflow(segs []segment, maxWidth int) {
662662
if currentLine.Len() == 0 {
663663
currentLine.WriteString(span)
664664
currentLineWidth = w
665-
} else if currentLineWidth+1+w <= maxWidth {
666-
currentLine.WriteByte(' ')
667-
currentLine.WriteString(span)
668-
currentLineWidth += 1 + w
669665
} else {
670-
writeCurrentLine()
671-
startNewLine()
672-
currentLine.WriteString(span)
673-
currentLineWidth = w
666+
// Determine whether the current span fits onto the current line.
667+
//
668+
// One slightly tricky detail here is that c.escapeStartOfLine may
669+
// insert more text, making the line wider. In reflow mode, the line
670+
// never starts or ends with whitespaces, so the most we have to
671+
// worry about is one backslash.
672+
//
673+
// As a result, if the line's width is exactly maxWidth after
674+
// appending the current span, we need to be extra careful and only
675+
// consider the current span to fit if c.escapeStartOfLine won't
676+
// introduce an additional backslash.
677+
//
678+
// The current implementation of this check is rather inefficient,
679+
// but since the check is done at most once per line, the
680+
// performance might as well be good enough.
681+
fits := false
682+
if currentLineWidth+1+w < maxWidth {
683+
fits = true
684+
} else if currentLineWidth+1+w == maxWidth {
685+
line := currentLine.String() + " " + span
686+
fits = c.escapeStartOfLine(line, startOfParagraph, true) == line
687+
}
688+
if fits {
689+
currentLine.WriteByte(' ')
690+
currentLine.WriteString(span)
691+
currentLineWidth += 1 + w
692+
} else {
693+
writeCurrentLine()
694+
startNewLine()
695+
currentLine.WriteString(span)
696+
currentLineWidth = w
697+
}
674698
}
675699
if hardLineBreak {
676700
writeCurrentLine()

pkg/md/fmt_test.go

Lines changed: 107 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package md_test
22

33
import (
4+
"fmt"
45
"html"
56
"regexp"
67
"strings"
@@ -10,6 +11,7 @@ import (
1011
"github.com/google/go-cmp/cmp"
1112
. "src.elv.sh/pkg/md"
1213
"src.elv.sh/pkg/testutil"
14+
"src.elv.sh/pkg/wcwidth"
1315
)
1416

1517
var supplementalFmtCases = []testCase{
@@ -91,43 +93,38 @@ func TestFmtPreservesHTMLRender(t *testing.T) {
9193
}
9294
}
9395

94-
func TestReflowFmtPreservesHTMLRenderModuleWhitespaces(t *testing.T) {
95-
testutil.Set(t, &UnescapeHTML, html.UnescapeString)
96-
for _, tc := range fmtTestCases {
97-
t.Run(tc.testName(), func(t *testing.T) {
98-
testReflowFmtPreservesHTMLRenderModuloWhitespaces(t, tc.Markdown, 80)
99-
})
100-
}
101-
}
102-
10396
func FuzzFmtPreservesHTMLRender(f *testing.F) {
10497
for _, tc := range fmtTestCases {
10598
f.Add(tc.Markdown)
10699
}
107100
f.Fuzz(testFmtPreservesHTMLRender)
108101
}
109102

110-
func FuzzReflowFmtPreservesHTMLRenderModuleWhitespaces(f *testing.F) {
111-
for _, tc := range fmtTestCases {
112-
f.Add(tc.Markdown, 20)
113-
f.Add(tc.Markdown, 80)
114-
}
115-
f.Fuzz(testReflowFmtPreservesHTMLRenderModuloWhitespaces)
116-
}
117-
118103
func testFmtPreservesHTMLRender(t *testing.T, original string) {
119-
t.Helper()
120104
testFmtPreservesHTMLRenderModulo(t, original, 0, nil)
121105
}
122106

107+
func TestReflowFmtPreservesHTMLRenderModuleWhitespaces(t *testing.T) {
108+
testReflowFmt(t, testReflowFmtPreservesHTMLRenderModuloWhitespaces)
109+
}
110+
111+
func FuzzReflowFmtPreservesHTMLRenderModuleWhitespaces(f *testing.F) {
112+
fuzzReflowFmt(f, testReflowFmtPreservesHTMLRenderModuloWhitespaces)
113+
}
114+
123115
var (
124116
paragraph = regexp.MustCompile(`(?s)<p>.*?</p>`)
125117
whitespaceRun = regexp.MustCompile(`[ \t\n]+`)
126118
brWithWhitespaces = regexp.MustCompile(`[ \t\n]*<br />[ \t\n]*`)
127119
)
128120

129121
func testReflowFmtPreservesHTMLRenderModuloWhitespaces(t *testing.T, original string, w int) {
130-
t.Helper()
122+
if strings.Contains(original, "<p>") {
123+
t.Skip("markdown contains <p>")
124+
}
125+
if strings.Contains(original, "</p>") {
126+
t.Skip("markdown contains </p>")
127+
}
131128
testFmtPreservesHTMLRenderModulo(t, original, w, func(html string) string {
132129
// Coalesce whitespaces in each paragraph.
133130
return paragraph.ReplaceAllStringFunc(html, func(p string) string {
@@ -141,8 +138,98 @@ func testReflowFmtPreservesHTMLRenderModuloWhitespaces(t *testing.T, original st
141138
})
142139
}
143140

141+
func TestReflowFmtResultIsUnchangedUnderFmt(t *testing.T) {
142+
testReflowFmt(t, testReflowFmtResultIsUnchangedUnderFmt)
143+
}
144+
145+
func FuzzReflowFmtResultIsUnchangedUnderFmt(f *testing.F) {
146+
fuzzReflowFmt(f, testReflowFmtResultIsUnchangedUnderFmt)
147+
}
148+
149+
func testReflowFmtResultIsUnchangedUnderFmt(t *testing.T, original string, w int) {
150+
reflowed := formatAndSkipIfUnsupported(t, original, w)
151+
formatted := RenderString(reflowed, &FmtCodec{})
152+
if reflowed != formatted {
153+
t.Errorf("original:\n%s\nreflowed:\n%s\nformatted:\n%s"+
154+
"markdown diff (-reflowed +formatted):\n%s",
155+
hr+"\n"+original+hr, hr+"\n"+reflowed+hr, hr+"\n"+formatted+hr,
156+
cmp.Diff(reflowed, formatted))
157+
}
158+
}
159+
160+
func TestReflowFmtResultFitsInWidth(t *testing.T) {
161+
testReflowFmt(t, testReflowFmtResultFitsInWidth)
162+
}
163+
164+
func FuzzReflowFmtResultFitsInWidth(f *testing.F) {
165+
fuzzReflowFmt(f, testReflowFmtResultFitsInWidth)
166+
}
167+
168+
var (
169+
// Match all markers that can be written by FmtCodec.
170+
markersRegexp = regexp.MustCompile(`^ *(?:(?:[-*>]|[0-9]{1,9}[.)]) *)*`)
171+
linkRegexp = regexp.MustCompile(`\[.*\]\(.*\)`)
172+
codeSpanRegexp = regexp.MustCompile("`.*`")
173+
)
174+
175+
func testReflowFmtResultFitsInWidth(t *testing.T, original string, w int) {
176+
if w <= 0 {
177+
t.Skip("width <= 0")
178+
}
179+
180+
var trace TraceCodec
181+
Render(original, &trace)
182+
for _, op := range trace.Ops() {
183+
switch op.Type {
184+
case OpHeading, OpCodeBlock, OpHTMLBlock:
185+
t.Skipf("input contains unsupported block type %s", op.Type)
186+
}
187+
}
188+
189+
reflowed := formatAndSkipIfUnsupported(t, original, w)
190+
191+
for _, line := range strings.Split(reflowed, "\n") {
192+
lineWidth := wcwidth.Of(line)
193+
if lineWidth <= w {
194+
continue
195+
}
196+
// Strip all markers
197+
content := line[len(markersRegexp.FindString(line)):]
198+
// Analyze whether the content is allowed to exceed width
199+
switch {
200+
case !strings.Contains(content, " "):
201+
case strings.Contains(content, "<"):
202+
case linkRegexp.MatchString(content):
203+
case codeSpanRegexp.MatchString(content):
204+
default:
205+
t.Errorf("line length > %d: %q\nfull reflowed:\n%s",
206+
w, line, hr+"\n"+reflowed+hr)
207+
}
208+
}
209+
}
210+
211+
var widths = []int{20, 51, 80}
212+
213+
func testReflowFmt(t *testing.T, test func(*testing.T, string, int)) {
214+
for _, tc := range fmtTestCases {
215+
for _, w := range widths {
216+
t.Run(fmt.Sprintf("%s/Width %d", tc.testName(), w), func(t *testing.T) {
217+
test(t, tc.Markdown, w)
218+
})
219+
}
220+
}
221+
}
222+
223+
func fuzzReflowFmt(f *testing.F, test func(*testing.T, string, int)) {
224+
for _, tc := range fmtTestCases {
225+
for _, w := range widths {
226+
f.Add(tc.Markdown, w)
227+
}
228+
}
229+
f.Fuzz(test)
230+
}
231+
144232
func testFmtPreservesHTMLRenderModulo(t *testing.T, original string, w int, processHTML func(string) string) {
145-
t.Helper()
146233
formatted := formatAndSkipIfUnsupported(t, original, w)
147234
originalRender := RenderString(original, &HTMLCodec{})
148235
formattedRender := RenderString(formatted, &HTMLCodec{})
@@ -163,7 +250,6 @@ func testFmtPreservesHTMLRenderModulo(t *testing.T, original string, w int, proc
163250
}
164251

165252
func formatAndSkipIfUnsupported(t *testing.T, original string, w int) string {
166-
t.Helper()
167253
if !utf8.ValidString(original) {
168254
t.Skipf("input is not valid UTF-8")
169255
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
string("0</p>\n0")
3+
int(80)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
string("\x02 00")
3+
int(-118)
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
go test fuzz v1
2+
string("\\# 000")
3+
int(5)

pkg/md/trace.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,13 @@ import (
66
)
77

88
// TraceCodec is a Codec that records all the Op's passed to its Do method.
9-
type TraceCodec struct{ strings.Builder }
9+
type TraceCodec struct {
10+
ops []Op
11+
strings.Builder
12+
}
1013

1114
func (c *TraceCodec) Do(op Op) {
15+
c.ops = append(c.ops, op)
1216
c.WriteString(op.Type.String())
1317
if op.Number != 0 {
1418
fmt.Fprintf(c, " Number=%d", op.Number)
@@ -37,3 +41,5 @@ func (c *TraceCodec) Do(op Op) {
3741
c.WriteString("\n")
3842
}
3943
}
44+
45+
func (c *TraceCodec) Ops() []Op { return c.ops }

0 commit comments

Comments
 (0)