Improve behavior of AI Changeset diff editor #14786
Conversation
partially fixes eclipse-theia#14785 Contributed on behalf of STMicroelectronics Signed-off-by: Thomas Mäder <[email protected]>
| readContents: async () => element.targetState ?? '', | ||
| saveContents: async (content: string, options?: ResourceSaveOptions): Promise<void> => { | ||
| element.accept(); |
There was a problem hiding this comment.
I think we should still save the content and not just plain accept, because the user may have edited something in the diff editor and then expects that the save stores the file with their edit and not just the original suggestion of the AI agent.
Setting the element to state "accept" is a nice touch though as it is a great reference for the user what they already reviewed and saved.
There was a problem hiding this comment.
Excellent work, thank you! Feels pretty good now.
I just found two issues while testing and reviewing the changes:
- When the user hits save (Ctrl-S) on a new file, Theia asks where to store this file, even though it should actually already be clear where to put this file.
- When the user modifies the content in the diff editor, the changes are lost. I think this may be easy to fix, if we just store the
content-- see inline comment.
WDYT?
Signed-off-by: Thomas Mäder <[email protected]>
There was a problem hiding this comment.
Thanks for the revision! This is certainly a great improvement to before, so I'm ok with merging it and addressing the remaining issues in follow-ups.
Issues I found:
- When opening new files (click on the added file item), the label is some random id instead of the file name. Save works fine though.
- When opening the diff editor on new files and saving, it still asks me where to save the file. I'd have expected that it would just create and save the current content to the defined URI location.
| @@ -1,3 +1,2 @@ | |||
| { | |||
| "npm/npmjs/-/unicode-property-aliases-ecmascript/2.1.0": "Manually approved" | |||
There was a problem hiding this comment.
This change seems unrelated
There was a problem hiding this comment.
Yes, it's not needed anymore
Signed-off-by: Thomas Mäder <[email protected]>
What it does
Moves the diff to the right side and applies the changes to the underlying file upon "save" in the diff editor. If an editor is open at the time, it will be updated and will retain the same dirty state as before.
partially fixes #14785
How to test
Create an AI change set (for example by invoking @Changeset in the AI chat) and play around with the diff editor, "accept", etc.
Follow-ups
Breaking changes
Attribution
Contributed on behalf of STMicroelectronics
Review checklist
Reminder for reviewers