Repository navigation
fix(core): preserve trailing blank lines when applying patches - #51803
icearia0219 wants to merge 2 commits into
Conversation
Fixes anomalyco#42081 Revisits the approach in anomalyco#42084 by codeAnqiang-ma, closed by automated cleanup. Signed-off-by: icearia0219 <[email protected]>
|
The following comment was made by an LLM, it may be inaccurate: |
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The legacy implementation mishandles pure additions when genuine trailing blank lines remain.
Review effort: Lite
Findings: None
What changed in this PR
Fixes apply_patch trailing blank-line preservation in both patch implementations.
Changes:
- Preserve trailing newline sentinels during patch application.
- Add regression tests for repeated edits, BOMs, explicit removal, and diff counts.
| File | Description |
|---|---|
packages/opencode/test/patch/patch.test.ts |
Tests repeated legacy updates. |
packages/opencode/src/patch/index.ts |
Applies the legacy patch fix. |
packages/core/test/tool-apply-patch.test.ts |
Tests repeated tool updates and diff counts. |
packages/core/test/patch.test.ts |
Tests newline and BOM behavior. |
packages/core/src/patch.ts |
Preserves trailing newline state. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Signed-off-by: icearia0219 <[email protected]>
|
Reproduced and fixed in 5026e30. The legacy pure-addition branch subtracted one from the insertion index when the last line was empty, even though Added six cases per implementation covering empty files, blank-only files, unterminated text, and one or more trailing newlines. Three legacy cases fail without this follow-up. After the fix, the selected core suites pass 26 tests and the legacy suites pass 55 tests; both package typechecks pass. The PR description includes the updated verification results. |
|
This branch has been upgraded to V2. We're no longer taking PRs for V1. If you think this is still relevant, please port it over and open it for V2. — from 𝕺𝖕𝖊𝖓𝕮𝖔𝖉𝖊 |
Issue for this PR
Closes #42081
Type of change
What does this PR do?
Preserve trailing blank lines when
apply_patchchanges another part of a file. Both patch implementations now remember whether they removed the trailing split sentinel and restore it, rather than confusing a blank content line with that sentinel.This revisits #42084 by @codeAnqiang-ma, which was closed by automated cleanup. Credit to that issue/PR for the diagnosis and approach. The bug still reproduces on current
dev; this submission adds repeated-edit coverage and checks the reported diff counts. Explicit blank-line removal still works.Following review, pure additions in the legacy implementation now append after all content lines, including genuine blank lines; the split sentinel has already been removed before insertion.
Authored and locally verified by Codex with the account owner's authorization; no manual user or full CLI/LLM end-to-end validation.
How did you verify your code works?
On Windows with Bun 1.3.14:
packages/core:bun test test/patch.test.ts test/tool-apply-patch.test.ts— 26 pass.packages/opencode:bun test test/patch/patch.test.ts test/tool/apply_patch.test.ts --timeout 30000— 55 pass.bun typecheckpasses in both packages.git diff --checkpass. Oxlint reports zero errors and the same 16 warnings as the unchanged base for these files.Tests use real temporary files, including repeated updates through tool settlement and the legacy patch API. The full repository suite was not run.
Screenshots / recordings
N/A — no UI changes.
Checklist