From f585ef5eeedc2489b570b6476c5b4d4e5b0b4787 Mon Sep 17 00:00:00 2001 From: Ian Qvist Date: Sun, 24 May 2026 19:38:35 +0200 Subject: [PATCH] Preserve trailing newlines instead of adding unconditionally --- packages/opencode/src/patch/index.ts | 11 ++--- packages/opencode/src/tool/apply_patch.ts | 4 +- packages/opencode/test/patch/patch.test.ts | 26 ++++++++++-- .../opencode/test/tool/apply_patch.test.ts | 42 +++++++++++++++---- 4 files changed, 65 insertions(+), 18 deletions(-) diff --git a/packages/opencode/src/patch/index.ts b/packages/opencode/src/patch/index.ts index 42b26fe96309..953ccf03d5e1 100644 --- a/packages/opencode/src/patch/index.ts +++ b/packages/opencode/src/patch/index.ts @@ -313,19 +313,20 @@ export function deriveNewContentsFromChunks( originalText: string, ): ApplyPatchFileUpdate { const originalContent = Bom.split(originalText) + const originalHadTrailingNewline = originalContent.text.endsWith("\n") - let originalLines = originalContent.text.split("\n") + const originalLines = originalContent.text.split("\n") // Drop trailing empty element for consistent line counting - if (originalLines.length > 0 && originalLines[originalLines.length - 1] === "") { + if (originalHadTrailingNewline || originalContent.text === "") { originalLines.pop() } const replacements = computeReplacements(originalLines, filePath, chunks) - let newLines = applyReplacements(originalLines, replacements) + const newLines = applyReplacements(originalLines, replacements) - // Ensure trailing newline - if (newLines.length === 0 || newLines[newLines.length - 1] !== "") { + // Preserve the original file's trailing newline state instead of adding one unconditionally. + if (originalHadTrailingNewline && newLines.length > 0 && newLines[newLines.length - 1] !== "") { newLines.push("") } diff --git a/packages/opencode/src/tool/apply_patch.ts b/packages/opencode/src/tool/apply_patch.ts index 84e84cc3962e..b4b6daeb217e 100644 --- a/packages/opencode/src/tool/apply_patch.ts +++ b/packages/opencode/src/tool/apply_patch.ts @@ -76,9 +76,7 @@ export const ApplyPatchTool = Tool.define( switch (hunk.type) { case "add": { const oldContent = "" - const newContent = - hunk.contents.length === 0 || hunk.contents.endsWith("\n") ? hunk.contents : `${hunk.contents}\n` - const next = Bom.split(newContent) + const next = Bom.split(hunk.contents) const diff = trimDiff(createTwoFilesPatch(filePath, filePath, oldContent, next.text)) let additions = 0 diff --git a/packages/opencode/test/patch/patch.test.ts b/packages/opencode/test/patch/patch.test.ts index e4952b9e0039..1ebb94bd35b3 100644 --- a/packages/opencode/test/patch/patch.test.ts +++ b/packages/opencode/test/patch/patch.test.ts @@ -331,11 +331,11 @@ PATCH` expect(result.modified).toHaveLength(1) const content = yield* Effect.promise(() => fs.readFile(emptyFile, "utf-8")) - expect(content).toBe("First line\n") + expect(content).toBe("First line") }), ) - it.live("should handle files with no trailing newline", () => + it.live("should preserve files with no trailing newline", () => Effect.gen(function* () { const filePath = path.join(tempDir, "no-newline.txt") yield* Effect.promise(() => fs.writeFile(filePath, "no newline")) @@ -351,7 +351,27 @@ PATCH` expect(result.modified).toHaveLength(1) const content = yield* Effect.promise(() => fs.readFile(filePath, "utf-8")) - expect(content).toBe("has newline now\n") + expect(content).toBe("has newline now") + }), + ) + + it.live("should preserve files with a trailing newline", () => + Effect.gen(function* () { + const filePath = path.join(tempDir, "with-newline.txt") + yield* Effect.promise(() => fs.writeFile(filePath, "has newline\n")) + + const patchText = `*** Begin Patch +*** Update File: ${filePath} +@@ +-has newline ++still has newline +*** End Patch` + + const result = yield* Patch.applyPatch(patchText) + expect(result.modified).toHaveLength(1) + + const content = yield* Effect.promise(() => fs.readFile(filePath, "utf-8")) + expect(content).toBe("still has newline\n") }), ) diff --git a/packages/opencode/test/tool/apply_patch.test.ts b/packages/opencode/test/tool/apply_patch.test.ts index be5754f3b40d..9c8fbe99082f 100644 --- a/packages/opencode/test/tool/apply_patch.test.ts +++ b/packages/opencode/test/tool/apply_patch.test.ts @@ -152,7 +152,7 @@ describe("tool.apply_patch freeform", () => { expect(updateFile?.patch).toContain("-line2") expect(updateFile?.patch).toContain("+changed") - expect(yield* readText(path.join(test.directory, "nested", "new.txt"))).toBe("created\n") + expect(yield* readText(path.join(test.directory, "nested", "new.txt"))).toBe("created") expect(yield* readText(modifyPath)).toBe("line1\nchanged\n") yield* expectReadFailure(deletePath) }), @@ -244,7 +244,7 @@ describe("tool.apply_patch freeform", () => { }), ) - it.instance("appends trailing newline on update", () => + it.instance("preserves missing trailing newline on update", () => Effect.gen(function* () { const test = yield* TestInstance const { ctx } = makeCtx() @@ -257,8 +257,24 @@ describe("tool.apply_patch freeform", () => { yield* execute({ patchText }, ctx) const contents = yield* readText(target) - expect(contents.endsWith("\n")).toBe(true) - expect(contents).toBe("first line\nsecond line\n") + expect(contents.endsWith("\n")).toBe(false) + expect(contents).toBe("first line\nsecond line") + }), + ) + + it.instance("keeps trailing newline when original file has one", () => + Effect.gen(function* () { + const test = yield* TestInstance + const { ctx } = makeCtx() + const target = path.join(test.directory, "with_newline.txt") + yield* writeText(target, "line at end\n") + + const patchText = + "*** Begin Patch\n*** Update File: with_newline.txt\n@@\n-line at end\n+updated line\n*** End Patch" + + yield* execute({ patchText }, ctx) + + expect(yield* readText(target)).toBe("updated line\n") }), ) @@ -312,7 +328,19 @@ describe("tool.apply_patch freeform", () => { const patchText = "*** Begin Patch\n*** Add File: duplicate.txt\n+new content\n*** End Patch" yield* execute({ patchText }, ctx) - expect(yield* readText(target)).toBe("new content\n") + expect(yield* readText(target)).toBe("new content") + }), + ) + + it.instance("adds file with explicit trailing newline", () => + Effect.gen(function* () { + const test = yield* TestInstance + const { ctx } = makeCtx() + + const patchText = "*** Begin Patch\n*** Add File: explicit_newline.txt\n+new content\n+\n*** End Patch" + + yield* execute({ patchText }, ctx) + expect(yield* readText(path.join(test.directory, "explicit_newline.txt"))).toBe("new content\n") }), ) @@ -457,7 +485,7 @@ describe("tool.apply_patch freeform", () => { EOF` yield* execute({ patchText }, ctx) - expect(yield* readText(path.join(test.directory, "heredoc_test.txt"))).toBe("heredoc content\n") + expect(yield* readText(path.join(test.directory, "heredoc_test.txt"))).toBe("heredoc content") }), ) @@ -473,7 +501,7 @@ EOF` EOF` yield* execute({ patchText }, ctx) - expect(yield* readText(path.join(test.directory, "heredoc_no_cat.txt"))).toBe("no cat prefix\n") + expect(yield* readText(path.join(test.directory, "heredoc_no_cat.txt"))).toBe("no cat prefix") }), )