fix: preserve HTML entities in diff/search/replace operations #8071
Conversation
- Remove HTML entity unescaping from applyDiffTool.ts - Remove HTML entity unescaping from multiApplyDiffTool.ts - Remove HTML entity unescaping from writeToFileTool.ts - Update tests to verify HTML entities are preserved - Add comprehensive test suite for HTML entity preservation This ensures that encoded characters like &, <, and > are preserved exactly as written during diff and file operations, preventing unintended modifications to code and markup. Fixes #8069
dosubot
bot
added
size:M
hannesrudolph
added
the
Issue/PR - Triage
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| if (diffContent && !cline.api.getModel().id.includes("claude")) { | ||
| diffContent = unescapeHtmlEntities(diffContent) | ||
| } | ||
| // HTML entities should be preserved exactly as provided |
There was a problem hiding this comment.
Good change to preserve HTML entities. However, I notice that executeCommandTool.ts still uses unescapeHtmlEntities. Is this intentional? Shell commands might need actual characters like < and > for redirection, but it would be good to document why command execution needs different handling than file operations.
| }) | ||
|
|
||
| it("unescapes HTML entities for non-Claude models", async () => { | ||
| it("preserves HTML entities for all models", async () => { |
There was a problem hiding this comment.
Could we add a test case that specifically reproduces the original issue from #8069? The current test verifies that entities are preserved, but it would be helpful to have a test that demonstrates the actual bug scenario (e.g., applying a diff with < that was incorrectly becoming <).
| content: item.content ? unescapeHtmlEntities(item.content) : item.content, | ||
| })) | ||
| : diffItems | ||
| // HTML entities should be preserved exactly as provided |
There was a problem hiding this comment.
The comment style here is slightly different from the one in applyDiffTool.ts (2 lines vs 3 lines). Consider using consistent formatting across files for better readability.
| if (!cline.api.getModel().id.includes("claude")) { | ||
| newContent = unescapeHtmlEntities(newContent) | ||
| } | ||
| // HTML entities should be preserved exactly as provided |
There was a problem hiding this comment.
Good to see HTML entities being preserved here as well. This ensures consistency across all file write operations.
Summary
This PR attempts to address Issue #8069 by removing HTML entity unescaping from diff and file operation tools. Feedback and guidance are welcome.
Problem
When applying changes, encoded characters like
&,<, and>were being unescaped during diff/search/replace operations, leading to unintended edits and broken code or markup.Solution
unescapeHtmlEntitiescalls fromapplyDiffTool.tsunescapeHtmlEntitiescalls frommultiApplyDiffTool.tsunescapeHtmlEntitiescalls fromwriteToFileTool.tsChanges
Testing
Impact
This ensures that encoded characters are preserved exactly as written during diff and file operations, preventing unintended modifications to code and markup.
Fixes #8069
Important
Preserve HTML entities in diff, search, and replace operations by removing unescaping calls from key tools and updating tests.
unescapeHtmlEntitiescalls fromapplyDiffTool.ts,multiApplyDiffTool.ts, andwriteToFileTool.tsto preserve HTML entities during operations.writeToFileTool.spec.tsto verify HTML entities are preserved.This description was created by
for 662c40b. You can customize this summary. It will automatically update as commits are pushed.