Skip to content

Fixes some issues with line wrapping around inline code #215

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 21 additions & 9 deletions Sources/Markdown/Walker/Walkers/MarkupFormatter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -366,12 +366,13 @@ public struct MarkupFormatter: MarkupWalker {

// MARK: Formatter Utilities

/// True if the current line length is over the preferred line limit.
var isOverPreferredLineLimit: Bool {
/// Returns `true` if the current line length, plus an optional addition,
/// is over the preferred line limit.
func isOverPreferredLineLimit(with addition: String = "") -> Bool {
guard let lineLimit = formattingOptions.preferredLineLimit else {
return false
}
return state.lastLineLength >= lineLimit.maxLength
return state.lastLineLength + addition.count >= lineLimit.maxLength
}

/**
Expand Down Expand Up @@ -590,8 +591,12 @@ public struct MarkupFormatter: MarkupWalker {
// However, there is one exception:
// we might already be right at the edge of a line when
// this method was called.
if state.lastLineLength + word.count >= lineLimit.maxLength {
queueNewline()
if isOverPreferredLineLimit(with: word) && state.queuedNewlines == 0 {
// An exception to the exception: don't push punctuation to the
// next line, even if this line is at or longer than the limit.
if !(word.allSatisfy(\.isPunctuation) && word.count <= 3) {
queueNewline()
}
}
print(word, for: element)
wordsThisLine += 1
Expand Down Expand Up @@ -776,13 +781,14 @@ public struct MarkupFormatter: MarkupWalker {

public mutating func visitInlineCode(_ inlineCode: InlineCode) {
let savedState = state
let atLineStart = state.lastLineLength == 0
softWrapPrint("`\(inlineCode.code)`", for: inlineCode)

// Splitting inline code elements is allowed if it contains spaces.
// If printing with automatic wrapping still put us over the line,
// prefer to print it on the next line to give as much opportunity
// to keep the contents on one line.
if inlineCode.indexInParent > 0 && (isOverPreferredLineLimit || state.effectiveLineNumber > savedState.effectiveLineNumber) {
if !atLineStart && inlineCode.indexInParent > 0 && (isOverPreferredLineLimit() || state.effectiveLineNumber > savedState.effectiveLineNumber) {
restoreState(to: savedState)
queueNewline()
softWrapPrint("`\(inlineCode.code)`", for: inlineCode)
Expand Down Expand Up @@ -813,7 +819,7 @@ public struct MarkupFormatter: MarkupWalker {
// Image elements' source URLs can't be split. If wrapping the alt text
// of an image still put us over the line, prefer to print it on the
// next line to give as much opportunity to keep the alt text contents on one line.
if image.indexInParent > 0 && (isOverPreferredLineLimit || state.effectiveLineNumber > savedState.effectiveLineNumber) {
if image.indexInParent > 0 && (isOverPreferredLineLimit() || state.effectiveLineNumber > savedState.effectiveLineNumber) {
restoreState(to: savedState)
queueNewline()
printImage()
Expand Down Expand Up @@ -850,7 +856,7 @@ public struct MarkupFormatter: MarkupWalker {
// Link elements' destination URLs can't be split. If wrapping the link text
// of a link still put us over the line, prefer to print it on the
// next line to give as much opportunity to keep the link text contents on one line.
if link.indexInParent > 0 && (isOverPreferredLineLimit || state.effectiveLineNumber > savedState.effectiveLineNumber) {
if link.indexInParent > 0 && (isOverPreferredLineLimit() || state.effectiveLineNumber > savedState.effectiveLineNumber) {
restoreState(to: savedState)
queueNewline()
printRegularLink()
Expand Down Expand Up @@ -1140,6 +1146,12 @@ public struct MarkupFormatter: MarkupWalker {
}

public mutating func visitSymbolLink(_ symbolLink: SymbolLink) {
let atLineStart = state.lastLineLength == 0
let composited = "``\(symbolLink.destination ?? "")``"

if !atLineStart && isOverPreferredLineLimit(with: composited) {
queueNewline()
}
print("``", for: symbolLink)
print(symbolLink.destination ?? "", for: symbolLink)
print("``", for: symbolLink)
Expand All @@ -1162,7 +1174,7 @@ public struct MarkupFormatter: MarkupWalker {
// gets into the realm of JSON formatting which might be out of scope of
// this formatter. Therefore if exceeded, prefer to print it on the next
// line to give as much opportunity to keep the attributes on one line.
if attributes.indexInParent > 0 && (isOverPreferredLineLimit || state.effectiveLineNumber > savedState.effectiveLineNumber) {
if attributes.indexInParent > 0 && (isOverPreferredLineLimit() || state.effectiveLineNumber > savedState.effectiveLineNumber) {
restoreState(to: savedState)
queueNewline()
printInlineAttributes()
Expand Down
102 changes: 101 additions & 1 deletion Tests/MarkdownTests/Visitors/MarkupFormatterTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -967,6 +967,61 @@ class MarkupFormatterLineSplittingTests: XCTestCase {
}
}

/**
Test that breaks are inserted before symbolic links when necessary to
honor the preferred line limit.
*/
func testParagraphWithLongSymbolicLinks() {
let source = """
Because options are parsed before arguments, an option that consumes or
suppresses the `--` terminator can prevent a ``postTerminator`` argument
array from capturing any input. In particular, the
``SingleValueParsingStrategy/unconditional``,
``ArrayParsingStrategy/unconditionalSingleValue``, and
``ArrayParsingStrategy/remaining`` parsing strategies can all consume
the terminator as part of their values.
"""
let expected = """
Because options are parsed before arguments, an option that consumes or
suppresses the `--` terminator can prevent a ``postTerminator`` argument
array from capturing any input. In particular, the
``SingleValueParsingStrategy/unconditional``,
``ArrayParsingStrategy/unconditionalSingleValue``, and
``ArrayParsingStrategy/remaining`` parsing strategies can all consume the
terminator as part of their values.
"""
Comment on lines +975 to +992
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: This example data confused me for a bit because the input was already wrapped almost the same as the expected output (only one word wrapped differently). Because of this, I found it difficult to reason about how this test would have behaved differently without the changes in this PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! That wrapping change wasn't intended – I'll fix that and change the example data to describe the behavior it's validating. (I wasn't quite sure how to go here since these are fixes for bugs that aren't exposed in the current tests.)

let options = MarkupFormatter.Options(preferredLineLimit: PreferredLineLimit(maxLength: 74, breakWith: .softBreak))
let document = Document(parsing: source, options: [.parseSymbolLinks])
let printed = document.format(options: options)
XCTAssertEqual(expected, printed)

let expectedTreeDump = """
Document
└─ Paragraph
├─ Text "Because options are parsed before arguments, an option that consumes or"
├─ SoftBreak
├─ Text "suppresses the "
├─ InlineCode `--`
├─ Text " terminator can prevent a "
├─ InlineCode `postTerminator`
├─ Text " argument"
├─ SoftBreak
├─ Text "array from capturing any input. In particular, the"
├─ SoftBreak
├─ InlineCode `SingleValueParsingStrategy/unconditional`
├─ Text ","
├─ SoftBreak
├─ InlineCode `ArrayParsingStrategy/unconditionalSingleValue`
├─ Text ", and"
├─ SoftBreak
├─ InlineCode `ArrayParsingStrategy/remaining`
├─ Text " parsing strategies can all consume the"
├─ SoftBreak
└─ Text "terminator as part of their values."
"""
XCTAssertEqual(expectedTreeDump, Document(parsing: printed).debugDescription())
}

/**
Test that line breaks maintain block structure in a flat, unordered list.
*/
Expand Down Expand Up @@ -1217,7 +1272,6 @@ class MarkupFormatterLineSplittingTests: XCTestCase {
let expected = """
> Really really
> really long line
> >\u{0020}
> > Whoa, really
> > really really
> > really long
Expand Down Expand Up @@ -1372,6 +1426,52 @@ class MarkupFormatterLineSplittingTests: XCTestCase {
XCTAssertEqual(expected, printed)
XCTAssertTrue(document.hasSameStructure(as: Document(parsing: printed)))
}

/**
Test that wrapping at the start of a long inline code run doesn't cause
an extra newline.
*/
func testBreakAtLongInlineCode() {
let source = "This is a long line `that contains inline code`."
let document = Document(parsing: source)
let printed = document.format(options: .init(preferredLineLimit: .init(maxLength: 20, breakWith: .softBreak)))
let expected = """
This is a long line
`that contains
inline code`.
"""
let expectedTreeDump = """
Document
└─ Paragraph
├─ Text "This is a long line"
├─ SoftBreak
├─ InlineCode `that contains inline code`
└─ Text "."
"""
XCTAssertEqual(expected, printed)
XCTAssertEqual(expectedTreeDump, Document(parsing: printed).debugDescription())
}

/**
Test that wrapping at the start of a short inline code run doesn't cause
an extra newline.
*/
func testBreakAtShortInlineCode() {
let source = """
Perform an atomic logical AND operation on the value referenced by
`pointer` and return the original value, applying the specified memory
ordering.
"""
let expected = """
Perform an atomic logical AND operation on the value referenced by
`pointer` and return the original value, applying the specified memory
ordering.
"""
Comment on lines +1460 to +1469
Copy link
Contributor

Choose a reason for hiding this comment

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

minor: The input and expected output are identical. Because of this, I find it harder to reason about how this test would have behaved differently without the changes in this PR.

let document = Document(parsing: source)
let printed = document.format(options: .init(preferredLineLimit: .init(maxLength: 76, breakWith: .softBreak)))
XCTAssertEqual(expected, printed)
XCTAssertTrue(document.hasSameStructure(as: Document(parsing: printed)))
}
}

class MarkupFormatterTableTests: XCTestCase {
Expand Down