Skip to content

refactor: simplify apply_diff XML schema #6812

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

refactor: simplify apply_diff XML schema #6812

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 12 additions & 44 deletions src/core/diff/strategies/multi-file-search-replace.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,42 +105,31 @@ When applying the diffs, be extra careful to remember to change any closing brac
ALWAYS make as many changes in a single 'apply_diff' request as possible using multiple SEARCH/REPLACE blocks

Parameters:
- args: Contains one or more file elements, where each file contains:
- file: One or more file elements, where each file contains:
- path: (required) The path of the file to modify (relative to the current workspace directory ${args.cwd})
- diff: (required) One or more diff elements containing:
- content: (required) The search/replace block defining the changes.
- start_line: (required) The line number of original content where the search block starts.
- diff: (required) One or more diff elements containing the search/replace blocks directly (no content wrapper needed)

Diff format:
\`\`\`
<<<<<<< SEARCH
:start_line: (required) The line number of original content where the search block starts.
-------
[exact content to find including whitespace]
=======
[new content to replace with]
>>>>>>> REPLACE
\`\`\`

Example:

Original file:
\`\`\`
1 | def calculate_total(items):
2 | total = 0
3 | for item in items:
4 | total += item
5 | return total
\`\`\`

Search/Replace content:
<apply_diff>
<args>
<file>
<path>eg.file.py</path>
<diff>
<content>
\`\`\`
<<<<<<< SEARCH
def calculate_total(items):
total = 0
Expand All @@ -152,91 +141,70 @@ def calculate_total(items):
"""Calculate total with 10% markup"""
return sum(item * 1.1 for item in items)
>>>>>>> REPLACE
\`\`\`
</content>
</diff>
</file>
</args>
</apply_diff>

Search/Replace content with multi edits across multiple files:
<apply_diff>
<args>
<file>
<path>eg.file.py</path>
<diff>
<content>
\`\`\`
<<<<<<< SEARCH
def calculate_total(items):
sum = 0
=======
def calculate_sum(items):
sum = 0
>>>>>>> REPLACE
\`\`\`
</content>
</diff>
<diff>
<content>
\`\`\`
<<<<<<< SEARCH
total += item
return total
=======
sum += item
return sum
>>>>>>> REPLACE
\`\`\`
</content>
</diff>
</file>
<file>
<path>eg.file2.py</path>
<diff>
<content>
\`\`\`
<<<<<<< SEARCH
def greet(name):
return "Hello " + name
=======
def greet(name):
return f"Hello {name}!"
>>>>>>> REPLACE
\`\`\`
</content>
</diff>
</file>
</args>
</apply_diff>


Usage:
<apply_diff>
<args>
<file>
<path>File path here</path>
<diff>
<content>
Your search/replace content here
You can use multi search/replace block in one diff block, but make sure to include the line numbers for each block.
Only use a single line of '=======' between search and replacement content, because multiple '=======' will corrupt the file.
</content>
<start_line>1</start_line>
<<<<<<< SEARCH
Your exact search content here
=======
Your replacement content here
>>>>>>> REPLACE
</diff>
</file>
<file>
<path>Another file path</path>
<diff>
<content>
Another search/replace content here
You can apply changes to multiple files in a single request.
Each file requires its own path, start_line, and diff elements.
</content>
<start_line>5</start_line>
<<<<<<< SEARCH
Another search content here
=======
Another replacement content here
>>>>>>> REPLACE
</diff>
</file>
</args>
</apply_diff>`
}

Expand Down
41 changes: 27 additions & 14 deletions src/core/tools/multiApplyDiffTool.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,9 +106,10 @@ export async function applyDiffTool(
}

if (argsXmlTag) {
// Parse file entries from XML (new way)
// Parse file entries from XML
try {
const parsed = parseXml(argsXmlTag, ["file.diff.content"]) as ParsedXmlResult
// Try parsing with simplified schema (no content wrapper)
const parsed = parseXml(argsXmlTag, ["file.diff"]) as ParsedXmlResult
const files = Array.isArray(parsed.file) ? parsed.file : [parsed.file].filter(Boolean)

for (const file of files) {
Expand All @@ -132,8 +133,19 @@ export async function applyDiffTool(
let diffContent: string
let startLine: number | undefined

diffContent = diff.content
startLine = diff.start_line ? parseInt(diff.start_line) : undefined
// For simplified format, diff is the content directly
// For legacy format, diff has content and start_line properties
if (typeof diff === "object" && diff.content) {
// Legacy format with content wrapper
diffContent = diff.content
Copy link
Contributor

@roomote roomote bot Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this maintains backward compatibility but changes the recommended format, would it be helpful to add a deprecation notice when the legacy format is detected? This could guide users to migrate to the cleaner schema over time. For example:

Suggested change
diffContent = diff.content
// Legacy format with content wrapper
console.warn('[Deprecation] The <content> wrapper in apply_diff is deprecated. Please use the simplified format with diff content directly.');
diffContent = diff.content

startLine = diff.start_line ? parseInt(diff.start_line) : undefined
} else {
// New simplified format - diff is the content string directly
diffContent = String(diff)
Copy link
Contributor

@roomote roomote bot Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider adding a type guard or validation here to ensure diff is actually a string in the simplified format case. While String(diff) works, explicit type checking would make the code more robust:

Suggested change
diffContent = String(diff)
// New simplified format - diff is the content string directly
if (typeof diff !== 'string') {
throw new Error('Invalid diff format: expected string for simplified format');
}
diffContent = diff;

// Extract start_line from the diff content if present
const startLineMatch = diffContent.match(/:start_line:\s*(\d+)/)
startLine = startLineMatch ? parseInt(startLineMatch[1]) : undefined
Copy link
Contributor

@roomote roomote bot Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this intentional? The code extracts :start_line: from the diff content here, but the documentation in multi-file-search-replace.ts no longer mentions this format. Should we update the documentation to clarify that :start_line: can still be used within the diff content for cases where it's needed?

}

operationsMap[filePath].diff.push({
content: diffContent,
Expand All @@ -148,16 +160,17 @@ export async function applyDiffTool(
2. Missing required <file>, <path>, or <diff> tags
3. Invalid characters or encoding in the XML

Expected structure:
<args>
<file>
<path>relative/path/to/file.ext</path>
<diff>
<content>diff content here</content>
<start_line>line number</start_line>
</diff>
</file>
</args>
Expected structure (simplified):
Copy link
Contributor

@roomote roomote bot Aug 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message shows the simplified structure, which is great! However, would it be helpful to also mention that the legacy format is still supported for backward compatibility? This could help users who are migrating from the old format.

<file>
<path>relative/path/to/file.ext</path>
<diff>
<<<<<<< SEARCH
exact content to find
=======
replacement content
>>>>>>> REPLACE
</diff>
</file>

Original error: ${errorMessage}`
cline.consecutiveMistakeCount++
Expand Down
Loading