From 2bc22994508aa19cca255297b680337762dbd094 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Fri, 23 Sep 2022 14:46:02 -0600 Subject: [PATCH 1/6] emit table column alignments into Render JSON rdar://77749746 --- .../Content/RenderBlockContent.swift | 25 ++++++++++- .../Rendering/RenderContentCompiler.swift | 24 ++++++++++- .../Resources/RenderNode.spec.json | 12 ++++++ .../Model/RenderContentMetadataTests.swift | 43 ++++++++++++++++++- 4 files changed, 100 insertions(+), 4 deletions(-) diff --git a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift index 4ef0999d1a..d6bd814f5e 100644 --- a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift +++ b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift @@ -243,6 +243,8 @@ public enum RenderBlockContent: Equatable { public struct Table: Equatable { /// The style of header in this table. public var header: HeaderType + /// The text alignment of each column in this table. + public var alignments: [ColumnAlignment]? /// The rows in this table. public var rows: [TableRow] /// Any extended information that describes cells in this table. @@ -251,11 +253,14 @@ public enum RenderBlockContent: Equatable { public var metadata: RenderContentMetadata? /// Creates a new table with the given data. - public init(header: HeaderType, rows: [TableRow], extendedData: Set, metadata: RenderContentMetadata? = nil) { + public init(header: HeaderType, alignments: [ColumnAlignment]?, rows: [TableRow], extendedData: Set, metadata: RenderContentMetadata? = nil) { self.header = header self.rows = rows self.extendedData = extendedData self.metadata = metadata + if let alignments = alignments, !alignments.allSatisfy({ $0 == .unset }) { + self.alignments = alignments + } } } @@ -374,6 +379,18 @@ public enum RenderBlockContent: Equatable { /// The table doesn't contain headers. case none } + + /// The methods by which a table column can have its text aligned. + public enum ColumnAlignment: String, Codable, Equatable { + /// Force text alignment to be left-justified. + case left + /// Force text alignment to be right-justified. + case right + /// Force text alignment to be centered. + case center + /// Leave text alignment to the default. + case unset + } /// A table row that contains a list of row cells. public struct TableRow: Codable, Equatable { @@ -552,7 +569,7 @@ public enum RenderBlockContent: Equatable { // not follow from the struct layout. extension RenderBlockContent.Table: Codable { enum CodingKeys: String, CodingKey { - case header, rows, extendedData, metadata + case header, alignments, rows, extendedData, metadata } // TableCellExtendedData encodes the row and column indices as a dynamic key with the format "{row}_{column}". @@ -589,6 +606,7 @@ extension RenderBlockContent.Table: Codable { let container = try decoder.container(keyedBy: CodingKeys.self) self.header = try container.decode(RenderBlockContent.HeaderType.self, forKey: .header) + self.alignments = try container.decodeIfPresent([RenderBlockContent.ColumnAlignment].self, forKey: .alignments) self.rows = try container.decode([RenderBlockContent.TableRow].self, forKey: .rows) self.metadata = try container.decodeIfPresent(RenderContentMetadata.self, forKey: .metadata) @@ -610,6 +628,9 @@ extension RenderBlockContent.Table: Codable { public func encode(to encoder: Encoder) throws { var container = encoder.container(keyedBy: CodingKeys.self) try container.encode(header, forKey: .header) + if let alignments = alignments, !alignments.isEmpty, !alignments.allSatisfy({ $0 == .unset }) { + try container.encode(alignments, forKey: .alignments) + } try container.encode(rows, forKey: .rows) try container.encodeIfPresent(metadata, forKey: .metadata) diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index 0808fdccb2..80f6c571e4 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -262,8 +262,30 @@ struct RenderContentCompiler: MarkupVisitor { } rows.append(RenderBlockContent.TableRow(cells: cells)) } + + var tempAlignments = [RenderBlockContent.ColumnAlignment]() + for alignment in table.columnAlignments { + switch alignment { + case .left: tempAlignments.append(.left) + case .right: tempAlignments.append(.right) + case .center: tempAlignments.append(.center) + case nil: tempAlignments.append(.unset) + } + } + while tempAlignments.count < table.maxColumnCount { + tempAlignments.append(.unset) + } + if tempAlignments.allSatisfy({ $0 == .unset }) { + tempAlignments = [] + } + let alignments: [RenderBlockContent.ColumnAlignment]? + if tempAlignments.isEmpty { + alignments = nil + } else { + alignments = tempAlignments + } - return [RenderBlockContent.table(.init(header: .row, rows: [RenderBlockContent.TableRow(cells: headerCells)] + rows, extendedData: extendedData, metadata: nil))] + return [RenderBlockContent.table(.init(header: .row, alignments: alignments, rows: [RenderBlockContent.TableRow(cells: headerCells)] + rows, extendedData: extendedData, metadata: nil))] } mutating func visitStrikethrough(_ strikethrough: Strikethrough) -> [RenderContent] { diff --git a/Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json b/Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json index a354f6c0ec..a1fc6bcad8 100644 --- a/Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json +++ b/Sources/SwiftDocC/SwiftDocC.docc/Resources/RenderNode.spec.json @@ -949,6 +949,18 @@ "none" ] }, + "alignments": { + "type": "array", + "items": { + "type": "string", + "enum": [ + "left", + "right", + "center", + "unset" + ] + } + }, "rows": { "type": "array", "items": { diff --git a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift index e1c85e9b25..426fce1e4a 100644 --- a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift +++ b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift @@ -37,7 +37,7 @@ class RenderContentMetadataTests: XCTestCase { RenderInlineContent.text("Content"), ]) - let table = RenderBlockContent.table(.init(header: .both, rows: [], extendedData: [], metadata: metadata)) + let table = RenderBlockContent.table(.init(header: .both, alignments: [], rows: [], extendedData: [], metadata: metadata)) let data = try JSONEncoder().encode(table) let roundtrip = try JSONDecoder().decode(RenderBlockContent.self, from: data) @@ -157,6 +157,47 @@ class RenderContentMetadataTests: XCTestCase { try assertRoundTripCoding(renderedTable) } + + func testRenderingTableColumnAlignments() throws { + let (bundle, context) = try testBundleAndContext(named: "TestBundle") + var renderContentCompiler = RenderContentCompiler(context: context, bundle: bundle, identifier: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/path", fragment: nil, sourceLanguage: .swift)) + + let source = """ + | one | two | three | four | + | :-- | --: | :---: | ---- | + | one | two | three | four | + """ + let document = Document(parsing: source) + + // Verifies that a markdown table renders correctly. + + let result = try XCTUnwrap(renderContentCompiler.visit(document.child(at: 0)!)) + let renderedTable = try XCTUnwrap(result.first as? RenderBlockContent) + + let renderCell: ([RenderBlockContent]) -> String = { cell in + return cell.reduce(into: "") { (result, element) in + switch element { + case .paragraph(let p): + guard let para = p.inlineContent.first else { return } + result.append(para.plainText) + default: XCTFail("Unexpected element"); return + } + } + } + + switch renderedTable { + case .table(let t): + XCTAssertEqual(t.header, .row) + XCTAssertEqual(t.rows.count, 2) + guard t.rows.count == 2 else { return } + XCTAssertEqual(t.rows[0].cells.map(renderCell), ["one", "two", "three", "four"]) + XCTAssertEqual(t.rows[1].cells.map(renderCell), ["one", "two", "three", "four"]) + XCTAssertEqual(t.alignments, [.left, .right, .center, .unset]) + default: XCTFail("Unexpected element") + } + + try assertRoundTripCoding(renderedTable) + } func testStrikethrough() throws { let (bundle, context) = try testBundleAndContext(named: "TestBundle") From bdbb5231e9332a16ea07a6e23b1caba28281b564 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Mon, 26 Sep 2022 13:46:00 -0600 Subject: [PATCH 2/6] review: make alignments parameter optional --- .../SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift | 2 +- Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift index d6bd814f5e..1d135d8024 100644 --- a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift +++ b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift @@ -253,7 +253,7 @@ public enum RenderBlockContent: Equatable { public var metadata: RenderContentMetadata? /// Creates a new table with the given data. - public init(header: HeaderType, alignments: [ColumnAlignment]?, rows: [TableRow], extendedData: Set, metadata: RenderContentMetadata? = nil) { + public init(header: HeaderType, alignments: [ColumnAlignment]? = nil, rows: [TableRow], extendedData: Set, metadata: RenderContentMetadata? = nil) { self.header = header self.rows = rows self.extendedData = extendedData diff --git a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift index 426fce1e4a..b7db638e57 100644 --- a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift +++ b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift @@ -37,7 +37,7 @@ class RenderContentMetadataTests: XCTestCase { RenderInlineContent.text("Content"), ]) - let table = RenderBlockContent.table(.init(header: .both, alignments: [], rows: [], extendedData: [], metadata: metadata)) + let table = RenderBlockContent.table(.init(header: .both, rows: [], extendedData: [], metadata: metadata)) let data = try JSONEncoder().encode(table) let roundtrip = try JSONDecoder().decode(RenderBlockContent.self, from: data) From dd609be2d7b04533c1deda7a183a66553f1fe9a6 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Mon, 26 Sep 2022 13:53:04 -0600 Subject: [PATCH 3/6] review: assert that all-unset column alignments result in nil --- Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift index b7db638e57..021b89a5d8 100644 --- a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift +++ b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift @@ -102,6 +102,7 @@ class RenderContentMetadataTests: XCTestCase { XCTAssertEqual(t.rows[0].cells.map(renderCell), ["Column 1", "Column 2"]) XCTAssertEqual(t.rows[1].cells.map(renderCell), ["Cell 1", "Cell 2"]) XCTAssertEqual(t.rows[2].cells.map(renderCell), ["Cell 3", "Cell 4"]) + XCTAssertNil(t.alignments) default: XCTFail("Unexpected element") } } @@ -152,6 +153,7 @@ class RenderContentMetadataTests: XCTestCase { for expectedData in expectedExtendedData { XCTAssert(t.extendedData.contains(expectedData)) } + XCTAssertNil(t.alignments) default: XCTFail("Unexpected element") } From c4d96fecbecab87d221b0a5b43cf5b6cd381f4b5 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Tue, 27 Sep 2022 09:34:04 -0600 Subject: [PATCH 4/6] document Table initializer and alignments property to describe nil --- .../Rendering/Content/RenderBlockContent.swift | 16 ++++++++++++++-- .../Model/Rendering/RenderContentCompiler.swift | 2 +- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift index 1d135d8024..377a5b8d2b 100644 --- a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift +++ b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift @@ -244,6 +244,9 @@ public enum RenderBlockContent: Equatable { /// The style of header in this table. public var header: HeaderType /// The text alignment of each column in this table. + /// + /// A `nil` value for this property is the same as all the columns being + /// ``RenderBlockContent/ColumnAlignment/unset``. public var alignments: [ColumnAlignment]? /// The rows in this table. public var rows: [TableRow] @@ -253,12 +256,21 @@ public enum RenderBlockContent: Equatable { public var metadata: RenderContentMetadata? /// Creates a new table with the given data. - public init(header: HeaderType, alignments: [ColumnAlignment]? = nil, rows: [TableRow], extendedData: Set, metadata: RenderContentMetadata? = nil) { + /// + /// - Parameters: + /// - header: The style of header in this table. + /// - rawAlignments: The text alignment for each column in this table. If all the + /// alignments are ``RenderBlockContent/ColumnAlignment/unset``, the ``alignments`` + /// property will be set to `nil`. + /// - rows: The cell data for this table. + /// - extendedData: Any extended information that describes cells in this table. + /// - metadata: Additional metadata for this table, if necessary. + public init(header: HeaderType, rawAlignments: [ColumnAlignment]? = nil, rows: [TableRow], extendedData: Set, metadata: RenderContentMetadata? = nil) { self.header = header self.rows = rows self.extendedData = extendedData self.metadata = metadata - if let alignments = alignments, !alignments.allSatisfy({ $0 == .unset }) { + if let alignments = rawAlignments, !alignments.allSatisfy({ $0 == .unset }) { self.alignments = alignments } } diff --git a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift index 80f6c571e4..4d16754306 100644 --- a/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift +++ b/Sources/SwiftDocC/Model/Rendering/RenderContentCompiler.swift @@ -285,7 +285,7 @@ struct RenderContentCompiler: MarkupVisitor { alignments = tempAlignments } - return [RenderBlockContent.table(.init(header: .row, alignments: alignments, rows: [RenderBlockContent.TableRow(cells: headerCells)] + rows, extendedData: extendedData, metadata: nil))] + return [RenderBlockContent.table(.init(header: .row, rawAlignments: alignments, rows: [RenderBlockContent.TableRow(cells: headerCells)] + rows, extendedData: extendedData, metadata: nil))] } mutating func visitStrikethrough(_ strikethrough: Strikethrough) -> [RenderContent] { From 84abaa1d6a79f9b8e0b98419973b9dbd3b46e1f8 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Tue, 27 Sep 2022 09:34:36 -0600 Subject: [PATCH 5/6] filter out all-unset alignments in Table's decoder --- .../Model/Rendering/Content/RenderBlockContent.swift | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift index 377a5b8d2b..8a70bbb61d 100644 --- a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift +++ b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift @@ -618,7 +618,14 @@ extension RenderBlockContent.Table: Codable { let container = try decoder.container(keyedBy: CodingKeys.self) self.header = try container.decode(RenderBlockContent.HeaderType.self, forKey: .header) - self.alignments = try container.decodeIfPresent([RenderBlockContent.ColumnAlignment].self, forKey: .alignments) + + let rawAlignments = try container.decodeIfPresent([RenderBlockContent.ColumnAlignment].self, forKey: .alignments) + if let alignments = rawAlignments, !alignments.allSatisfy({ $0 == .unset }) { + self.alignments = alignments + } else { + self.alignments = nil + } + self.rows = try container.decode([RenderBlockContent.TableRow].self, forKey: .rows) self.metadata = try container.decodeIfPresent(RenderContentMetadata.self, forKey: .metadata) From ef38e3adcaa7fbff3b256ca79a3cb5adee5440e1 Mon Sep 17 00:00:00 2001 From: Victoria Mitchell Date: Wed, 28 Sep 2022 15:44:21 -0600 Subject: [PATCH 6/6] ensure that all-unset alignments and no alignments compare equal --- .../Content/RenderBlockContent.swift | 26 ++++++- .../Model/RenderContentMetadataTests.swift | 74 +++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) diff --git a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift index 8a70bbb61d..1471eb89e0 100644 --- a/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift +++ b/Sources/SwiftDocC/Model/Rendering/Content/RenderBlockContent.swift @@ -240,7 +240,7 @@ public enum RenderBlockContent: Equatable { } /// A table that contains a list of row data. - public struct Table: Equatable { + public struct Table { /// The style of header in this table. public var header: HeaderType /// The text alignment of each column in this table. @@ -577,6 +577,30 @@ public enum RenderBlockContent: Equatable { } } +extension RenderBlockContent.Table: Equatable { + public static func == (lhs: RenderBlockContent.Table, rhs: RenderBlockContent.Table) -> Bool { + guard lhs.header == rhs.header + && lhs.extendedData == rhs.extendedData + && lhs.metadata == rhs.metadata + && lhs.rows == rhs.rows + else { + return false + } + + var lhsAlignments = lhs.alignments + if let align = lhsAlignments, align.allSatisfy({ $0 == .unset }) { + lhsAlignments = nil + } + + var rhsAlignments = rhs.alignments + if let align = rhsAlignments, align.allSatisfy({ $0 == .unset }) { + rhsAlignments = nil + } + + return lhsAlignments == rhsAlignments + } +} + // Writing a manual Codable implementation for tables because the encoding of `extendedData` does // not follow from the struct layout. extension RenderBlockContent.Table: Codable { diff --git a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift index 021b89a5d8..07d79cac60 100644 --- a/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift +++ b/Tests/SwiftDocCTests/Model/RenderContentMetadataTests.swift @@ -200,6 +200,80 @@ class RenderContentMetadataTests: XCTestCase { try assertRoundTripCoding(renderedTable) } + + /// Verifies that a table with `nil` alignments and a table with all-unset alignments still compare as equal. + func testRenderedTableEquality() throws { + let (bundle, context) = try testBundleAndContext(named: "TestBundle") + var renderContentCompiler = RenderContentCompiler(context: context, bundle: bundle, identifier: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/path", fragment: nil, sourceLanguage: .swift)) + + let source = """ + | Column 1 | Column 2 | + | ------------- | ------------- | + | Cell 1 | Cell 2 | + | Cell 3 | Cell 4 | + """ + let document = Document(parsing: source) + + let result = try XCTUnwrap(renderContentCompiler.visit(document.child(at: 0)!)) + let renderedTable = try XCTUnwrap(result.first as? RenderBlockContent) + guard case let .table(decodedTable) = renderedTable else { + XCTFail("Unexpected RenderBlockContent element") + return + } + XCTAssertNil(decodedTable.alignments) + var modifiedTable = decodedTable + modifiedTable.alignments = [.unset, .unset] + + XCTAssertEqual(decodedTable, modifiedTable) + } + + /// Verifies that two tables with otherwise-identical contents but different column alignments compare as unequal. + func testRenderedTableInequality() throws { + let (bundle, context) = try testBundleAndContext(named: "TestBundle") + var renderContentCompiler = RenderContentCompiler(context: context, bundle: bundle, identifier: ResolvedTopicReference(bundleIdentifier: bundle.identifier, path: "/path", fragment: nil, sourceLanguage: .swift)) + + let decodedTableWithUnsetColumns: RenderBlockContent.Table + do { + let source = """ + | Column 1 | Column 2 | + | ------------- | ------------- | + | Cell 1 | Cell 2 | + | Cell 3 | Cell 4 | + """ + let document = Document(parsing: source) + + let result = try XCTUnwrap(renderContentCompiler.visit(document.child(at: 0)!)) + let renderedTable = try XCTUnwrap(result.first as? RenderBlockContent) + guard case let .table(decodedTable) = renderedTable else { + XCTFail("Unexpected RenderBlockContent element") + return + } + decodedTableWithUnsetColumns = decodedTable + } + + let decodedTableWithLeftColumns: RenderBlockContent.Table + do { + let source = """ + | Column 1 | Column 2 | + | :------------ | :------------ | + | Cell 1 | Cell 2 | + | Cell 3 | Cell 4 | + """ + let document = Document(parsing: source) + + // Verifies that a markdown table renders correctly. + + let result = try XCTUnwrap(renderContentCompiler.visit(document.child(at: 0)!)) + let renderedTable = try XCTUnwrap(result.first as? RenderBlockContent) + guard case let .table(decodedTable) = renderedTable else { + XCTFail("Unexpected RenderBlockContent element") + return + } + decodedTableWithLeftColumns = decodedTable + } + + XCTAssertNotEqual(decodedTableWithUnsetColumns, decodedTableWithLeftColumns) + } func testStrikethrough() throws { let (bundle, context) = try testBundleAndContext(named: "TestBundle")