-
Notifications
You must be signed in to change notification settings - Fork 1.4k
edits: show diffs for files being approved during edits #1905
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
Requires a PR in core as well to adopt this nicely.
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.
Pull Request Overview
This PR adds diff visualization for files requiring confirmation during edit operations. When the AI wants to edit sensitive or system files that require user approval, the confirmation dialog now shows unified diffs of the proposed changes instead of just the raw input/patch content.
Key changes:
- Introduced
formatDiffAsUnified()utility to generate unified diff format from old/new content - Refactored
createEditConfirmation()to accept an async callback that generates detailed diff previews - Implemented caching in edit tools to avoid recomputing patches/edits during prepare→invoke flow
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/extension/tools/node/editFileToolUtils.tsx | Added formatDiffAsUnified() function to generate unified diffs and updated createEditConfirmation() signature to support async detail messages |
| src/extension/tools/node/abstractReplaceStringTool.tsx | Refactored to cache prepared edits and generate diff confirmations; changed from urisForInput() to extractReplaceInputs() pattern |
| src/extension/tools/node/replaceStringTool.tsx | Updated to implement new extractReplaceInputs() method and use cached prepareEdits() |
| src/extension/tools/node/multiReplaceStringTool.tsx | Updated to implement new extractReplaceInputs() method for multi-file replacements |
| src/extension/tools/node/applyPatchTool.tsx | Added caching for patch processing and diff generation in confirmation dialogs |
| src/extension/tools/node/createFileTool.tsx | Updated to show diff for new file creation (empty→content) |
| src/extension/tools/node/insertEditTool.tsx | Updated function signature to match new async createEditConfirmation() API |
| src/extension/tools/node/editNotebookTool.tsx | Changed sendEditNotebookTelemetry() to accept model as string or LanguageModelChat object |
Goes with microsoft/vscode-copilot-chat#1905 - Adds common markdown rendering to the `IChatContentPartRenderContext`. These have to get passed down everywhere that can render markdown 'properly' and so this made sense, though we may want to revisit this and make them actual dependency-injected services with scope instantiation service. - Add a new basic `MarkdownDiffBlockPart` and allow consumers to optionally enable that for handling of `diff` blocks in their markdown. - Some further tweaks to deal with md/code in nested blocks.
Goes with microsoft/vscode-copilot-chat#1905 - Adds common markdown rendering to the `IChatContentPartRenderContext`. These have to get passed down everywhere that can render markdown 'properly' and so this made sense, though we may want to revisit this and make them actual dependency-injected services with scope instantiation service. - Add a new basic `MarkdownDiffBlockPart` and allow consumers to optionally enable that for handling of `diff` blocks in their markdown. - Some further tweaks to deal with md/code in nested blocks.
* edits: show diff for sensitive edit confirmations Goes with microsoft/vscode-copilot-chat#1905 - Adds common markdown rendering to the `IChatContentPartRenderContext`. These have to get passed down everywhere that can render markdown 'properly' and so this made sense, though we may want to revisit this and make them actual dependency-injected services with scope instantiation service. - Add a new basic `MarkdownDiffBlockPart` and allow consumers to optionally enable that for handling of `diff` blocks in their markdown. - Some further tweaks to deal with md/code in nested blocks. * comment
Requires a PR in core as well to adopt this nicely.