feat(plugins): encoding and byte order mark options for CSV export - #2652
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Automations to automatically generate PRs for you. |
|
Screenshots for the Before / After section. Before — the options pane ends at Delimiter, Quote, Line break, Decimal, with Reset to Defaults visible. After, UTF-8 — an Encoding row opens the picker group, with Include byte order mark below it, enabled because UTF-8 carries one. After, Windows-1252 — the same pane with the encoding changed. Include byte order mark is dimmed, because that encoding has no mark. The stored choice survives the detour, so switching back to UTF-8 restores it. |
Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
Fixes #2534.
CSV export had no concept of output encoding. Every byte it wrote went through
String.toUTF8Data(), a no-argument PluginKit extension hardcoded to.utf8and shared by seven plugins, so a file for a tool that expects Windows-1252 could not be produced and the byte order mark Excel on Windows needs to read UTF-8 was never written. A value the encoding could not represent had nowhere to be reported.What this adds
An Encoding picker on the CSV export options (UTF-8, ISO Latin 1, Windows-1252, spelled the way the CSV import picker already spells them), an Include byte order mark checkbox below it, and a warning that names the characters the encoding could not represent instead of writing
?in silence.The default is unchanged: UTF-8, no mark.
CSVExportBytesTests.defaultExportIsPlainUTF8asserts the stock export is byte-for-byte what it was.The warning arrives through
ExportFormatResult.warnings, which already reaches anNSAlertthat retitles itself "Export completed with warnings", switches to.warningstyle, and withholds the "Do not show this again" checkbox.XLSXExportPlugin,SQLExportPluginandParquetExportPluginalready use it.Measured facts the design rests on
Each of these was measured with a
swiftcharness on this toolchain, not read off a doc page.String.data(using:)is all or nothing: one unrepresentable scalar returns nil for the whole string.allowLossyConversionwrites the substitute and reports nothing.isoLatin1andwindowsCP1252, the only byte a lossy conversion ever emits is0x3F, and it is never a CSV metacharacter. A substitution cannot corrupt field structure.afollowed by U+0301 encodes to0xE1in ISO Latin 1 while U+0301 alone does not. The scan therefore walks grapheme clusters, not scalars, or it would name characters the file kept intact..utf16,.utf32and.unicodeprepend a mark to the result of everydata(using:)call ("A"givesFF FE 41 00, then"B"givesFF FE 42 00). A row-per-call writer using one of those would write a mark before every row. Only explicit-endian variants are safe, which is whyPluginTextEncodingdocuments that constraint.NSString.getBytes(...remaining:)converts the longest encodable run and reports where it stopped, so the scan skips whole runs instead of testing one character at a time. On a 200 KB field holding one unrepresentable character: 0.33 ms against 26 ms for a per-character walk, with identical results on all 22 cross-check cases.Why no UTF-16
The reporter's BOM line names UTF-16, and it was deliberately left out. Compiled against the real
CSVDialectandCSVStreamingParsersources, a UTF-16 LE CSV with a mark parses back as mojibake (namereads as渀愀洀攀) plus a spurious empty row, because the parser is byte-oriented: it ends a row on a single0x0Aand a field on a single delimiter byte, applying the encoding only when decoding the resulting run. UTF-16 BE round-trips clean. Shipping UTF-16 LE export would mean writing files TablePro's own CSV inspector and auto-detecting importer misread, and shipping BE alone is incoherent. The parser defect predates this change (the inspector already offers both variants) and is written up below.The repair that had to land first
CSVExportOptionswas the only export options struct in the repo with no custominit(from:). Measured: adding any non-optional field makesJSONDecoderthrowkeyNotFoundon every stored payload,PluginSettingsStorage.loadanswers a throwing decode with nil, andloadSettingsleaves the defaults. Shipping the two new fields without that init would have silently reset every user's saved delimiter, quote, line break and decimal choice.SQLExportModels,MarkdownExportModels,JSONExportModels,HTMLExportModels,XMLExportModelsandParquetExportModelsall already had one.Two released defects fixed on the way
Both are in the file and view this change edits, and leaving either would have shipped a new control that behaved differently from the one beside it.
Text(handling.rawValue), which resolves to the verbatimStringoverload; thedisplayNamethat would have localized it was dead code and pointed atBundle(for: CSVExportPlugin.self), a.tablepluginthat ships no catalog and returns the key with no fallback. The three translations were already in the catalog and were never asked for.# Table:comment line in a multi-table export hardcoded\nwhile every other write used the chosen line ending, so a CRLF export carried one lone LF per table and a reader splitting on CRLF joined the comment onto the header row.Review
Codex was out of credits (
You've hit your usage limit ... try again at Sep 7th), so/code-review highread the diff instead. It returned seven findings and all seven were acted on:settingsper line. One plugin instance serves every window, so changing the picker in another window's dialog could switch encoding mid-file. The whole options struct is now captured once, which also closes the same hole for the delimiter and the line ending.detectingUnrepresented:to stop paying for an answer it has.U+0081.Localizable.xcstrings, which would have shipped English into the ko, tr, vi, zh-Hans and zh-Hant builds. They are in the catalog and translated;StringCatalogIntegrityTestspasses on them and coverage holds at 5078/5080, the same two format-only strings as before.await; a second call could restore the first call's test options into the developer's real settings. Runs are now chained.What was built and tested
verify.sh buildverify.sh build CSVExportverify.sh test(5 suites, incl.StringCatalogIntegrityTests)verify.sh lint(absolute paths)verify.sh docsverify.sh abivs merge baseThe ABI diff is purely additive: a new enum, two new structs and a new caseless enum, with no existing signature touched. No
currentPluginKitVersionbump and no registry re-release.CSVExportPluginis bundled, so it always ships against a matching PluginKit, and no registry plugin references the new symbols.verify.sh pluginsfails locally onmacro expansion @TaskLocal:1:2: error: unknown attribute 'usableFromInlinenonisolated'from oracle-nio. That is the only error in 8,730 lines of log, it is a known incompatibility between the vendored fork and the local Xcode 27 beta, and it blocks the aggregate for any change underPlugins/. The changed plugin target was built on its own instead; CI runs the aggregate on its own toolchain.No
TableProUITestscoverage was added. The export sheet needs a live connection, and none of the four existing CSV export options has UI automation either, so this stays consistent with the rest of the pane rather than adding a lone flaky case.Before / after
The CSV options pane before, then with the Encoding picker and the byte order mark checkbox, then with Windows-1252 selected so the checkbox dims because that encoding has no mark. Screenshots are attached in a follow-up comment.
One consequence worth flagging: the two new rows push Decimal and Reset to Defaults below the fold of the options pane, which is a
ScrollViewand scrolls to reach them. Say the word if you would rather the new controls sat last instead.Other defects found while investigating
Verified, not fixed here, and not blocking this change.
CSVExportPlugin.swift:76calls the legacystreamRows(table:databaseName:); onlystreamRows(for:)readsPluginExportTable.rowScope. Set a filter and a row limit on a table, export as CSV, and every row and column is written while the filter icon stays lit and its accessibility value still reports the narrowing. HTML, JSON, SQL, Markdown, XML and Parquet all usestreamRows(for:). XLSX and MQL have the same bug. Small: one call site per plugin.CSVInspectorPlugin/CSVWriter.swift:98appends the encoded line insideif letand the line ending outside it. Open a Windows-1252 CSV, edit a cell to hold東京, save: the document reports a clean save, the unsaved dot clears, and that row is now an empty line.CSVWriter.WriteError.encodingFailedis declared and never thrown. This is the same class of defect this PR fixes for export. Small.CSVStreamingParseris byte-oriented, so UTF-16 LE is unreadable. Measured above. Shared by CSV import and the CSV inspector, and the inspector's own picker already offers UTF-16 LE and BE. Fixing it means makingindexRows,parseRowandfieldcode-unit aware. Medium, and it is what would unblock UTF-16 export.CSVDialect.swift:117ends withcounts.max(by:)over a[UInt8: Int], and Dictionary iteration order depends on the per-process hash seed. Measured over 14 processes ona;b\n1,5;2,5\n(2 commas, 2 semicolons): 8 runs chose;, 6 chose,. When comma wins, the header parses as one field and the second column is silently dropped from the import. A single-column CSV is a four-way tie at zero, and the inspector then writes back whichever delimiter that launch picked. Small: iterate a fixed-order candidate array.TablePro/Views/Inspector/CSVPropertyOptions.swift:35says "Latin-1". Pre-existing, cosmetic.Dropped after verification: the claim that
PluginExportError.encodingFailedis a reachable defect. Its message does hardcode UTF-8 and it is thrown from one site, butString.data(using: .utf8)cannot return nil for a Swift String, so it has never reached a user. This change does not route through it.https://claude.ai/code/session_01JH7gTeN4YSSASrHFeFJ9xT