Merge pull request microsoft#182494 from microsoft/joh/ridiculous-herring

joh/ridiculous herring

Author: jrieken

src/vs/workbench/contrib/interactiveEditor/browser/interactiveEditorController.ts

@@ -6,6 +6,7 @@
 import { renderMarkdown } from 'vs/base/browser/markdownRenderer';
 import { Barrier, raceCancellationError } from 'vs/base/common/async';
 import { CancellationTokenSource } from 'vs/base/common/cancellation';
+import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { Emitter, Event } from 'vs/base/common/event';
 import { DisposableStore, toDisposable } from 'vs/base/common/lifecycle';
 import { isEqual } from 'vs/base/common/resources';
@@ -24,6 +25,7 @@ import { InlineCompletionsController } from 'vs/editor/contrib/inlineCompletions
 import { localize } from 'vs/nls';
 import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
 import { IContextKey, IContextKeyService } from 'vs/platform/contextkey/common/contextkey';
+import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
 import { IInstantiationService, ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
 import { ILogService } from 'vs/platform/log/common/log';
 import { EditResponse, EmptyResponse, ErrorResponse, IInteractiveEditorSessionService, MarkdownResponse, Session, SessionExchange } from 'vs/workbench/contrib/interactiveEditor/browser/interactiveEditorSession';
@@ -36,23 +38,23 @@ import { INotebookEditorService } from 'vs/workbench/contrib/notebook/browser/se
 import { CellUri } from 'vs/workbench/contrib/notebook/common/notebookCommon';
 
 const enum State {
-	CREATE_SESSION,
-	INIT_UI,
-	WAIT_FOR_INPUT,
-	MAKE_REQUEST,
-	APPLY_RESPONSE,
-	SHOW_RESPONSE,
-	PAUSE,
-	DONE,
+	CREATE_SESSION = 'CREATE_SESSION',
+	INIT_UI = 'INIT_UI',
+	WAIT_FOR_INPUT = 'WAIT_FOR_INPUT',
+	MAKE_REQUEST = 'MAKE_REQUEST',
+	APPLY_RESPONSE = 'APPLY_RESPONSE',
+	SHOW_RESPONSE = 'SHOW_RESPONSE',
+	PAUSE = 'PAUSE',
+	DONE = 'DONE',
 }
 
 const enum Message {
 	NONE = 0,
-	END_SESSION = 2 ** 0,
-	PAUSE_SESSION = 2 ** 1,
-	CANCEL_REQUEST = 2 ** 2,
-	CANCEL_INPUT = 2 ** 3,
-	ACCEPT_INPUT = 2 ** 4
+	END_SESSION = 1 << 0,
+	PAUSE_SESSION = 1 << 1,
+	CANCEL_REQUEST = 1 << 2,
+	CANCEL_INPUT = 1 << 3,
+	ACCEPT_INPUT = 1 << 4
 }
 
 export interface InteractiveEditorRunOptions {
@@ -101,6 +103,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		@IConfigurationService private readonly _configurationService: IConfigurationService,
 		@IModelService private readonly _modelService: IModelService,
 		@INotebookEditorService private readonly _notebookEditorService: INotebookEditorService,
+		@IDialogService private readonly _dialogService: IDialogService,
 		@IContextKeyService contextKeyService: IContextKeyService,
 	) {
 		this._ctxHasActiveRequest = CTX_INTERACTIVE_EDITOR_HAS_ACTIVE_REQUEST.bindTo(contextKeyService);
@@ -148,39 +151,13 @@ export class InteractiveEditorController implements IEditorContribution {
 
 	private async _nextState(state: State, options: InteractiveEditorRunOptions | undefined): Promise<void> {
 		this._logService.trace('[IE] setState to ', state);
-		let nextState: State | undefined;
-		switch (state) {
-			case State.CREATE_SESSION:
-				nextState = await this._createSession(options);
-				break;
-			case State.INIT_UI:
-				nextState = await this._initUI();
-				break;
-			case State.WAIT_FOR_INPUT:
-				nextState = await this._waitForInput(options);
-				break;
-			case State.MAKE_REQUEST:
-				nextState = await this._makeRequest();
-				break;
-			case State.APPLY_RESPONSE:
-				nextState = await this._applyResponse();
-				break;
-			case State.SHOW_RESPONSE:
-				nextState = await this._showResponse();
-				break;
-			case State.PAUSE:
-				this._pause();
-				break;
-			case State.DONE:
-				this._done();
-				break;
-		}
+		const nextState = await this[state](options);
 		if (nextState) {
 			this._nextState(nextState, options);
 		}
 	}
 
-	private async _createSession(options: InteractiveEditorRunOptions | undefined): Promise<State.DONE | State.INIT_UI> {
+	private async [State.CREATE_SESSION](options: InteractiveEditorRunOptions | undefined): Promise<State.DONE | State.INIT_UI> {
 		assertType(this._editor.hasModel());
 
 		let session: Session | undefined = options?.existingSession;
@@ -206,6 +183,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		delete options?.existingSession;
 
 		if (!session) {
+			this._dialogService.info(localize('create.fail', "Failed to start editor chat"), localize('create.fail.detail', "Please consult the error log and try again later."));
 			return State.DONE;
 		}
 
@@ -225,7 +203,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		return State.INIT_UI;
 	}
 
-	private async _initUI(): Promise<State.WAIT_FOR_INPUT | State.SHOW_RESPONSE> {
+	private async [State.INIT_UI](): Promise<State.WAIT_FOR_INPUT | State.SHOW_RESPONSE> {
 		assertType(this._activeSession);
 
 		// hide/cancel inline completions when invoking IE
@@ -296,7 +274,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		}
 	}
 
-	private async _waitForInput(options: InteractiveEditorRunOptions | undefined): Promise<State.DONE | State.PAUSE | State.WAIT_FOR_INPUT | State.MAKE_REQUEST> {
+	private async [State.WAIT_FOR_INPUT](options: InteractiveEditorRunOptions | undefined): Promise<State.DONE | State.PAUSE | State.WAIT_FOR_INPUT | State.MAKE_REQUEST> {
 		assertType(this._activeSession);
 
 		this._zone.show(this._activeSession.wholeRange.getEndPosition());
@@ -360,7 +338,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		return State.MAKE_REQUEST;
 	}
 
-	private async _makeRequest(): Promise<State.APPLY_RESPONSE | State.PAUSE | State.DONE> {
+	private async [State.MAKE_REQUEST](): Promise<State.APPLY_RESPONSE | State.PAUSE | State.DONE> {
 		assertType(this._editor.hasModel());
 		assertType(this._activeSession);
 		assertType(this._activeSession.lastInput);
@@ -427,7 +405,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		}
 	}
 
-	private async _applyResponse(): Promise<State.SHOW_RESPONSE | State.DONE> {
+	private async [State.APPLY_RESPONSE](): Promise<State.SHOW_RESPONSE | State.DONE> {
 		assertType(this._activeSession);
 		assertType(this._strategy);
 
@@ -461,7 +439,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		return State.SHOW_RESPONSE;
 	}
 
-	private async _showResponse(): Promise<State.WAIT_FOR_INPUT | State.DONE> {
+	private async [State.SHOW_RESPONSE](): Promise<State.WAIT_FOR_INPUT | State.DONE> {
 		assertType(this._activeSession);
 		assertType(this._strategy);
 
@@ -510,7 +488,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		return State.WAIT_FOR_INPUT;
 	}
 
-	private async _pause() {
+	private async [State.PAUSE]() {
 		assertType(this._activeSession);
 
 		this._ctxLastEditKind.reset();
@@ -528,10 +506,10 @@ export class InteractiveEditorController implements IEditorContribution {
 		this._activeSession = undefined;
 	}
 
-	private async _done() {
+	private async [State.DONE]() {
 		assertType(this._activeSession);
 		this._interactiveEditorSessionService.releaseSession(this._activeSession);
-		this._pause();
+		this[State.PAUSE]();
 	}
 
 	// ---- controller API
@@ -612,7 +590,13 @@ export class InteractiveEditorController implements IEditorContribution {
 		if (this._strategy) {
 			const strategy = this._strategy;
 			this._strategy = undefined;
-			await strategy?.apply();
+			try {
+				await strategy?.apply();
+			} catch (err) {
+				this._dialogService.error(localize('err.apply', "Failed to apply changes.", toErrorMessage(err)));
+				this._logService.error('[IE] FAILED to apply changes');
+				this._logService.error(err);
+			}
 			strategy?.dispose();
 			this._messages.fire(Message.END_SESSION);
 
@@ -626,7 +610,13 @@ export class InteractiveEditorController implements IEditorContribution {
 		if (this._strategy) {
 			const strategy = this._strategy;
 			this._strategy = undefined;
-			await strategy?.cancel();
+			try {
+				await strategy?.cancel();
+			} catch (err) {
+				this._dialogService.error(localize('err.discard', "Failed to discard changes.", toErrorMessage(err)));
+				this._logService.error('[IE] FAILED to discard changes');
+				this._logService.error(err);
+			}
 			strategy?.dispose();
 			this._messages.fire(Message.END_SESSION);
 		}

src/vs/workbench/contrib/interactiveEditor/browser/interactiveEditorLivePreviewWidget.ts

@@ -148,8 +148,8 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
 	private _updateFromChanges(range: Range, changes: LineRangeMapping[]): void {
 		assertType(this.editor.hasModel());
 
-		if (changes.length === 0) {
-			// no change
+		if (changes.length === 0 || this._textModelv0.getValueLength() === 0) {
+			// no change or changes to an empty file
 			this._logService.debug('[IE] livePreview-mode: no diff');
 			this.hide();
 

src/vs/workbench/contrib/interactiveEditor/browser/interactiveEditorSession.ts

@@ -282,7 +282,14 @@ export class InteractiveEditorSessionService implements IInteractiveEditorSessio
 
 		const textModel = editor.getModel();
 		const selection = editor.getSelection();
-		const raw = await provider.prepareInteractiveEditorSession(textModel, selection, token);
+		let raw: IInteractiveEditorSession | undefined | null;
+		try {
+			raw = await provider.prepareInteractiveEditorSession(textModel, selection, token);
+		} catch (error) {
+			this._logService.error('[IE] FAILED to prepare session', provider.debugName);
+			this._logService.error(error);
+			return undefined;
+		}
 		if (!raw) {
 			this._logService.trace('[IE] NO session', provider.debugName);
 			return undefined;

src/vs/workbench/contrib/interactiveEditor/browser/interactiveEditorStrategies.ts

@@ -40,8 +40,6 @@ export abstract class EditModeStrategy {
 
 	abstract renderChanges(response: EditResponse, changes: LineRangeMapping[]): Promise<void>;
 
-	abstract hide(): Promise<void>;
-
 	abstract toggleInlineDiff(): void;
 }
 
@@ -101,10 +99,6 @@ export class PreviewStrategy extends EditModeStrategy {
 		}
 	}
 
-	override async hide(): Promise<void> {
-		// nothing to do, input widget will be hidden by controller
-	}
-
 	async cancel(): Promise<void> {
 		// nothing to do
 	}
@@ -206,6 +200,7 @@ export class LiveStrategy extends EditModeStrategy {
 	private readonly _inlineDiffDecorations: InlineDiffDecorations;
 	private readonly _ctxInlineDiff: IContextKey<boolean>;
 	private _lastResponse?: EditResponse;
+	private _editCount: number = 0;
 
 	constructor(
 		protected readonly _session: Session,
@@ -255,15 +250,14 @@ export class LiveStrategy extends EditModeStrategy {
 	}
 
 	async apply() {
+		if (this._editCount > 0) {
+			this._editor.pushUndoStop();
+		}
 		if (this._lastResponse?.workspaceEdits) {
 			await this._bulkEditService.apply(this._lastResponse.workspaceEdits);
 		}
 	}
 
-	override async hide(): Promise<void> {
-		this._inlineDiffDecorations.clear();
-	}
-
 	async cancel() {
 		const { textModelN: modelN, textModel0: model0, lastSnapshot } = this._session;
 		if (modelN.isDisposed() || (model0.isDisposed() && !lastSnapshot)) {
@@ -281,7 +275,7 @@ export class LiveStrategy extends EditModeStrategy {
 		}
 	}
 
-	override async makeChanges(_response: EditResponse, edits: ISingleEditOperation[]): Promise<void> {
+	override async makeChanges(_response: EditResponse, edits: ISingleEditOperation[], ignoreInlineDiff?: boolean): Promise<void> {
 		const cursorStateComputerAndInlineDiffCollection: ICursorStateComputer = (undoEdits) => {
 			let last: Position | null = null;
 			for (const edit of undoEdits) {
@@ -291,9 +285,11 @@ export class LiveStrategy extends EditModeStrategy {
 			return last && [Selection.fromPositions(last)];
 		};
 
-		this._editor.pushUndoStop();
-		this._editor.executeEdits('interactive-editor-live', edits, cursorStateComputerAndInlineDiffCollection);
-		this._editor.pushUndoStop();
+		// push undo stop before first edit
+		if (++this._editCount === 1) {
+			this._editor.pushUndoStop();
+		}
+		this._editor.executeEdits('interactive-editor-live', edits, ignoreInlineDiff ? undefined : cursorStateComputerAndInlineDiffCollection);
 	}
 
 	override async renderChanges(response: EditResponse, textModel0Changes: LineRangeMapping[]) {
@@ -356,15 +352,8 @@ export class LivePreviewStrategy extends LiveStrategy {
 		super.dispose();
 	}
 
-	override async hide(): Promise<void> {
-		this._diffZone.hide();
-		super.hide();
-	}
-
 	override async makeChanges(_response: EditResponse, edits: ISingleEditOperation[]): Promise<void> {
-		this._editor.pushUndoStop();
-		this._editor.executeEdits('interactive-editor-livePreview', edits);
-		this._editor.pushUndoStop();
+		super.makeChanges(_response, edits, true);
 	}
 
 	override async renderChanges(response: EditResponse, changes: LineRangeMapping[]) {