Skip to content

Commit 941f698

Browse files
objecthubobjecthub
committed
Converting HTML into NSAttributedString objects has serious bugs. This change improves on converting nested lists. It also introduces a new option for AttributedStringGenerator: tightLists. This produces more compact lists.
1 parent d9bc97f commit 941f698

File tree

6 files changed

+109
-31
lines changed

6 files changed

+109
-31
lines changed

‎Sources/MarkdownKit/AttributedString/AttributedStringGenerator.swift‎

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -33,30 +33,53 @@
3333
/// override how individual Markdown structures are converted into attributed strings.
3434
///
3535
open class AttributedStringGenerator {
36-
36+
37+
/// Options for the attributed string generator
38+
public struct Options: OptionSet {
39+
public let rawValue: UInt
40+
41+
public init(rawValue: UInt) {
42+
self.rawValue = rawValue
43+
}
44+
45+
public static let tightLists = Options(rawValue: 1 << 0)
46+
}
47+
3748
/// Customized html generator to work around limitations of the current HTML to
3849
/// `NSAttributedString` conversion logic provided by the operating system.
3950
open class InternalHtmlGenerator: HtmlGenerator {
40-
var outer: AttributedStringGenerator?
51+
var outer: AttributedStringGenerator
4152

4253
public init(outer: AttributedStringGenerator) {
4354
self.outer = outer
4455
}
4556

46-
open override func generate(block: Block, tight: Bool = false) -> String {
57+
open override func generate(block: Block, parent: Parent, tight: Bool = false) -> String {
4758
switch block {
4859
case .list(_, _, _):
49-
return super.generate(block: block, tight: tight) + "<p style=\"margin: 0;\" />\n"
60+
let res = super.generate(block: block, parent: .block(block, parent), tight: tight)
61+
if case .block(.listItem(_, _, _), _) = parent {
62+
return res
63+
} else {
64+
return res + "<p style=\"margin: 0;\" />\n"
65+
}
66+
case .paragraph(let text):
67+
if case .block(.listItem(_, _, _), .block(.list(_, let tight, _), _)) = parent,
68+
tight || self.outer.options.contains(.tightLists) {
69+
return self.generate(text: text) + "\n"
70+
} else {
71+
return "<p>" + self.generate(text: text) + "</p>\n"
72+
}
5073
case .indentedCode(_),
5174
.fencedCode(_, _):
5275
return "<table style=\"width: 100%; margin-bottom: 3px;\"><tbody><tr>" +
5376
"<td class=\"codebox\">" +
54-
super.generate(block: block, tight: tight) +
77+
super.generate(block: block, parent: .block(block, parent), tight: tight) +
5578
"</td></tr></tbody></table><p style=\"margin: 0;\" />\n"
5679
case .blockquote(let blocks):
5780
return "<table class=\"blockquote\"><tbody><tr>" +
5881
"<td class=\"quote\" /><td style=\"width: 0.5em;\" /><td>\n" +
59-
self.generate(blocks: blocks) +
82+
self.generate(blocks: blocks, parent: .block(block, parent)) +
6083
"</td></tr><tr style=\"height: 0;\"><td /><td /><td /></tr></tbody></table>\n"
6184
case .thematicBreak:
6285
return "<p><table style=\"width: 100%; margin-bottom: 3px;\"><tbody>" +
@@ -76,7 +99,7 @@ open class AttributedStringGenerator {
7699
}
77100
}
78101
var html = "<table class=\"mtable\" " +
79-
"cellpadding=\"\(self.outer?.tableCellPadding ?? 2)\"><thead><tr>\n"
102+
"cellpadding=\"\(self.outer.tableCellPadding)\"><thead><tr>\n"
80103
var i = 0
81104
for head in header {
82105
html += "<th\(tagsuffix[i])\(self.generate(text: head))&nbsp;</th>"
@@ -100,7 +123,9 @@ open class AttributedStringGenerator {
100123
html += "<dt>" + self.generate(text: def.item) + "</dt>\n"
101124
for descr in def.descriptions {
102125
if case .listItem(_, _, let blocks) = descr {
103-
html += "<dd>" + self.generate(blocks: blocks) + "</dd>\n"
126+
html += "<dd>" +
127+
self.generate(blocks: blocks, parent: .block(block, parent)) +
128+
"</dd>\n"
104129
}
105130
}
106131
}
@@ -109,7 +134,7 @@ open class AttributedStringGenerator {
109134
case .custom(let customBlock):
110135
return customBlock.generateHtml(via: self, and: self.outer, tight: tight)
111136
default:
112-
return super.generate(block: block, tight: tight)
137+
return super.generate(block: block, parent: parent, tight: tight)
113138
}
114139
}
115140

@@ -120,7 +145,7 @@ open class AttributedStringGenerator {
120145
if let uriStr = uri {
121146
let url = URL(string: uriStr)
122147
if (url?.scheme == nil) || (url?.isFileURL ?? false),
123-
let baseUrl = self.outer?.imageBaseUrl {
148+
let baseUrl = self.outer.imageBaseUrl {
124149
let url = URL(fileURLWithPath: uriStr, relativeTo: baseUrl)
125150
if url.isFileURL {
126151
return "<img src=\"\(url.absoluteString)\"" +
@@ -142,6 +167,9 @@ open class AttributedStringGenerator {
142167
/// Default `AttributedStringGenerator` implementation.
143168
public static let standard: AttributedStringGenerator = AttributedStringGenerator()
144169

170+
/// The generator options.
171+
public let options: Options
172+
145173
/// The base font size.
146174
public let fontSize: Float
147175

@@ -201,7 +229,8 @@ open class AttributedStringGenerator {
201229

202230

203231
/// Constructor providing customization options for the generated `NSAttributedString` markup.
204-
public init(fontSize: Float = 14.0,
232+
public init(options: Options = [],
233+
fontSize: Float = 14.0,
205234
fontFamily: String = "\"Times New Roman\",Times,serif",
206235
fontColor: String = mdDefaultColor,
207236
codeFontSize: Float = 13.0,
@@ -221,6 +250,7 @@ open class AttributedStringGenerator {
221250
maxImageHeight: String? = nil,
222251
customStyle: String = "",
223252
imageBaseUrl: URL? = nil) {
253+
self.options = options
224254
self.fontSize = fontSize
225255
self.fontFamily = fontFamily
226256
self.fontColor = fontColor
@@ -249,21 +279,23 @@ open class AttributedStringGenerator {
249279

250280
/// Generates an attributed string from the given Markdown blocks
251281
open func generate(block: Block) -> NSAttributedString? {
252-
return self.generateAttributedString(self.htmlGenerator.generate(block: block))
282+
return self.generateAttributedString(self.htmlGenerator.generate(block: block, parent: .none))
253283
}
254284

255285
/// Generates an attributed string from the given Markdown blocks
256286
open func generate(blocks: Blocks) -> NSAttributedString? {
257-
return self.generateAttributedString(self.htmlGenerator.generate(blocks: blocks))
287+
return self.generateAttributedString(self.htmlGenerator.generate(blocks: blocks, parent: .none))
258288
}
259289

260290
private func generateAttributedString(_ htmlBody: String) -> NSAttributedString? {
261-
let htmlDoc = self.generateHtml(htmlBody)
262-
let httpData = Data(htmlDoc.utf8)
263-
return try? NSAttributedString(data: httpData,
264-
options: [.documentType: NSAttributedString.DocumentType.html,
265-
.characterEncoding: String.Encoding.utf8.rawValue],
266-
documentAttributes: nil)
291+
if let httpData = self.generateHtml(htmlBody).data(using: .utf8) {
292+
return try? NSAttributedString(data: httpData,
293+
options: [.documentType: NSAttributedString.DocumentType.html,
294+
.characterEncoding: String.Encoding.utf8.rawValue],
295+
documentAttributes: nil)
296+
} else {
297+
return nil
298+
}
267299
}
268300

269301
open var htmlGenerator: HtmlGenerator {

‎Sources/MarkdownKit/HTML/HtmlGenerator.swift‎

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,12 @@ import Foundation
2626
/// individual Markdown structures are converted into HTML.
2727
///
2828
open class HtmlGenerator {
29-
29+
30+
public enum Parent {
31+
case none
32+
indirect case block(Block, Parent)
33+
}
34+
3035
/// Default `HtmlGenerator` implementation
3136
public static let standard = HtmlGenerator()
3237

@@ -38,36 +43,42 @@ open class HtmlGenerator {
3843
guard case .document(let blocks) = doc else {
3944
preconditionFailure("cannot generate HTML from \(doc)")
4045
}
41-
return self.generate(blocks: blocks)
46+
return self.generate(blocks: blocks, parent: .none)
4247
}
4348

44-
open func generate(blocks: Blocks, tight: Bool = false) -> String {
49+
open func generate(blocks: Blocks, parent: Parent, tight: Bool = false) -> String {
4550
var res = ""
4651
for block in blocks {
47-
res += self.generate(block: block, tight: tight)
52+
res += self.generate(block: block, parent: parent, tight: tight)
4853
}
4954
return res
5055
}
5156

52-
open func generate(block: Block, tight: Bool = false) -> String {
57+
open func generate(block: Block, parent: Parent, tight: Bool = false) -> String {
5358
switch block {
5459
case .document(_):
5560
preconditionFailure("broken block \(block)")
5661
case .blockquote(let blocks):
57-
return "<blockquote>\n" + self.generate(blocks: blocks) + "</blockquote>\n"
62+
return "<blockquote>\n" +
63+
self.generate(blocks: blocks, parent: .block(block, parent)) +
64+
"</blockquote>\n"
5865
case .list(let start, let tight, let blocks):
5966
if let startNumber = start {
6067
return "<ol start=\"\(startNumber)\">\n" +
61-
self.generate(blocks: blocks, tight: tight) +
68+
self.generate(blocks: blocks, parent: .block(block, parent), tight: tight) +
6269
"</ol>\n"
6370
} else {
64-
return "<ul>\n" + self.generate(blocks: blocks, tight: tight) + "</ul>\n"
71+
return "<ul>\n" +
72+
self.generate(blocks: blocks, parent: .block(block, parent), tight: tight) +
73+
"</ul>\n"
6574
}
6675
case .listItem(_, _, let blocks):
6776
if tight, let text = blocks.text {
6877
return "<li>" + self.generate(text: text) + "</li>\n"
6978
} else {
70-
return "<li>" + self.generate(blocks: blocks) + "</li>\n"
79+
return "<li>" +
80+
self.generate(blocks: blocks, parent: .block(block, parent)) +
81+
"</li>\n"
7182
}
7283
case .paragraph(let text):
7384
return "<p>" + self.generate(text: text) + "</p>\n"
@@ -136,7 +147,9 @@ open class HtmlGenerator {
136147
case .paragraph(let text) = blocks.first! {
137148
html += "<dd>" + self.generate(text: text) + "</dd>\n"
138149
} else {
139-
html += "<dd>" + self.generate(blocks: blocks) + "</dd>\n"
150+
html += "<dd>" +
151+
self.generate(blocks: blocks, parent: .block(block, parent)) +
152+
"</dd>\n"
140153
}
141154
}
142155
}

‎Tests/MarkdownKitTests/ExtendedMarkdownHtmlTests.swift‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,13 @@ class ExtendedMarkdownHtmlTests: XCTestCase, MarkdownKitFactory {
2727
return HtmlGenerator().generate(doc: ExtendedMarkdownParser.standard.parse(str))
2828
.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines)
2929
}
30-
30+
31+
func testSimpleNestedLists() {
32+
XCTAssertEqual(
33+
generateHtml("- Apple\n\t- Banana"),
34+
"<ul>\n<li><p>Apple</p>\n<ul>\n<li>Banana</li>\n</ul>\n</li>\n</ul>")
35+
}
36+
3137
func testTables() {
3238
XCTAssertEqual(generateHtml(" Column A | Column B\n" +
3339
" -------- | --------\n"),
@@ -83,6 +89,7 @@ class ExtendedMarkdownHtmlTests: XCTestCase, MarkdownKitFactory {
8389
}
8490

8591
static let allTests = [
92+
("testSimpleNestedLists", testSimpleNestedLists),
8693
("testTables", testTables),
8794
("testDescriptionLists", testDescriptionLists),
8895
]

‎Tests/MarkdownKitTests/MarkdownASTests.swift‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,18 @@ class MarkdownASTests: XCTestCase {
3232
.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines)
3333
}
3434

35+
func testSimpleNestedLists() {
36+
XCTAssertEqual(
37+
generateHtml("- Apple\n\t- Banana"),
38+
"<ul>\n<li><p>Apple</p>\n<ul>\n<li>Banana</li>\n</ul>\n</li>\n</ul>\n<p style=\"margin: 0;\" />")
39+
XCTAssertEqual(
40+
AttributedStringGenerator(options: [.tightLists])
41+
.htmlGenerator
42+
.generate(doc: MarkdownParser.standard.parse("- Apple\n\t- Banana"))
43+
.trimmingCharacters(in: CharacterSet.whitespacesAndNewlines),
44+
"<ul>\n<li>Apple\n<ul>\n<li>Banana</li>\n</ul>\n</li>\n</ul>\n<p style=\"margin: 0;\" />")
45+
}
46+
3547
func testRelativeImageUrls() {
3648
XCTAssertEqual(generateHtml("![Test image](imagefile.png)"),
3749
"<p><img src=\"imagefile.png\" alt=\"Test image\"/></p>")

‎Tests/MarkdownKitTests/MarkdownBlockTests.swift‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,14 @@ class MarkdownBlockTests: XCTestCase, MarkdownKitFactory {
221221
list(listItem("-", tight: true, paragraph("bar")))))
222222
}
223223

224+
func testSimpleNestedList() {
225+
XCTAssertEqual(parseBlocks("- Apple\n\t- Banana"),
226+
document(list(tight: false,
227+
listItem("-", paragraph("Apple"),
228+
list(tight: true,
229+
listItem("-", tight: true, paragraph("Banana")))))))
230+
}
231+
224232
func testNestedList() {
225233
XCTAssertEqual(parseBlocks("- foo\n- bar\n - one\n - two\n - three\n- goo"),
226234
document(list(tight: false,
@@ -232,7 +240,7 @@ class MarkdownBlockTests: XCTestCase, MarkdownKitFactory {
232240
listItem("-", tight: true, paragraph("three")))),
233241
listItem("-", tight: true, paragraph("goo")))))
234242
}
235-
243+
236244
func testBlockquoteList() {
237245
XCTAssertEqual(parseBlocks(">>- one\n>>\n > > two"),
238246
document(blockquote(blockquote(list(listItem("-", paragraph("one"))),

‎Tests/MarkdownKitTests/MarkdownHtmlTests.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,12 @@ class MarkdownHtmlTests: XCTestCase, MarkdownKitFactory {
6161
"<p>Four</p>\n</li>\n</ul>")
6262
}
6363

64+
func testSimpleNestedLists() {
65+
XCTAssertEqual(
66+
generateHtml("- Apple\n\t- Banana"),
67+
"<ul>\n<li><p>Apple</p>\n<ul>\n<li>Banana</li>\n</ul>\n</li>\n</ul>")
68+
}
69+
6470
func testNestedLists() {
6571
XCTAssertEqual(generateHtml("""
6672
- foo

0 commit comments

Comments
 (0)