Improvements to apply_diff (#52)

Author: mrubens

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Roo Cline Changelog
 
+## [2.1.14]
+
+- Fix bug where diffs were not being applied correctly and try Aider's [unified diff prompt](https://github.com/Aider-AI/aider/blob/3995accd0ca71cea90ef76d516837f8c2731b9fe/aider/coders/udiff_prompts.py#L75-L105)
+- If diffs are enabled, automatically reject write_to_file commands that lead to truncated output
+
 ## [2.1.13]
 
 - Fix https://github.com/RooVetGit/Roo-Cline/issues/50 where sound effects were not respecting settings
@@ -47,4 +52,4 @@
 ## [2.1.2]
 
 - Support for auto-approval of write operations and command execution
-- Support for .clinerules custom instructions
+- Support for .clinerules custom instructions

package-lock.json

@@ -3,7 +3,7 @@
   "displayName": "Roo Cline",
   "description": "A fork of Cline, an autonomous coding agent, with some added experimental configuration and automation features.",
   "publisher": "RooVeterinaryInc",
-  "version": "2.1.13",
+  "version": "2.1.14",
   "icon": "assets/icons/rocket.png",
   "galleryBanner": {
     "color": "#617A91",

package.json

@@ -12,7 +12,7 @@ import { ApiHandler, buildApiHandler } from "../api"
 import { ApiStream } from "../api/transform/stream"
 import { DiffViewProvider } from "../integrations/editor/DiffViewProvider"
 import { findToolName, formatContentBlockToMarkdown } from "../integrations/misc/export-markdown"
-import { extractTextFromFile } from "../integrations/misc/extract-text"
+import { extractTextFromFile, addLineNumbers } from "../integrations/misc/extract-text"
 import { TerminalManager } from "../integrations/terminal/TerminalManager"
 import { UrlContentFetcher } from "../services/browser/UrlContentFetcher"
 import { listFiles } from "../services/glob/list-files"
@@ -46,7 +46,7 @@ import { formatResponse } from "./prompts/responses"
 import { addCustomInstructions, SYSTEM_PROMPT } from "./prompts/system"
 import { truncateHalfConversation } from "./sliding-window"
 import { ClineProvider, GlobalFileNames } from "./webview/ClineProvider"
-import { showOmissionWarning } from "../integrations/editor/detect-omission"
+import { detectCodeOmission } from "../integrations/editor/detect-omission"
 import { BrowserSession } from "../services/browser/BrowserSession"
 
 const cwd =
@@ -1101,7 +1101,32 @@ export class Cline {
 								await this.diffViewProvider.update(newContent, true)
 								await delay(300) // wait for diff view to update
 								this.diffViewProvider.scrollToFirstDiff()
-								showOmissionWarning(this.diffViewProvider.originalContent || "", newContent)
+
+								// Check for code omissions before proceeding
+								if (detectCodeOmission(this.diffViewProvider.originalContent || "", newContent)) {
+									if (this.diffEnabled) {
+										await this.diffViewProvider.revertChanges()
+										pushToolResult(formatResponse.toolError(
+											"Content appears to be truncated. Found comments indicating omitted code (e.g., '// rest of code unchanged', '/* previous code */'). Please provide the complete file content without any omissions if possible, or otherwise use the 'apply_diff' tool to apply the diff to the original file."
+										))
+										break
+									} else {
+										vscode.window
+											.showWarningMessage(
+												"Potential code truncation detected. This happens when the AI reaches its max output limit.",
+												"Follow this guide to fix the issue",
+											)
+											.then((selection) => {
+												if (selection === "Follow this guide to fix the issue") {
+													vscode.env.openExternal(
+														vscode.Uri.parse(
+															"https://github.com/cline/cline/wiki/Troubleshooting-%E2%80%90-Cline-Deleting-Code-with-%22Rest-of-Code-Here%22-Comments",
+														),
+													)
+												}
+											})
+									}
+								}
 
 								const completeMessage = JSON.stringify({
 									...sharedMessageProps,
@@ -1133,8 +1158,8 @@ export class Cline {
 									)
 									pushToolResult(
 										`The user made the following updates to your content:\n\n${userEdits}\n\n` +
-											`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file:\n\n` +
-											`<final_file_content path="${relPath.toPosix()}">\n${finalContent}\n</final_file_content>\n\n` +
+											`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
+											`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(finalContent || '')}\n</final_file_content>\n\n` +
 											`Please note:\n` +
 											`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
 											`2. Proceed with the task using this updated file content as the new baseline.\n` +
@@ -1231,11 +1256,32 @@ export class Cline {
 									break
 								}
 
-								await fs.writeFile(absolutePath, newContent)
-								await vscode.window.showTextDocument(vscode.Uri.file(absolutePath), { preview: false })
+								const { newProblemsMessage, userEdits, finalContent } =
+									await this.diffViewProvider.saveChanges()
+								this.didEditFile = true // used to determine if we should wait for busy terminal to update before sending api request
+								if (userEdits) {
+									await this.say(
+										"user_feedback_diff",
+										JSON.stringify({
+											tool: fileExists ? "editedExistingFile" : "newFileCreated",
+											path: getReadablePath(cwd, relPath),
+											diff: userEdits,
+										} satisfies ClineSayTool),
+									)
+									pushToolResult(
+										`The user made the following updates to your content:\n\n${userEdits}\n\n` +
+											`The updated content, which includes both your original modifications and the user's edits, has been successfully saved to ${relPath.toPosix()}. Here is the full, updated content of the file, including line numbers:\n\n` +
+											`<final_file_content path="${relPath.toPosix()}">\n${addLineNumbers(finalContent || '')}\n</final_file_content>\n\n` +
+											`Please note:\n` +
+											`1. You do not need to re-write the file with these changes, as they have already been applied.\n` +
+											`2. Proceed with the task using this updated file content as the new baseline.\n` +
+											`3. If the user's edits have addressed part of the task or changed the requirements, adjust your approach accordingly.` +
+											`${newProblemsMessage}`,
+									)
+								} else {
+									pushToolResult(`Changes successfully applied to ${relPath.toPosix()}:\n\n${newProblemsMessage}`)
+								}
 								await this.diffViewProvider.reset()
-
-								pushToolResult(`Changes successfully applied to ${relPath.toPosix()}:\n\n${diffRepresentation}`)
 								break
 							}
 						} catch (error) {
@@ -2242,3 +2288,4 @@ export class Cline {
 		return `<environment_details>\n${details.trim()}\n</environment_details>`
 	}
 }
+

src/core/Cline.ts

@@ -46,7 +46,7 @@ Usage:
 </execute_command>
 
 ## read_file
-Description: Request to read the contents of a file at the specified path. Use this when you need to examine the contents of an existing file you do not know the contents of, for example to analyze code, review text files, or extract information from configuration files. Automatically extracts raw text from PDF and DOCX files. May not be suitable for other types of binary files, as it returns the raw content as a string.
+Description: Request to read the contents of a file at the specified path. Use this when you need to examine the contents of an existing file you do not know the contents of, for example to analyze code, review text files, or extract information from configuration files. The output includes line numbers prefixed to each line (e.g. "1 | const x = 1"), making it easier to reference specific lines when creating diffs or discussing code. Automatically extracts raw text from PDF and DOCX files. May not be suitable for other types of binary files, as it returns the raw content as a string.
 Parameters:
 - path: (required) The path of the file to read (relative to the current working directory ${cwd.toPosix()})
 Usage:
@@ -69,10 +69,66 @@ Your file content here
 
 ${diffEnabled ? `
 ## apply_diff
-Description: Apply a diff to a file at the specified path. The diff should be in unified format (diff -u) and can be used to apply changes to a file. This tool is useful when you need to make specific modifications to a file based on a set of changes provided in a diff.
+Description: Apply a diff to a file at the specified path. The diff should be in unified format ('diff -U0') and can be used to apply changes to a file. This tool is useful when you need to make specific modifications to a file based on a set of changes provided in a diff.
+
+Diff Format Requirements:
+
+1. Header (REQUIRED):
+   \`\`\`
+   --- path/to/original/file
+   +++ path/to/modified/file
+   \`\`\`
+   - Must include both lines exactly as shown
+   - Use actual file paths
+   - NO timestamps after paths
+
+2. Hunks:
+   \`\`\`
+   @@ -lineStart,lineCount +lineStart,lineCount @@
+   -removed line
+   +added line
+   \`\`\`
+   - Each hunk starts with @@ showing line numbers for changes
+   - Format: @@ -originalStart,originalCount +newStart,newCount @@
+   - Use - for removed/changed lines
+   - Use + for new/modified lines
+   - Indentation must match exactly
+
+Complete Example:
+\`\`\`
+--- src/utils/helper.ts
++++ src/utils/helper.ts
+@@ -10,3 +10,4 @@
+-function oldFunction(x: number): number {
+-  return x + 1;
+-}
++function newFunction(x: number): number {
++  const result = x + 2;
++  return result;
++}
+\`\`\`
+
+Common Pitfalls:
+1. Missing or incorrect header lines
+2. Incorrect line numbers in @@ lines
+3. Wrong indentation in changed lines
+4. Incomplete context (missing lines that need changing)
+5. Not marking all modified lines with - and +
+
+Best Practices:
+1. Replace entire code blocks:
+   - Remove complete old version with - lines
+   - Add complete new version with + lines
+   - Include correct line numbers
+2. Moving code requires two hunks:
+   - First hunk: Remove from old location
+   - Second hunk: Add to new location
+3. One hunk per logical change
+4. Verify line numbers match the file
+
 Parameters:
 - path: (required) The path of the file to apply the diff to (relative to the current working directory ${cwd.toPosix()})
-- diff: (required) The diff in unified format (diff -u) to apply to the file.
+- diff: (required) The diff in unified format (diff -U0) to apply to the file.
 Usage:
 <apply_diff>
 <path>File path here</path>
@@ -342,3 +398,4 @@ The following additional instructions are provided by the user, and should be fo
 ${joinedInstructions}`
 		: ""
 }
+

src/core/prompts/system.ts

@@ -76,6 +76,15 @@ jest.mock('../../Cline', () => {
     }
 })
 
+// Mock extract-text
+jest.mock('../../../integrations/misc/extract-text', () => ({
+    extractTextFromFile: jest.fn().mockImplementation(async (filePath: string) => {
+        const content = 'const x = 1;\nconst y = 2;\nconst z = 3;'
+        const lines = content.split('\n')
+        return lines.map((line, index) => `${index + 1} | ${line}`).join('\n')
+    })
+}))
+
 // Spy on console.error and console.log to suppress expected messages
 beforeAll(() => {
     jest.spyOn(console, 'error').mockImplementation(() => {})
@@ -258,4 +267,10 @@ describe('ClineProvider', () => {
         expect(mockContext.globalState.update).toHaveBeenCalledWith('soundEnabled', false)
         expect(mockPostMessage).toHaveBeenCalled()
     })
+
+    test('file content includes line numbers', async () => {
+        const { extractTextFromFile } = require('../../../integrations/misc/extract-text')
+        const result = await extractTextFromFile('test.js')
+        expect(result).toBe('1 | const x = 1;\n2 | const y = 2;\n3 | const z = 3;')
+    })
 })

src/core/webview/__tests__/ClineProvider.test.ts

@@ -1,12 +1,10 @@
-import * as vscode from "vscode"
-
 /**
  * Detects potential AI-generated code omissions in the given file content.
  * @param originalFileContent The original content of the file.
  * @param newFileContent The new content of the file to check.
  * @returns True if a potential omission is detected, false otherwise.
  */
-function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean {
+export function detectCodeOmission(originalFileContent: string, newFileContent: string): boolean {
 	const originalLines = originalFileContent.split("\n")
 	const newLines = newFileContent.split("\n")
 	const omissionKeywords = ["remain", "remains", "unchanged", "rest", "previous", "existing", "..."]
@@ -33,26 +31,3 @@ function detectCodeOmission(originalFileContent: string, newFileContent: string)
 	return false
 }
 
-/**
- * Shows a warning in VSCode if a potential code omission is detected.
- * @param originalFileContent The original content of the file.
- * @param newFileContent The new content of the file to check.
- */
-export function showOmissionWarning(originalFileContent: string, newFileContent: string): void {
-	if (detectCodeOmission(originalFileContent, newFileContent)) {
-		vscode.window
-			.showWarningMessage(
-				"Potential code truncation detected. This happens when the AI reaches its max output limit.",
-				"Follow this guide to fix the issue",
-			)
-			.then((selection) => {
-				if (selection === "Follow this guide to fix the issue") {
-					vscode.env.openExternal(
-						vscode.Uri.parse(
-							"https://github.com/cline/cline/wiki/Troubleshooting-%E2%80%90-Cline-Deleting-Code-with-%22Rest-of-Code-Here%22-Comments",
-						),
-					)
-				}
-			})
-	}
-}

src/integrations/editor/detect-omission.ts

@@ -22,7 +22,7 @@ export async function extractTextFromFile(filePath: string): Promise<string> {
 		default:
 			const isBinary = await isBinaryFile(filePath).catch(() => false)
 			if (!isBinary) {
-				return await fs.readFile(filePath, "utf8")
+				return addLineNumbers(await fs.readFile(filePath, "utf8"))
 			} else {
 				throw new Error(`Cannot read text for file type: ${fileExtension}`)
 			}
@@ -32,12 +32,12 @@ export async function extractTextFromFile(filePath: string): Promise<string> {
 async function extractTextFromPDF(filePath: string): Promise<string> {
 	const dataBuffer = await fs.readFile(filePath)
 	const data = await pdf(dataBuffer)
-	return data.text
+	return addLineNumbers(data.text)
 }
 
 async function extractTextFromDOCX(filePath: string): Promise<string> {
 	const result = await mammoth.extractRawText({ path: filePath })
-	return result.value
+	return addLineNumbers(result.value)
 }
 
 async function extractTextFromIPYNB(filePath: string): Promise<string> {
@@ -51,5 +51,17 @@ async function extractTextFromIPYNB(filePath: string): Promise<string> {
 		}
 	}
 
-	return extractedText
+	return addLineNumbers(extractedText)
 }
+
+export function addLineNumbers(content: string): string {
+	const lines = content.split('\n')
+	const maxLineNumberWidth = String(lines.length).length
+	return lines
+		.map((line, index) => {
+			const lineNumber = String(index + 1).padStart(maxLineNumberWidth, ' ')
+			return `${lineNumber} | ${line}`
+		}).join('\n')
+}
+
+

src/integrations/misc/extract-text.ts

@@ -20,7 +20,7 @@ export const isWAV = (filepath: string): boolean => {
 	return path.extname(filepath).toLowerCase() === ".wav"
 }
 
-let isSoundEnabled = true
+let isSoundEnabled = false
 
 /**
  * Set sound configuration

src/utils/sound.ts

@@ -301,7 +301,7 @@ const SettingsView = ({ onDone }: SettingsViewProps) => {
 							marginTop: "5px",
 							color: "var(--vscode-descriptionForeground)",
 						}}>
-						When enabled, Cline will be able to apply diffs to make changes to files.
+						When enabled, Cline will be able to apply diffs to make changes to files and will automatically reject truncated full-file edits.
 					</p>
 				</div>