Skip to content

fix: preserve line endings when updating a file - #48

Merged
code-yeongyu merged 1 commit into
mainfrom
fix/preserve-line-endings
Oct 3, 2026
Merged

code-yeongyu merged 1 commit into
mainfrom
fix/preserve-line-endings

Conversation

@code-yeongyu

@code-yeongyu code-yeongyu commented Oct 3, 2026 •

Copy link
Copy Markdown
Owner

Fixes #47.

Summary

  • The bug: an update split the file on LF and joined it back with LF, so a one-line patch rewrote every line of a CRLF or mixed-ending file. That showed up in git, and in the tool's own preview as a whole-file diff.

  • The model: src/line-endings.ts parses the file into lines that keep their own ending (\r\n, \n, or a lone \r). A matched chunk is replaced around its context lines, so those keep their exact text and ending. Inserted lines take the file's first ending. Every line still ends with one, as before. This is Codex's PreserveLineEndings model (text_file.rs, file_update.rs compute_replacements).

  • Codex comparison: in Codex this model is behind apply_patch_preserve_line_endings (under development, default off), and its conformance scenarios run with it on. oh-my-pi restores the detected file ending after edits.

  • Two user-visible changes:

    1. Endings are preserved.
    2. A context line matched only after trimming (e.g. trailing spaces) is now left exactly as it is in the file, instead of being rewritten to the patch's copy.

    LF-only files produce the same bytes as before.

Verification

  • bun run check (typecheck + biome)
  • bun run test: 67/67
  • New test/line-endings.test.ts: 6 user-facing cases. Four fail on main and pass here: CRLF update with an insertion, mixed endings, a trimmed-match context line, and a CRLF one-line change counting (+1 -1) in the tool's rendered result. The other two pass on both and guard against regressions: an LF file without a final newline, and a failed hunk on a CRLF file leaving its bytes untouched.
  • Codex conformance scenarios replayed: 023 and 024 now pass. 24/26 on this branch; the remaining one, 017, is fixed by fix: reject stray lines between file sections instead of silently dropping them #46.
  • npm pack --dry-run includes src/line-endings.ts

apply_patch impact

  • No schema or grammar change
  • Path safety unchanged
  • CHANGELOG entry under [Unreleased]

Review in cubic

@code-yeongyu

Copy link
Copy Markdown
Owner Author

Lead in-session review at 6a03692: PASS. SourceText keeps each line's own ending; replacements are split around the chunk's context lines, so context lines keep their exact source text and ending (also correct under fuzzy matching, where the source text should win); inserted lines take the file's first ending; every line still ends with an ending, so the trailing-newline behavior and LF-only output are unchanged. Matches Codex's PreserveLineEndings mode (default-off upstream, as noted).

Updates split the file on LF and joined it back with LF, so a one-line patch
rewrote every line of a CRLF or mixed-ending file. The file is now parsed into
lines that keep their own ending; a chunk is replaced around its context lines,
which keep their exact text and ending; inserted lines take the file's first
ending. This is Codex's PreserveLineEndings model (text_file.rs,
compute_replacements).

Fixes #47
@code-yeongyu
code-yeongyu force-pushed the fix/preserve-line-endings branch from 6a03692 to abf6c13 Compare October 3, 2026 12:28
@code-yeongyu
code-yeongyu merged commit d6160ce into main Oct 3, 2026
7 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.

apply_patch rewrites CRLF and mixed line endings to LF on every update

1 participant