From a57b7d3214de4de97635eff07aeaf74201670a96 Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Wed, 2 Sep 2026 20:16:21 +0200 Subject: [PATCH] feat(apple): give `HtmlConfig`'s optional settings their real Swift types MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `spreadsheetLimit` is an `NSValue`-boxed `ODRTableDimensions`, and Swift has no `@encode`: a caller had to spell `"{ODRTableDimensions=II}"` out to build the box at all, which is what OpenDocument.ios#185 carries — a second source of truth for the struct's layout that a new field would break silently, since `getValue:size:` checks the size and not the encoding. The six boxed properties are now `NS_REFINED_FOR_SWIFT`, and `apple/swift/Html+Optionals.swift` carries them under their own names as real optionals: `TableDimensions?`, `UInt64?`, `HtmlViewportMode?`, `UInt32?`, `Double?`, `UInt32?`. `@encode` stays in the ObjC layer, as an `NSValue (ODRTableDimensions)` category beside the struct. ObjC callers see the boxed properties unchanged. Nothing under `apple/tests` set any of them, so the write path had no coverage; two tests now round-trip all six and prove the sheet limit reaches the html. Closes #759. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_018cMYRXLJdiCkH65Jm2W9B5 --- CHANGELOG.md | 5 +++ apple/AGENTS.md | 5 +++ apple/README.md | 8 +++++ apple/include/OdrCoreObjC/ODRHtml.h | 18 ++++++---- apple/include/OdrCoreObjC/ODRTable.h | 9 +++++ apple/src/ODRTable.mm | 19 +++++++++++ apple/swift/Html+Optionals.swift | 44 ++++++++++++++++++++++++ apple/tests/OdrCoreTests.swift | 51 ++++++++++++++++++++++++++++ 8 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 apple/swift/Html+Optionals.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c6d91719..e12e3643b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- **Breaking** (Swift only): `HtmlConfig`'s optional settings are Swift + optionals of the real type rather than `NSNumber`/`NSValue` boxes — + `spreadsheetLimit` is a `TableDimensions?`, `initialZoom` a `Double?`, and so + on. ObjC callers see the boxed properties unchanged. Closes #759. + - The HTML renderer warns, rather than silently dropping, when it reaches an element type it has no `translate_*` for. Across the test corpus that is `page_break` and nothing else. Towards #150. diff --git a/apple/AGENTS.md b/apple/AGENTS.md index cfd6176d3..50392a050 100644 --- a/apple/AGENTS.md +++ b/apple/AGENTS.md @@ -94,6 +94,11 @@ consumer, and a SwiftPM binary target gives the consumer no way to pass `NS_ERROR_ENUM`. The ObjC API is the API; the Swift target on top is only for what annotations cannot express, the same way `../android` refuses to reimplement the java API in kotlin. +- **A boxed `std::optional` is `NS_REFINED_FOR_SWIFT`.** The box moves to + `__name` and `swift/*+Optionals.swift` carries the real optional under the + real name. Swift has no `@encode`, so a boxed struct is otherwise unwritable + from Swift; `@encode` stays in the ObjC layer (`NSValue (ODRTableDimensions)` + in `ODRTable.h`). - **Pin `os.version` in every conan profile.** An unset deployment target floats with the runner's SDK and would disagree with `Package.swift`'s `platforms:`; `CMakeLists.txt` fails the configure rather than let that ship. diff --git a/apple/README.md b/apple/README.md index 4baa46d75..ff54e6657 100644 --- a/apple/README.md +++ b/apple/README.md @@ -35,6 +35,14 @@ for view in service.views { Nothing needs configuring first: the renderer's css and JS are part of the library and are written into the HTML it produces. +`HtmlConfig`'s optional settings are plain Swift optionals of the right type: + +```swift +config.spreadsheetLimit = TableDimensions(rows: 100_000, columns: 500) +config.initialZoom = 1.5 +config.pageRangeEnd = nil // to the last page +``` + ## Serve it into a web view Rendering on demand and serving over loopback is what OpenDocument.ios does, and diff --git a/apple/include/OdrCoreObjC/ODRHtml.h b/apple/include/OdrCoreObjC/ODRHtml.h index 441fb15d9..166ceac3d 100644 --- a/apple/include/OdrCoreObjC/ODRHtml.h +++ b/apple/include/OdrCoreObjC/ODRHtml.h @@ -88,22 +88,27 @@ NS_SWIFT_NAME(HtmlConfig) @property(nonatomic) ODRHtmlColorScheme colorScheme; /// `nil` for no limit. -@property(nonatomic, strong, nullable) NSValue *spreadsheetLimit; +@property(nonatomic, strong, nullable) + NSValue *spreadsheetLimit NS_REFINED_FOR_SWIFT; /// Most cells written for one sheet; bounds the rows by the sheet's width. /// `nil` for no budget. -@property(nonatomic, strong, nullable) NSNumber *spreadsheetCellLimit; +@property(nonatomic, strong, nullable) + NSNumber *spreadsheetCellLimit NS_REFINED_FOR_SWIFT; @property(nonatomic) BOOL spreadsheetLimitByContent; @property(nonatomic) ODRHtmlTableGridlines spreadsheetGridlines; @property(nonatomic) ODRHtmlViewportMode viewportMode; /// Overrides `viewportMode` for spreadsheets when set. -@property(nonatomic, strong, nullable) NSNumber *spreadsheetViewportMode; +@property(nonatomic, strong, nullable) + NSNumber *spreadsheetViewportMode NS_REFINED_FOR_SWIFT; /// Raw `content` for the viewport meta tag; overrides the modes above. @property(nonatomic, copy, nullable) NSString *viewportContent; /// The width the output is shown at, in css pixels; fits paged content to it. -@property(nonatomic, strong, nullable) NSNumber *viewportWidth; +@property(nonatomic, strong, nullable) + NSNumber *viewportWidth NS_REFINED_FOR_SWIFT; /// The zoom the view opens at, 1 being actual size; `nil` follows the fit. -@property(nonatomic, strong, nullable) NSNumber *initialZoom; +@property(nonatomic, strong, nullable) + NSNumber *initialZoom NS_REFINED_FOR_SWIFT; /// The least distance the generated content keeps from the view's border. A /// set side raises the inset the view already has, never lowers it. @@ -121,7 +126,8 @@ NS_SWIFT_NAME(HtmlConfig) /// Render only pages `[begin, end)`, 0-based. `nil` end means to the last page. @property(nonatomic) uint32_t pageRangeBegin; -@property(nonatomic, strong, nullable) NSNumber *pageRangeEnd; +@property(nonatomic, strong, nullable) + NSNumber *pageRangeEnd NS_REFINED_FOR_SWIFT; @property(nonatomic) ODRPdfTextMode pdfTextMode; @property(nonatomic, copy) NSArray *pdfDualLayerFallbackFonts; diff --git a/apple/include/OdrCoreObjC/ODRTable.h b/apple/include/OdrCoreObjC/ODRTable.h index 0434580dd..7d8000421 100644 --- a/apple/include/OdrCoreObjC/ODRTable.h +++ b/apple/include/OdrCoreObjC/ODRTable.h @@ -21,6 +21,15 @@ NS_INLINE ODRTableDimensions ODRTableDimensionsMake(uint32_t rows, return (ODRTableDimensions){.rows = rows, .columns = columns}; } +/// Boxing needs `@encode`, which Swift has no equivalent of. +@interface NSValue (ODRTableDimensions) ++ (NSValue *)odr_valueWithTableDimensions:(ODRTableDimensions)dimensions + NS_SWIFT_NAME(value(tableDimensions:)); +/// Zeroes for a value holding anything else. +@property(nonatomic, readonly) + ODRTableDimensions odr_tableDimensionsValue NS_SWIFT_NAME(tableDimensionsValue); +@end + /// A cell address — `odr::TablePosition`. typedef struct ODRTablePosition { uint32_t column; diff --git a/apple/src/ODRTable.mm b/apple/src/ODRTable.mm index e43d21065..cff24b7ca 100644 --- a/apple/src/ODRTable.mm +++ b/apple/src/ODRTable.mm @@ -2,6 +2,8 @@ #import "ODRInternal.h" +#include + #include #include @@ -15,6 +17,23 @@ static_assert(sizeof(ODRTablePosition) == sizeof(odr::TablePosition), "ODRTablePosition drifted from odr::TablePosition"); +@implementation NSValue (ODRTableDimensions) + ++ (NSValue *)odr_valueWithTableDimensions:(ODRTableDimensions)dimensions { + return [NSValue valueWithBytes:&dimensions + objCType:@encode(ODRTableDimensions)]; +} + +- (ODRTableDimensions)odr_tableDimensionsValue { + ODRTableDimensions dimensions = ODRTableDimensionsMake(0, 0); + if (strcmp(self.objCType, @encode(ODRTableDimensions)) == 0) { + [self getValue:&dimensions size:sizeof(dimensions)]; + } + return dimensions; +} + +@end + @implementation ODRTableAddress // The parses throw on anything that is not a cell address — an empty string, a diff --git a/apple/swift/Html+Optionals.swift b/apple/swift/Html+Optionals.swift new file mode 100644 index 000000000..c94ca9444 --- /dev/null +++ b/apple/swift/Html+Optionals.swift @@ -0,0 +1,44 @@ +import Foundation + +// `HtmlConfig`'s optional settings are `NS_REFINED_FOR_SWIFT`, so the +// `NSNumber`/`NSValue` boxes are `__`-prefixed and these carry the names. + +extension HtmlConfig { + public var spreadsheetLimit: TableDimensions? { + get { __spreadsheetLimit?.tableDimensionsValue } + set { + __spreadsheetLimit = newValue.map { NSValue.value(tableDimensions: $0) } + } + } + + public var spreadsheetCellLimit: UInt64? { + get { __spreadsheetCellLimit?.uint64Value } + set { __spreadsheetCellLimit = newValue.map(NSNumber.init(value:)) } + } + + public var spreadsheetViewportMode: HtmlViewportMode? { + get { + __spreadsheetViewportMode.flatMap { + HtmlViewportMode(rawValue: $0.intValue) + } + } + set { + __spreadsheetViewportMode = newValue.map { NSNumber(value: $0.rawValue) } + } + } + + public var viewportWidth: UInt32? { + get { __viewportWidth?.uint32Value } + set { __viewportWidth = newValue.map(NSNumber.init(value:)) } + } + + public var initialZoom: Double? { + get { __initialZoom?.doubleValue } + set { __initialZoom = newValue.map(NSNumber.init(value:)) } + } + + public var pageRangeEnd: UInt32? { + get { __pageRangeEnd?.uint32Value } + set { __pageRangeEnd = newValue.map(NSNumber.init(value:)) } + } +} diff --git a/apple/tests/OdrCoreTests.swift b/apple/tests/OdrCoreTests.swift index f4514c8d7..0bf5076b5 100644 --- a/apple/tests/OdrCoreTests.swift +++ b/apple/tests/OdrCoreTests.swift @@ -153,6 +153,57 @@ final class HtmlTests: XCTestCase { XCTAssertFalse(try view.writeHtml(resources: &resources).isEmpty) } + func testSpreadsheetLimitRoundTripsAndReachesTheHtml() throws { + let config = HtmlConfig() + config.spreadsheetLimit = TableDimensions(rows: 2, columns: 1) + XCTAssertEqual(config.spreadsheetLimit?.rows, 2) + XCTAssertEqual(config.spreadsheetLimit?.columns, 1) + config.spreadsheetLimitByContent = false + + // A csv renders as a spreadsheet, so this needs no fixture. + let path = try write( + "alpha,beta\ngamma,delta\nepsilon,zeta\n", as: "table.csv") + let file = try DecodedFile.decode(path: path) + let service = try HtmlTranslator.translate( + file: file, cachePath: try temporaryDirectory(), config: config) + var resources: NSArray? + let html = try XCTUnwrap(service.views.first).writeHtml(resources: &resources) + + XCTAssertTrue(html.contains("alpha"), "the first cell is missing") + XCTAssertFalse(html.contains("epsilon"), "the row limit did not apply") + XCTAssertFalse(html.contains("beta"), "the column limit did not apply") + + config.spreadsheetLimit = nil + XCTAssertNil(config.spreadsheetLimit) + } + + func testBoxedNumbersRoundTrip() throws { + let config = HtmlConfig() + config.spreadsheetCellLimit = 1234 + config.spreadsheetViewportMode = .fitWidth + config.viewportWidth = 390 + config.initialZoom = 1.5 + config.pageRangeEnd = 7 + + XCTAssertEqual(config.spreadsheetCellLimit, 1234) + XCTAssertEqual(config.spreadsheetViewportMode, .fitWidth) + XCTAssertEqual(config.viewportWidth, 390) + XCTAssertEqual(config.initialZoom, 1.5) + XCTAssertEqual(config.pageRangeEnd, 7) + + config.spreadsheetCellLimit = nil + config.spreadsheetViewportMode = nil + config.viewportWidth = nil + config.initialZoom = nil + config.pageRangeEnd = nil + + XCTAssertNil(config.spreadsheetCellLimit) + XCTAssertNil(config.spreadsheetViewportMode) + XCTAssertNil(config.viewportWidth) + XCTAssertNil(config.initialZoom) + XCTAssertNil(config.pageRangeEnd) + } + func testBringOfflineWritesFiles() throws { let output = try temporaryDirectory() let html = try service().bringOffline(to: output)