Skip to content

Commit ca605df

Browse files
Githubguy132010daniel-lxs
authored andcommitted
Fix: Allow write_to_file to handle newline-only and empty content (#3550)
* Fix: Allow write_to_file to handle newline-only and empty content * fix: update writeToFileTool to return early without error on missing or empty parameters * fix: preserve newlines in content parameters and update error handling in writeToFileTool tests * fix: update parseAssistantMessage and parseAssistantMessageV2 to preserve newlines in content parameters while stripping leading and trailing newlines --------- Co-authored-by: Daniel Riccio <[email protected]>
1 parent 6c52d91 commit ca605df

File tree

4 files changed

+34
-22
lines changed

4 files changed

+34
-22
lines changed

src/core/assistant-message/parseAssistantMessage.ts

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
2424
const paramClosingTag = `</${currentParamName}>`
2525
if (currentParamValue.endsWith(paramClosingTag)) {
2626
// End of param value.
27-
currentToolUse.params[currentParamName] = currentParamValue.slice(0, -paramClosingTag.length).trim()
27+
// Don't trim content parameters to preserve newlines, but strip first and last newline only
28+
const paramValue = currentParamValue.slice(0, -paramClosingTag.length)
29+
currentToolUse.params[currentParamName] =
30+
currentParamName === "content"
31+
? paramValue.replace(/^\n/, "").replace(/\n$/, "")
32+
: paramValue.trim()
2833
currentParamName = undefined
2934
continue
3035
} else {
@@ -72,9 +77,11 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
7277
const contentEndIndex = toolContent.lastIndexOf(contentEndTag)
7378

7479
if (contentStartIndex !== -1 && contentEndIndex !== -1 && contentEndIndex > contentStartIndex) {
80+
// Don't trim content to preserve newlines, but strip first and last newline only
7581
currentToolUse.params[contentParamName] = toolContent
7682
.slice(contentStartIndex, contentEndIndex)
77-
.trim()
83+
.replace(/^\n/, "")
84+
.replace(/\n$/, "")
7885
}
7986
}
8087

@@ -138,7 +145,10 @@ export function parseAssistantMessage(assistantMessage: string): AssistantMessag
138145
// Stream did not complete tool call, add it as partial.
139146
if (currentParamName) {
140147
// Tool call has a parameter that was not completed.
141-
currentToolUse.params[currentParamName] = accumulator.slice(currentParamValueStartIndex).trim()
148+
// Don't trim content parameters to preserve newlines, but strip first and last newline only
149+
const paramValue = accumulator.slice(currentParamValueStartIndex)
150+
currentToolUse.params[currentParamName] =
151+
currentParamName === "content" ? paramValue.replace(/^\n/, "").replace(/\n$/, "") : paramValue.trim()
142152
}
143153

144154
contentBlocks.push(currentToolUse)

src/core/assistant-message/parseAssistantMessageV2.ts

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,13 +76,13 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
7676
)
7777
) {
7878
// Found the closing tag for the parameter.
79-
const value = assistantMessage
80-
.slice(
81-
currentParamValueStart, // Start after the opening tag.
82-
currentCharIndex - closeTag.length + 1, // End before the closing tag.
83-
)
84-
.trim()
85-
currentToolUse.params[currentParamName] = value
79+
const value = assistantMessage.slice(
80+
currentParamValueStart, // Start after the opening tag.
81+
currentCharIndex - closeTag.length + 1, // End before the closing tag.
82+
)
83+
// Don't trim content parameters to preserve newlines, but strip first and last newline only
84+
currentToolUse.params[currentParamName] =
85+
currentParamName === "content" ? value.replace(/^\n/, "").replace(/\n$/, "") : value.trim()
8686
currentParamName = undefined // Go back to parsing tool content.
8787
// We don't continue loop here, need to check for tool close or other params at index i.
8888
} else {
@@ -146,10 +146,11 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
146146
const contentEnd = toolContentSlice.lastIndexOf(contentEndTag)
147147

148148
if (contentStart !== -1 && contentEnd !== -1 && contentEnd > contentStart) {
149+
// Don't trim content to preserve newlines, but strip first and last newline only
149150
const contentValue = toolContentSlice
150151
.slice(contentStart + contentStartTag.length, contentEnd)
151-
.trim()
152-
152+
.replace(/^\n/, "")
153+
.replace(/\n$/, "")
153154
currentToolUse.params[contentParamName] = contentValue
154155
}
155156
}
@@ -251,9 +252,10 @@ export function parseAssistantMessageV2(assistantMessage: string): AssistantMess
251252

252253
// Finalize any open parameter within an open tool use.
253254
if (currentToolUse && currentParamName) {
254-
currentToolUse.params[currentParamName] = assistantMessage
255-
.slice(currentParamValueStart) // From param start to end of string.
256-
.trim()
255+
const value = assistantMessage.slice(currentParamValueStart) // From param start to end of string.
256+
// Don't trim content parameters to preserve newlines, but strip first and last newline only
257+
currentToolUse.params[currentParamName] =
258+
currentParamName === "content" ? value.replace(/^\n/, "").replace(/\n$/, "") : value.trim()
257259
// Tool use remains partial.
258260
}
259261

src/core/tools/writeToFileTool.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,11 +73,11 @@ export async function writeToFileTool(
7373
// pre-processing newContent for cases where weaker models might add artifacts like markdown codeblock markers (deepseek/llama) or extra escape characters (gemini)
7474
if (newContent.startsWith("```")) {
7575
// cline handles cases where it includes language specifiers like ```python ```js
76-
newContent = newContent.split("\n").slice(1).join("\n").trim()
76+
newContent = newContent.split("\n").slice(1).join("\n")
7777
}
7878

7979
if (newContent.endsWith("```")) {
80-
newContent = newContent.split("\n").slice(0, -1).join("\n").trim()
80+
newContent = newContent.split("\n").slice(0, -1).join("\n")
8181
}
8282

8383
if (!cline.api.getModel().id.includes("claude")) {

src/integrations/editor/DiffViewProvider.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,8 @@ export class DiffViewProvider {
130130
// Replace all content up to the current line with accumulated lines.
131131
const edit = new vscode.WorkspaceEdit()
132132
const rangeToReplace = new vscode.Range(0, 0, endLine, 0)
133-
const contentToReplace = accumulatedLines.slice(0, endLine + 1).join("\n") + "\n"
133+
const contentToReplace =
134+
accumulatedLines.slice(0, endLine).join("\n") + (accumulatedLines.length > 0 ? "\n" : "")
134135
edit.replace(document.uri, rangeToReplace, this.stripAllBOMs(contentToReplace))
135136
await vscode.workspace.applyEdit(edit)
136137
// Update decorations.
@@ -230,12 +231,11 @@ export class DiffViewProvider {
230231
// show a diff with all the EOL differences.
231232
const newContentEOL = this.newContent.includes("\r\n") ? "\r\n" : "\n"
232233

233-
// `trimEnd` to fix issue where editor adds in extra new line
234-
// automatically.
235-
const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL
234+
// Normalize EOL characters without trimming content
235+
const normalizedEditedContent = editedContent.replace(/\r\n|\n/g, newContentEOL)
236236

237237
// Just in case the new content has a mix of varying EOL characters.
238-
const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL).trimEnd() + newContentEOL
238+
const normalizedNewContent = this.newContent.replace(/\r\n|\n/g, newContentEOL)
239239

240240
if (normalizedEditedContent !== normalizedNewContent) {
241241
// User made changes before approving edit.

0 commit comments

Comments
 (0)