feat(inspector): set CSV delimiter, quote, encoding, and line ending with reload#1943
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a48d85a45b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let data = store.data | ||
| dialect = newDialect | ||
| store = CSVRowStore(data: data, dialect: newDialect) |
There was a problem hiding this comment.
After any property change, this recreates the store with every row still backed by the original byte ranges. On Save, CSVRowStore.rowSource(at:) returns those ranges as .rawBytes, and CSVWriter.append copies them verbatim instead of serializing them with the newly selected dialect. Thus changing encoding or line ending and saving leaves existing rows in the old format (and editing only one row produces a mixed-format file); with an encoding change, the writer can even prepend a new BOM to payload bytes still in the prior encoding. Materialize/rewrite the original rows when the dialect changes so the Save action promised by this flow actually writes the selected settings.
Useful? React with 👍 / 👎.
| ("UTF-16 LE", .utf16LittleEndian), | ||
| ("UTF-16 BE", .utf16BigEndian), |
There was a problem hiding this comment.
For a BOM-less UTF-16 CSV, selecting either option here cannot correctly repair detection: CSVStreamingParser scans and splits one byte at a time, so after consuming an ASCII delimiter or newline byte it leaves the adjacent UTF-16 NUL byte at the start of the next field/row. That yields malformed fields and, for CRLF input, spurious rows. BOM-less UTF-16 is precisely a case where the manual encoding picker is needed, so decode into code-unit-aligned text (or make the parser encoding-aware) before exposing these choices.
Useful? React with 👍 / 👎.
745f56a to
ba0d32f
Compare
a48d85a to
31a66c0
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 31a66c02d2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| static let encodings: [(label: String, encoding: String.Encoding)] = [ | ||
| ("UTF-8", .utf8), | ||
| ("UTF-16 LE", .utf16LittleEndian), | ||
| ("UTF-16 BE", .utf16BigEndian), |
There was a problem hiding this comment.
When a user selects Latin-1 or Windows-1252 and then edits a row containing an unrepresentable character (for example, an emoji from an originally UTF-8 file), CSVWriter.append gets nil from data(using:) but still appends the line ending. The save therefore silently replaces the entire edited record with a blank row instead of reporting its existing encodingFailed error; reject the save or otherwise handle the failed conversion.
Useful? React with 👍 / 👎.
|
Reviewed this PR. The review surfaced only a false positive and one below-threshold edge, so no code changes were needed (the branch was rebased onto the updated stack):
|
ba0d32f to
ef9a509
Compare
31a66c0 to
1e9c6e6
Compare
ef9a509 to
216a906
Compare
1e9c6e6 to
6a09a40
Compare
216a906 to
e3eb043
Compare
6a09a40 to
31cdb39
Compare
e3eb043 to
140e8ea
Compare
31cdb39 to
1b2c45a
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b2c45a17a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| quoteChar: quotes.indices.contains(quoteIndex) ? quotes[quoteIndex].byte : base.quoteChar, | ||
| encoding: encodings.indices.contains(encodingIndex) ? encodings[encodingIndex].encoding : base.encoding, | ||
| lineEnding: lineEndings.indices.contains(lineEndingIndex) ? lineEndings[lineEndingIndex].value : base.lineEnding, | ||
| hasBom: base.hasBom |
There was a problem hiding this comment.
When a UTF-8-BOM CSV is reloaded after selecting Latin-1 or Windows-1252, this retains hasBom == true but changes encoding; CSVStreamingParser.bomSkip returns zero for those encodings, so the three BOM bytes are parsed as part of the first header/field (for Latin-1, name). Preserve the source BOM length independently of the chosen encoding, or clear/normalize it when building the override dialect.
Useful? React with 👍 / 👎.
1b2c45a to
a04770b
Compare
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a04770ba8f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| CSVDialect( | ||
| delimiter: delimiters.indices.contains(delimiterIndex) ? delimiters[delimiterIndex].byte : base.delimiter, | ||
| quoteChar: quotes.indices.contains(quoteIndex) ? quotes[quoteIndex].byte : base.quoteChar, |
There was a problem hiding this comment.
When a user selects (for example) Comma for a .tsv document and saves, the file is written using that delimiter, but reopening it calls read(from:ofType:), which forcibly resets every public.tab-separated-values-text dialect to tab. The reopened document is therefore parsed incorrectly, so this newly exposed selection cannot persist for TSV files. Either constrain TSV documents to tab in this dialog or stop overriding their selected delimiter on read.
Useful? React with 👍 / 👎.
1 participant
Part 5 toward #1469 (CSV structural editing). The Set CSV Properties dialog. Stacked on #1942 (base branch
feat/1469-csv-header-toggle).What
(The escape-character field is the last remaining property from the issue; it needs a change to the shared parser and lands in a follow-up so this dialog stays low-risk.)
Why
Auto-detection can guess the delimiter or encoding wrong. #1469 asks for a manual override with a Reload so the user can fix it without leaving the app.
How (native / HIG)
presentAsSheet), which the HIG recommends over a floating panel for a text/selection-heavy, document-bound task. It has a Reload primary and a Cancel, per the sheet guidance.CSVConfigurableDocumentprotocol (refiningInspectorDocument, in PluginKit) exposescsvDialectandreload(with:), so the app-target inspector reaches CSV-specific settings through a protocol instead of importing the plugin. Added as a new protocol, so it is additive (no version bump).reload(with:)re-parses the originally loaded bytes with the new dialect, re-infers types, clears the undo stack (like a revert), and marks the document edited so Save writes with the new settings.CSVPropertyOptionshelper so the picker choices and the resultingCSVDialectare unit-testable; the BOM is preserved from the original.Tests
CSVPropertyOptions: index lookup from a dialect, unknown-value fallback, and building a dialect from picker indices (with BOM preserved).Docs / changelog
docs/features/csv-inspector.mdxauto-detection section notes the override.[Unreleased].Needs a local Xcode build to confirm compilation. PluginKit change is additive (a new protocol); no version bump.