-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix: preserve C# generics and angle brackets in write_to_file tool #6771
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.
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
Conversation
…C# generics - Removed unescapeHtmlEntities call that was converting angle brackets in C# generic types - This fixes issue #6770 where C# code like Configure<BlazorServerSettings> was being incorrectly converted to Configure<BlazorServerSettings> - Updated tests to verify C# generics and angle brackets are preserved correctly
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewing my own code is like debugging in a mirror - everything looks backwards but the bugs are still mine.
| newContent = unescapeHtmlEntities(newContent) | ||
| } | ||
|
|
||
| // Determine if the path is outside the workspace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix removing the HTML entity unescaping. However, I noticed that applyDiffTool.ts (line 29) and multiApplyDiffTool.ts (line 418) still use unescapeHtmlEntities() for non-Claude models. Should we remove it from those tools as well for consistency? This could affect users working with C# or other languages with angle brackets across different tools.
|
|
||
| it("skips HTML unescaping for Claude models", async () => { | ||
| mockCline.api.getModel.mockReturnValue({ id: "claude-3" }) | ||
| it("preserves HTML entities in content without unescaping", async () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These test cases for C# generics and HTML entities look good. Consider adding a test case for mixed content (e.g., actual HTML with entities alongside C# code) to ensure we're not breaking legitimate HTML entity usage in edge cases?
| } | ||
|
|
||
| // Determine if the path is outside the workspace | ||
| const fullPath = relPath ? path.resolve(cline.cwd, removeClosingTag("path", relPath)) : "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a comment here explaining why we don't unescape HTML entities (to preserve code exactly as written, especially for languages like C# with generics). This would help prevent future developers from re-introducing this logic thinking it was an oversight.
|
Closing, see discussion on #4077 |
Summary
This PR fixes issue #6770 where C# code with generic types was being incorrectly modified when using the
write_to_filetool with non-Claude models.Problem
The
write_to_filetool was callingunescapeHtmlEntities()on content for non-Claude models, which was converting angle brackets in C# generic types (likeConfigure<BlazorServerSettings>) into HTML entities (Configure<BlazorServerSettings>).Solution
unescapeHtmlEntities()call fromwriteToFileTool.tsthat was being applied to non-Claude modelsChanges
src/core/tools/writeToFileTool.tssrc/core/tools/__tests__/writeToFileTool.spec.tsto verify C# generics are preserved correctlyTesting
Fixes #6770
Important
Fixes issue with
write_to_filetool by removing HTML entity conversion to preserve C# generics and angle brackets.unescapeHtmlEntities()call fromwriteToFileTool.tsto preserve C# generics and angle brackets.writeToFileTool.spec.tsto include tests for C# generics and HTML entity preservation.unescapeHtmlEntitiesinwriteToFileTool.spec.ts.This description was created by
for fe9811d. You can customize this summary. It will automatically update as commits are pushed.