More reuse via abstract modified file entry (microsoft#242311)

* More reuse via abstract modified file entry

re microsoft/vscode-copilot#13692 and microsoft/vscode-copilot#13693

* Updates

* add todo

* Fixes

---------

Co-authored-by: Don Jayamanne <[email protected]>

Author: jrieken

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedDocumentEntry.ts

@@ -33,7 +33,7 @@ import { IFileService } from '../../../../../platform/files/common/files.js';
 import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
 import { observableConfigValue } from '../../../../../platform/observable/common/platformObservableUtils.js';
 import { editorSelectionBackground } from '../../../../../platform/theme/common/colorRegistry.js';
-import { IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
+import { IUndoRedoElement, IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
 import { SaveReason, IEditorPane } from '../../../../common/editor.js';
 import { IFilesConfigurationService } from '../../../../services/filesConfiguration/common/filesConfigurationService.js';
 import { IResolvedTextFileEditorModel, stringToSnapshot } from '../../../../services/textfile/common/textfiles.js';
@@ -107,8 +107,8 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
 		@IFilesConfigurationService fileConfigService: IFilesConfigurationService,
 		@IChatService chatService: IChatService,
 		@IEditorWorkerService private readonly _editorWorkerService: IEditorWorkerService,
-		@IUndoRedoService private readonly _undoRedoService: IUndoRedoService,
 		@IFileService fileService: IFileService,
+		@IUndoRedoService undoRedoService: IUndoRedoService,
 		@IInstantiationService instantiationService: IInstantiationService
 	) {
 		super(
@@ -119,6 +119,7 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
 			fileConfigService,
 			chatService,
 			fileService,
+			undoRedoService,
 			instantiationService
 		);
 
@@ -199,12 +200,9 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
 		this._setDocValue(this.initialContent);
 	}
 
-	override async acceptStreamingEditsEnd(tx: ITransaction) {
+	protected override async _areOriginalAndModifiedIdentical(): Promise<boolean> {
 		const diff = await this._diffOperation;
-		super.acceptStreamingEditsEnd(tx);
-		if (diff?.identical) {
-			this.accept(tx);
-		}
+		return diff ? diff.identical : false;
 	}
 
 	protected override _resetEditsState(tx: ITransaction): void {
@@ -266,13 +264,10 @@ export class ChatEditingModifiedDocumentEntry extends AbstractChatEditingModifie
 		}
 	}
 
-	override acceptStreamingEditsStart(responseModel: IChatResponseModel, tx: ITransaction) {
-		super.acceptStreamingEditsStart(responseModel, tx);
-
-		// push stack element whenever streaming starts
-		const request = responseModel.session.getRequests().find(req => req.id === responseModel.requestId);
+	protected override _createUndoRedoElement(response: IChatResponseModel): IUndoRedoElement {
+		const request = response.session.getRequests().find(req => req.id === response.requestId);
 		const label = request?.message.text ? localize('chatEditing1', "Chat Edit: '{0}'", request.message.text) : localize('chatEditing2', "Chat Edit");
-		this._undoRedoService.pushElement(new SingleModelEditStackElement(label, 'chat.edit', this.modifiedModel, null));
+		return new SingleModelEditStackElement(label, 'chat.edit', this.modifiedModel, null);
 	}
 
 	async acceptAgentEdits(resource: URI, textEdits: (TextEdit | ICellEditOperation)[], isLastEdits: boolean, responseModel: IChatResponseModel): Promise<void> {

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedFileEntry.ts

@@ -17,6 +17,7 @@ import { IFileService } from '../../../../../platform/files/common/files.js';
 import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
 import { observableConfigValue } from '../../../../../platform/observable/common/platformObservableUtils.js';
 import { editorBackground, registerColor, transparent } from '../../../../../platform/theme/common/colorRegistry.js';
+import { IUndoRedoElement, IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
 import { IEditorPane } from '../../../../common/editor.js';
 import { IFilesConfigurationService } from '../../../../services/filesConfiguration/common/filesConfigurationService.js';
 import { ICellEditOperation } from '../../../notebook/common/notebookCommon.js';
@@ -88,6 +89,7 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im
 		@IFilesConfigurationService fileConfigService: IFilesConfigurationService,
 		@IChatService protected readonly _chatService: IChatService,
 		@IFileService protected readonly _fileService: IFileService,
+		@IUndoRedoService private readonly _undoRedoService: IUndoRedoService,
 		@IInstantiationService protected readonly _instantiationService: IInstantiationService,
 	) {
 		super();
@@ -223,15 +225,26 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im
 		this._resetEditsState(tx);
 		this._isCurrentlyBeingModifiedByObs.set(responseModel, tx);
 		this._autoAcceptCtrl.get()?.cancel();
+
+		const undoRedoElement = this._createUndoRedoElement(responseModel);
+		if (undoRedoElement) {
+			this._undoRedoService.pushElement(undoRedoElement);
+		}
 	}
 
+	protected abstract _createUndoRedoElement(response: IChatResponseModel): IUndoRedoElement | undefined;
+
 	abstract acceptAgentEdits(uri: URI, edits: (TextEdit | ICellEditOperation)[], isLastEdits: boolean, responseModel: IChatResponseModel): Promise<void>;
 
 	async acceptStreamingEditsEnd(tx: ITransaction) {
 		this._resetEditsState(tx);
 
-		// AUTO accept mode
-		if (!this.reviewMode.get() && !this._autoAcceptCtrl.get()) {
+		if (await this._areOriginalAndModifiedIdentical()) {
+			// ACCEPT if identical
+			this.accept(tx);
+
+		} else if (!this.reviewMode.get() && !this._autoAcceptCtrl.get()) {
+			// AUTO accept mode
 
 			const acceptTimeout = this._autoAcceptTimeout.get() * 1000;
 			const future = Date.now() + acceptTimeout;
@@ -259,6 +272,8 @@ export abstract class AbstractChatEditingModifiedFileEntry extends Disposable im
 		}
 	}
 
+	protected abstract _areOriginalAndModifiedIdentical(): Promise<boolean>;
+
 	protected _resetEditsState(tx: ITransaction): void {
 		this._isCurrentlyBeingModifiedByObs.set(undefined, tx);
 		this._rewriteRatioObs.set(0, tx);

src/vs/workbench/contrib/chat/browser/chatEditing/chatEditingModifiedNotebookEntry.ts

@@ -37,7 +37,7 @@ import { IFileService } from '../../../../../platform/files/common/files.js';
 import { IInstantiationService } from '../../../../../platform/instantiation/common/instantiation.js';
 import { observableConfigValue } from '../../../../../platform/observable/common/platformObservableUtils.js';
 import { editorSelectionBackground } from '../../../../../platform/theme/common/colorRegistry.js';
-import { IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
+import { IUndoRedoElement, IUndoRedoService } from '../../../../../platform/undoRedo/common/undoRedo.js';
 import { IEditorPane, SaveReason } from '../../../../common/editor.js';
 import { IFilesConfigurationService } from '../../../../services/filesConfiguration/common/filesConfigurationService.js';
 import { SnapshotContext } from '../../../../services/workingCopy/common/fileWorkingCopy.js';
@@ -193,9 +193,10 @@ export class ChatEditingModifiedNotebookEntry extends AbstractChatEditingModifie
 		@IFileService fileService: IFileService,
 		@IInstantiationService instantiationService: IInstantiationService,
 		@ITextModelService private readonly textModelService: ITextModelService,
-		@IModelService private readonly modelService: IModelService
+		@IModelService private readonly modelService: IModelService,
+		@IUndoRedoService undoRedoService: IUndoRedoService
 	) {
-		super(modifiedResourceRef.object.notebook.uri, telemetryInfo, kind, configurationService, fileConfigService, chatService, fileService, instantiationService);
+		super(modifiedResourceRef.object.notebook.uri, telemetryInfo, kind, configurationService, fileConfigService, chatService, fileService, undoRedoService, instantiationService);
 		this._register(modifiedResourceRef);
 		this._register(originalResourceRef);
 		this.modifiedModel = modifiedResourceRef.object.notebook;
@@ -335,6 +336,15 @@ export class ChatEditingModifiedNotebookEntry extends AbstractChatEditingModifie
 		this.cellEntryMap.forEach(entry => !entry.disposed && entry.clearCurrentEditLineDecoration());
 	}
 
+	protected override _createUndoRedoElement(_response: IChatResponseModel): IUndoRedoElement | undefined {
+		// TODO@amunger
+		return undefined;
+	}
+
+	protected override async _areOriginalAndModifiedIdentical(): Promise<boolean> {
+		return createSnapshot(this.originalModel, this.transientOptions, this.configurationService) === createSnapshot(this.modifiedModel, this.transientOptions, this.configurationService);
+	}
+
 	override async acceptAgentEdits(resource: URI, edits: (TextEdit | ICellEditOperation)[], isLastEdits: boolean, responseModel: IChatResponseModel): Promise<void> {
 		const isCellUri = resource.scheme === Schemas.vscodeNotebookCell;
 		const cell = isCellUri && this.modifiedModel.cells.find(cell => isEqual(cell.uri, resource));