edit_file tool #184
edit_file tool #184
Conversation
| if (edit.dryRun) { | ||
| // Create git-style diff preview | ||
| const preview = [ | ||
| `@@ line ${lineNumber} @@`, | ||
| '<<<<<<< ORIGINAL', | ||
| edit.oldText, | ||
| '=======', | ||
| edit.newText, | ||
| '>>>>>>> MODIFIED' | ||
| ].join('\n'); | ||
|
|
||
| previews.push({ | ||
| original: edit.oldText, | ||
| modified: edit.newText, | ||
| lineNumber, | ||
| preview | ||
| }); | ||
| continue; | ||
| } | ||
|
|
||
| // Apply the edit | ||
| content = content.slice(0, pos) + edit.newText + content.slice(pos + edit.oldText.length); | ||
| } | ||
|
|
||
| if (edits.some(e => e.dryRun)) { | ||
| return previews; | ||
| } |
There was a problem hiding this comment.
Since dryRun seems changes the logic and the return type, can we create two different functions instead? We can reuse common bits between them instead of if/else-ing most of the logic in one function.
There was a problem hiding this comment.
Looks like this is still a good suggestion after the refactoring I did last night. I can take a look at this later as I'll need to rerun all of my tests after making this change.
|
|
||
| const EditFileArgsSchema = z.object({ | ||
| path: z.string(), | ||
| edits: z.array(EditOperation), |
There was a problem hiding this comment.
IMO dryRun makes more sense as an argument here, rather than per-operation.
There was a problem hiding this comment.
I'll take a look at this while working on the other dry run suggestion, thanks.
|
@jspahrsummers thank you for the review and encouragement. I made some significant changes to the PR based on some testing last night. I have changed it to using a git style diff and substring matching informed by how Aider handles things. This made things a lot simpler and works a lot better with less special handling. I used Claude to generate a bunch of test documents of likely file types this would be used with described in TEST_DOCUMENTS.md, you can regenerate them using that document and the file system tool. TEST_RESULTS.md records findings and it worked really well, the edit_file tool was used for editing that document while testing other than the initial creation. I've recorded the limitations there and added them to the read me, but I don't think they are significant concerns for the reasons stated. (boils down to there may be issues with mixed tabs/spaces) |
Suggested implementation for an edit_file per #183
Description
Adds a edit_file tool to filesystem.
Server Details
Motivation and Context
Described in the issue, but write_file really isn't trustworthy for existing files.
How Has This Been Tested?
Claude Desktop, not a ton of interactions with it yet. I tried it on some code in a git repo on Windows which surfaced some line ending issues it can handle now.
Breaking Changes
No.
Types of changes
Checklist
Additional context