Skip to content

Keep a file's own line endings when saving it - #22

Draft
ewraj wants to merge 1 commit into
mainfrom
fix/preserve-line-endings
Draft

ewraj wants to merge 1 commit into
mainfrom
fix/preserve-line-endings

Conversation

@ewraj

@ewraj ewraj commented Oct 8, 2026

Copy link
Copy Markdown
Owner

Foundation fix F10 from PHASE_6_OVERVIEW.md (#17). This is a bug on main today, independent of Phase 6.

The bug

CodeMirror splits a document on \r\n, \r and \n, and doc.toString() always joins it back with \n. CodeView hands doc.toString() to the session, and save writes it through the adapter. As a result, editing one character of a Windows (CRLF) file in a picked folder rewrites every line ending on disk, and the whole file shows as changed in version control.

The fix

The editor keeps normalising to \n, which is what keeps positions and anchors simple. The file's line break is restored at the one place text leaves for disk:

  • New src/sources/lineBreaks.ts: dominantLineBreak, withLineBreak and keepLineBreaks.
  • CachingAdapter.writeFile and StoredAdapter.writeFile re-apply the dominant break of the cached text the edit replaces. Both the disk and the cache get the same bytes, so the next save sees CRLF again.
  • Notebooks are excluded. What's edited there is the generated \n reading view, and the file on disk is rebuilt as JSON by sourceToNotebook.
  • CachingAdapter is now exported so the save path can be tested directly.

Known limit: a file that mixes endings is written with the one it uses most. That's much closer to the original than today's rewrite to all \n. Keeping each line's own ending through arbitrary edits would mean mapping them through every change, which isn't worth it for what is almost always an accident.

Tested

  • Table-driven tests for detection (ties go to \n, and \r\n counts as one break) and for conversion.
  • Open CRLF → edit in a real CodeMirror EditorState → save through CachingAdapter: the bytes written to "disk" and to the cache are still CRLF. A second save keeps them. A Unix file stays Unix.
  • With the fix temporarily removed, the two CRLF tests fail, so the test does catch the bug.
  • The CRLF inputs are built as strings, not fixture files, because of autocrlf.
  • npm run typecheck, npm test (283 tests), npm run build and npm run test:e2e (29 tests, Chromium) all pass locally.

Not covered: an e2e test with a real picked folder. Playwright can't drive showDirectoryPicker, and the existing suite seeds IndexedDB instead.

🤖 Generated with Claude Code

CodeMirror reads \r\n, \r and \n alike and hands its document back joined
with \n. Save wrote that straight through, so editing one character of a
Windows file in a picked folder rewrote every line ending on disk, and git
showed the whole file as changed. AnnotateCode is supposed to leave code as
it found it.

The editor's normalising is what keeps positions and anchors simple, so it
stays. Instead the file's dominant line break, read from the cached text the
edit replaces, is put back in the adapter on the way out to disk and cache.
A file that mixes endings is written with the one it used most: much closer
than the all-\n rewrite, and keeping per-line endings through arbitrary edits
is not worth the machinery for what is almost always an accident.

Notebooks are left alone: what is edited there is the generated \n reading
view, and the file on disk is rebuilt as JSON.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ewraj added a commit that referenced this pull request Oct 8, 2026
Committing a note closes its card at once, then saves the note, and the chip
is drawn after the save. The test read the chips once, right after the card
closed, so it sometimes found none: 8 failures in 40 runs on main. It also
failed the rerun of #20 and one run on #22, neither of which touches notes.

Polling waits for the save. 80 of 80 runs pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
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.

1 participant