Skip to content

Add and rename sheet columns by offset, so an upgrade changes only the bytes it names - #67

Merged
juancobo merged 1 commit into
mainfrom
fix/column-bytes
Oct 17, 2026
Merged

juancobo merged 1 commit into
mainfrom
fix/column-bytes

Conversation

@juancobo

Copy link
Copy Markdown
Member

An upgrade that adds or renames a sheet column rewrote the whole file: every line ended in LF and every field was quoted afresh, so a site's CSVs showed changes on every line after an upgrade. The two operations now write only the header field they rename, or the fields they insert, and keep the file's line endings, quoting, byte order mark and final newline. A sheet whose records cannot be read back exactly is still written whole, as before. No D1 migrations.

Copilot AI balanced review requested due to automatic review settings October 17, 2026 02:54

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Column insertion repeatedly rebuilds the sheet, introducing quadratic copying that can delay upgrades.

1 open issue
What changed in this PR

Updates upgrade-time CSV column additions and renames to preserve untouched formatting rather than rewrite entire sheets.

Changes:

  • Adds offset-based edits with whole-sheet fallback.
  • Shares CSV field scanning and quoting helpers.
  • Tests preservation of line endings, quoting, byte order marks, and final newlines.
File Description
tests/​manifest-runner.test.ts Updates formatting-preservation expectations.
tests/​manifest-runner-rename-bytes.test.ts Adds byte-preservation regression tests.
tests/​csv-mixed-line-endings.test.ts Checks exact mixed-ending output.
app/​lib/​manifest-runner.server.ts Implements offset-based column edits.
app/​lib/​csv-records.ts Exports shared field helpers.
app/​lib/​csv-language-match.server.ts Reuses shared CSV helpers.

Review effort: Balanced

Comment on lines +406 to +408
let next = content;
for (const [at, text] of inserts.reverse()) next = next.slice(0, at) + text + next.slice(at);
return next;
@juancobo
juancobo merged commit 6846384 into main Oct 17, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants