Repository navigation
fix(core): preserve missing final newline in apply_patch - #59154
Open
benjaminkincses wants to merge 1 commit into
Open
benjaminkincses wants to merge 1 commit into
benjaminkincses wants to merge 1 commit into
Conversation
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
I verified the fix. With the simulated model in the TUI, I ran a patch that only edits the first line of a file with no final newline. On the base the file gained a trailing newline and the diff showed the last line changed. With this PR the file keeps its original ending and only the edited line changes. Calling derive() directly also showed that a file ending in a blank line now keeps it, CRLF endings are preserved, and a patch that rewrites the last line still adds the final newline as before. The patch tests in packages/core pass (91/91) and packages/util type-checks clean.
Public API: unchanged.
Nothing blocks a merge. Two notes:
- #56128 is open and rewrites the same lines in
derive(), always adding the final newline back. The two conflict, so only one direction can land. - A patch that rewrites the last line of a file without a final newline still adds one, so the
.editorconfigcase raised in the issue thread is only partly addressed. That seems a reasonable trade-off, since the patch format has no way to say "no newline at end of file".
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue for this PR
Closes #45912
Type of change
What does this PR do?
derive()inpackages/util/src/patch.tsalways appended a trailing newline to the patched content, even when the patch never touched the end of the file. A file without a final newline gained one, and the reported diff showed a change nobody asked for.The fix tracks whether any replacement reaches the end of the file. If none does, the result keeps the original ending: no final newline stays no final newline, a trailing blank line stays, CRLF endings stay as they are. If a replacement does rewrite the last line, the ending keeps the normalized behavior it has today, so the existing tests for that case do not change.
Same direction as #51803 proposed for the inverse case (a dropped trailing blank line), applied here to the missing final newline.
How did you verify your code works?
New tests in
packages/core/test/patch.test.tscover the issue repro, a mid-file edit on a file without a final newline, a file ending in a blank line, CRLF with no final newline, and a patch that rewrites the last line.Before:
derive("f", [{ oldLines: ["line1"], newLines: ["LINE1"] }], "line1\nline2\nline3")returned"LINE1\nline2\nline3\n".After: it returns
"LINE1\nline2\nline3".Ran
bun run script/test.ts test/patch.test.ts test/tool-patch.test.tsinpackages/core: 91 pass, 0 fail.bun typecheckclean inpackages/coreandpackages/util. Fullpackages/utilsuite: 135 pass, 0 fail.Checklist