Merge pull request #184317 from microsoft/joh/central-rooster

joh/central rooster

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

@@ -261,7 +261,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		this._sessionStore.clear();
 
 		const wholeRangeDecoration = this._editor.createDecorationsCollection([{
-			range: this._activeSession.wholeRange,
+			range: this._activeSession.wholeRange.value,
 			options: InteractiveEditorController._decoBlock
 		}]);
 		this._sessionStore.add(toDisposable(() => wholeRangeDecoration.clear()));
@@ -270,7 +270,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		this._zone.value.widget.placeholder = this._getPlaceholderText();
 		this._zone.value.widget.value = this._activeSession.lastInput ?? '';
 		this._zone.value.widget.updateInfo(this._activeSession.session.message ?? localize('welcome.1', "AI-generated code may be incorrect"));
-		this._zone.value.show(this._activeSession.wholeRange.getEndPosition());
+		this._zone.value.show(this._activeSession.wholeRange.value.getEndPosition());
 		this._zone.value.widget.preferredExpansionState = this._activeSession.lastExpansionState;
 
 		this._sessionStore.add(this._editor.onDidChangeModel((e) => {
@@ -289,7 +289,7 @@ export class InteractiveEditorController implements IEditorContribution {
 			const wholeRange = this._activeSession!.wholeRange;
 			let editIsOutsideOfWholeRange = false;
 			for (const { range } of e.changes) {
-				editIsOutsideOfWholeRange = !Range.areIntersectingOrTouching(range, wholeRange);
+				editIsOutsideOfWholeRange = !Range.areIntersectingOrTouching(range, wholeRange.value);
 			}
 
 			this._activeSession!.recordExternalEditOccurred(editIsOutsideOfWholeRange);
@@ -360,7 +360,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		assertType(this._activeSession);
 
 		this._zone.value.widget.placeholder = this._getPlaceholderText();
-		this._zone.value.show(this._activeSession.wholeRange.getEndPosition());
+		this._zone.value.show(this._activeSession.wholeRange.value.getEndPosition());
 
 		if (options?.message) {
 			this._zone.value.widget.value = options?.message;
@@ -411,7 +411,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		const refer = this._activeSession.session.slashCommands?.some(value => value.refer && input!.startsWith(`/${value.command}`));
 		if (refer) {
 			this._log('[IE] seeing refer command, continuing outside editor', this._activeSession.provider.debugName);
-			this._editor.setSelection(this._activeSession.wholeRange);
+			this._editor.setSelection(this._activeSession.wholeRange.value);
 			this._instaService.invokeFunction(sendRequest, input);
 
 			if (!this._activeSession.lastExchange) {
@@ -447,7 +447,7 @@ export class InteractiveEditorController implements IEditorContribution {
 		const request: IInteractiveEditorRequest = {
 			prompt: this._activeSession.lastInput,
 			selection: this._editor.getSelection(),
-			wholeRange: this._activeSession.wholeRange,
+			wholeRange: this._activeSession.wholeRange.value,
 			attempt: 0,
 		};
 		const task = this._activeSession.provider.provideResponse(this._activeSession.session, request, requestCts.token);
@@ -520,7 +520,8 @@ export class InteractiveEditorController implements IEditorContribution {
 
 			try {
 				this._ignoreModelContentChanged = true;
-				await this._strategy.makeChanges(response, editOperations);
+				this._activeSession.wholeRange.trackEdits(editOperations);
+				await this._strategy.makeChanges(editOperations);
 				this._ctxDidEdit.set(this._activeSession.hasChangedText);
 			} finally {
 				this._ignoreModelContentChanged = false;
@@ -569,13 +570,7 @@ export class InteractiveEditorController implements IEditorContribution {
 				return State.ACCEPT;
 			}
 
-			try {
-				this._ignoreModelContentChanged = true;
-				await this._strategy.renderChanges(response);
-				this._ctxDidEdit.set(this._activeSession.hasChangedText);
-			} finally {
-				this._ignoreModelContentChanged = false;
-			}
+			await this._strategy.renderChanges(response);
 		}
 
 		return State.WAIT_FOR_INPUT;

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

@@ -137,7 +137,7 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
 		this._sessionStore.add(this._diffEditor.onDidUpdateDiff(() => {
 			const result = this._diffEditor.getDiffComputationResult();
 			const hasFocus = this._diffEditor.hasTextFocus();
-			this._updateFromChanges(this._session.wholeRange, result?.changes2 ?? []);
+			this._updateFromChanges(this._session.wholeRange.value, result?.changes2 ?? []);
 			// TODO@jrieken find a better fix for this. this is the challenge:
 			// the _doShowForChanges method invokes show of the zone widget which removes and adds the
 			// zone and overlay parts. this dettaches and reattaches the dom nodes which means they lose
@@ -146,7 +146,7 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
 				this._diffEditor.focus();
 			}
 		}));
-		this._updateFromChanges(this._session.wholeRange, this._session.lastTextModelChanges);
+		this._updateFromChanges(this._session.wholeRange.value, this._session.lastTextModelChanges);
 		this._isVisible = true;
 	}
 

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

@@ -8,7 +8,7 @@ import { URI } from 'vs/base/common/uri';
 import { Event } from 'vs/base/common/event';
 import { ResourceEdit, ResourceFileEdit, ResourceTextEdit } from 'vs/editor/browser/services/bulkEditService';
 import { TextEdit } from 'vs/editor/common/languages';
-import { ITextModel } from 'vs/editor/common/model';
+import { IModelDeltaDecoration, ITextModel } from 'vs/editor/common/model';
 import { EditMode, IInteractiveEditorSessionProvider, IInteractiveEditorSession, IInteractiveEditorBulkEditResponse, IInteractiveEditorEditResponse, IInteractiveEditorMessageResponse, IInteractiveEditorResponse, IInteractiveEditorService } from 'vs/workbench/contrib/interactiveEditor/common/interactiveEditor';
 import { IRange, Range } from 'vs/editor/common/core/range';
 import { IActiveCodeEditor, ICodeEditor } from 'vs/editor/browser/editorBrowser';
@@ -24,6 +24,7 @@ import { Iterable } from 'vs/base/common/iterator';
 import { toErrorMessage } from 'vs/base/common/errorMessage';
 import { isCancellationError } from 'vs/base/common/errors';
 import { LineRangeMapping } from 'vs/editor/common/diff/linesDiffComputer';
+import { ISingleEditOperation } from 'vs/editor/common/core/editOperation';
 
 export type Recording = {
 	when: Date;
@@ -61,6 +62,50 @@ export enum ExpansionState {
 	NOT_CROPPED = 'not_cropped'
 }
 
+class SessionWholeRange {
+
+	private static readonly _options = { description: 'interactiveEditor/session/wholeRange' };
+
+	private readonly _store = new DisposableStore();
+	private readonly _decorationIds: string[] = [];
+
+	constructor(private readonly _textModel: ITextModel, wholeRange: IRange) {
+		this._decorationIds = _textModel.deltaDecorations([], [{ range: wholeRange, options: SessionWholeRange._options }]);
+		this._store.add(toDisposable(() => {
+			if (!_textModel.isDisposed()) {
+				_textModel.deltaDecorations(this._decorationIds, []);
+			}
+		}));
+	}
+
+	dispose() {
+		this._store.dispose();
+	}
+
+	trackEdits(edits: ISingleEditOperation[]): void {
+		const newDeco: IModelDeltaDecoration[] = [];
+		for (const edit of edits) {
+			newDeco.push({ range: edit.range, options: SessionWholeRange._options });
+		}
+		this._decorationIds.push(...this._textModel.deltaDecorations([], newDeco));
+	}
+
+	get value(): Range {
+		let result: Range | undefined;
+		for (const id of this._decorationIds) {
+			const range = this._textModel.getDecorationRange(id);
+			if (range) {
+				if (!result) {
+					result = range;
+				} else {
+					result = Range.plusRange(result, range);
+				}
+			}
+		}
+		return result!;
+	}
+}
+
 export class Session {
 
 	private _lastInput: string | undefined;
@@ -81,7 +126,7 @@ export class Session {
 		readonly textModelN: ITextModel,
 		readonly provider: IInteractiveEditorSessionProvider,
 		readonly session: IInteractiveEditorSession,
-		private readonly _wholeRangeMarkerId: string
+		readonly wholeRange: SessionWholeRange
 	) {
 		this.textModelNAltVersion = textModelN.getAlternativeVersionId();
 		this._teldata = {
@@ -122,11 +167,6 @@ export class Session {
 		return this._textModelNSnapshotAltVersion;
 	}
 
-	get wholeRange(): Range {
-		return this.textModelN.getDecorationRange(this._wholeRangeMarkerId)!;
-		// return new Range(1, 1, 1, 1);
-	}
-
 	createSnapshot(): void {
 		this._textModelNSnapshotAltVersion = this.textModelN.getAlternativeVersionId();
 	}
@@ -371,19 +411,15 @@ export class InteractiveEditorSessionService implements IInteractiveEditorSessio
 		if (!wholeRange) {
 			wholeRange = raw.wholeRange ? Range.lift(raw.wholeRange) : editor.getSelection();
 		}
-		if (Range.isEmpty(wholeRange)) {
-			wholeRange = new Range(wholeRange.startLineNumber, 1, wholeRange.endLineNumber, textModel.getLineMaxColumn(wholeRange.endLineNumber));
-		}
 
-		// install a marker for the decoration range
-		const [wholeRangeDecorationId] = textModel.deltaDecorations([], [{ range: wholeRange, options: { description: 'interactiveEditor/session/wholeRange' } }]);
-		store.add(toDisposable(() => {
-			if (!textModel.isDisposed()) {
-				textModel.deltaDecorations([wholeRangeDecorationId], []);
-			}
-		}));
+		// expand to whole lines
+		wholeRange = new Range(wholeRange.startLineNumber, 1, wholeRange.endLineNumber, textModel.getLineMaxColumn(wholeRange.endLineNumber));
 
-		const session = new Session(options.editMode, editor, textModel0, textModel, provider, raw, wholeRangeDecorationId);
+		// install managed-marker for the decoration range
+		const wholeRangeMgr = new SessionWholeRange(textModel, wholeRange);
+		store.add(wholeRangeMgr);
+
+		const session = new Session(options.editMode, editor, textModel0, textModel, provider, raw, wholeRangeMgr);
 
 		// store: key -> session
 		const key = this._key(editor, textModel.uri);

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

@@ -37,7 +37,7 @@ export abstract class EditModeStrategy {
 
 	abstract cancel(): Promise<void>;
 
-	abstract makeChanges(response: EditResponse, edits: ISingleEditOperation[]): Promise<void>;
+	abstract makeChanges(edits: ISingleEditOperation[]): Promise<void>;
 
 	abstract renderChanges(response: EditResponse): Promise<void>;
 
@@ -106,7 +106,7 @@ export class PreviewStrategy extends EditModeStrategy {
 		// nothing to do
 	}
 
-	override async makeChanges(_response: EditResponse, _edits: ISingleEditOperation[]): Promise<void> {
+	override async makeChanges(_edits: ISingleEditOperation[]): Promise<void> {
 		// nothing to do
 	}
 
@@ -287,7 +287,7 @@ export class LiveStrategy extends EditModeStrategy {
 		}
 	}
 
-	override async makeChanges(_response: EditResponse, edits: ISingleEditOperation[], ignoreInlineDiff?: boolean): Promise<void> {
+	override async makeChanges(edits: ISingleEditOperation[], ignoreInlineDiff?: boolean): Promise<void> {
 		const cursorStateComputerAndInlineDiffCollection: ICursorStateComputer = (undoEdits) => {
 			let last: Position | null = null;
 			for (const edit of undoEdits) {
@@ -362,10 +362,6 @@ export class LivePreviewStrategy extends LiveStrategy {
 		super.dispose();
 	}
 
-	override async makeChanges(_response: EditResponse, edits: ISingleEditOperation[]): Promise<void> {
-		super.makeChanges(_response, edits, true);
-	}
-
 	override async renderChanges(response: EditResponse) {
 
 		this._updateSummaryMessage();
@@ -374,7 +370,7 @@ export class LivePreviewStrategy extends LiveStrategy {
 		}
 
 		if (response.singleCreateFileEdit) {
-			this._previewZone.showCreation(this._session.wholeRange, response.singleCreateFileEdit.uri, await Promise.all(response.singleCreateFileEdit.edits));
+			this._previewZone.showCreation(this._session.wholeRange.value, response.singleCreateFileEdit.uri, await Promise.all(response.singleCreateFileEdit.edits));
 		} else {
 			this._previewZone.hide();
 		}

src/vs/workbench/contrib/interactiveEditor/common/interactiveEditorServiceImpl.ts

@@ -32,6 +32,6 @@ export class InteractiveEditorServiceImpl implements IInteractiveEditorService {
 	}
 
 	getAllProvider() {
-		return [...this._entries];
+		return [...this._entries].reverse();
 	}
 }

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

@@ -23,16 +23,39 @@ import { ITextModel } from 'vs/editor/common/model';
 import { IEditorProgressService, IProgressRunner } from 'vs/platform/progress/common/progress';
 import { mock } from 'vs/base/test/common/mock';
 import { Emitter, Event } from 'vs/base/common/event';
+import { equals } from 'vs/base/common/arrays';
 
 suite('InteractiveEditorController', function () {
 
 	class TestController extends InteractiveEditorController {
 
+		static INIT_SEQUENCE: readonly State[] = [State.CREATE_SESSION, State.INIT_UI, State.WAIT_FOR_INPUT];
+		static INIT_SEQUENCE_AUTO_SEND: readonly State[] = [...this.INIT_SEQUENCE, State.MAKE_REQUEST, State.APPLY_RESPONSE, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT];
+
 		private readonly _onDidChangeState = new Emitter<State>();
 		readonly onDidChangeState: Event<State> = this._onDidChangeState.event;
 
 		readonly states: readonly State[] = [];
 
+		waitFor(states: readonly State[]): Promise<void> {
+			const actual: State[] = [];
+
+			return new Promise<void>((resolve, reject) => {
+				const d = this.onDidChangeState(state => {
+					actual.push(state);
+					if (equals(states, actual)) {
+						d.dispose();
+						resolve();
+					}
+				});
+
+				setTimeout(() => {
+					d.dispose();
+					reject(`timeout, \nWANTED ${states.join('>')}, \nGOT ${actual.join('>')}`);
+				}, 1000);
+			});
+		}
+
 		protected override _nextState(state: State, options: InteractiveEditorRunOptions | undefined): Promise<void> {
 			this._onDidChangeState.fire(state);
 			(<State[]>this.states).push(state);
@@ -51,6 +74,7 @@ suite('InteractiveEditorController', function () {
 	let ctrl: TestController;
 	// let contextKeys: MockContextKeyService;
 	let interactiveEditorService: InteractiveEditorServiceImpl;
+	let interactiveEditorSessionService: IInteractiveEditorSessionService;
 	let instaService: TestInstantiationService;
 
 	setup(function () {
@@ -74,6 +98,7 @@ suite('InteractiveEditorController', function () {
 		);
 
 		instaService = workbenchInstantiationService(undefined, store).createChild(serviceCollection);
+		interactiveEditorSessionService = instaService.get(IInteractiveEditorSessionService);
 
 		model = instaService.get(IModelService).createModel('Hello\nWorld\nHello Again\nHello World\n', null);
 		editor = instantiateTestCodeEditor(instaService, model);
@@ -123,4 +148,117 @@ suite('InteractiveEditorController', function () {
 
 		assert.ok(ctrl.getWidgetPosition() === undefined);
 	});
+
+	test('wholeRange expands to whole lines, editor selection default', async function () {
+
+		editor.setSelection(new Range(1, 1, 1, 3));
+		ctrl = instaService.createInstance(TestController, editor);
+
+		const d = interactiveEditorService.addProvider({
+			debugName: 'Unit Test',
+			prepareInteractiveEditorSession() {
+				return {
+					id: Math.random()
+				};
+			},
+			provideResponse(session, request) {
+				throw new Error();
+			}
+		});
+
+		ctrl.run({});
+		await Event.toPromise(Event.filter(ctrl.onDidChangeState, e => e === State.WAIT_FOR_INPUT));
+
+		const session = interactiveEditorSessionService.getSession(editor, editor.getModel()!.uri);
+		assert.ok(session);
+		assert.deepStrictEqual(session.wholeRange.value, new Range(1, 1, 1, 6));
+
+		ctrl.cancelSession();
+		d.dispose();
+	});
+
+	test('wholeRange expands to whole lines, session provided', async function () {
+
+		editor.setSelection(new Range(1, 1, 1, 1));
+		ctrl = instaService.createInstance(TestController, editor);
+
+		const d = interactiveEditorService.addProvider({
+			debugName: 'Unit Test',
+			prepareInteractiveEditorSession() {
+				return {
+					id: Math.random(),
+					wholeRange: new Range(1, 1, 1, 3)
+				};
+			},
+			provideResponse(session, request) {
+				throw new Error();
+			}
+		});
+
+		ctrl.run({});
+		await Event.toPromise(Event.filter(ctrl.onDidChangeState, e => e === State.WAIT_FOR_INPUT));
+
+		const session = interactiveEditorSessionService.getSession(editor, editor.getModel()!.uri);
+		assert.ok(session);
+		assert.deepStrictEqual(session.wholeRange.value, new Range(1, 1, 1, 6));
+
+		ctrl.cancelSession();
+		d.dispose();
+	});
+
+	test('typing outside of wholeRange finishes session', async function () {
+		ctrl = instaService.createInstance(TestController, editor);
+		ctrl.run({ message: 'Hello', autoSend: true });
+
+		await ctrl.waitFor(TestController.INIT_SEQUENCE_AUTO_SEND);
+
+		const session = interactiveEditorSessionService.getSession(editor, editor.getModel()!.uri);
+		assert.ok(session);
+		assert.deepStrictEqual(session.wholeRange.value, new Range(1, 1, 1, 11));
+
+		editor.setSelection(new Range(2, 1, 2, 1));
+		editor.trigger('test', 'type', { text: 'a' });
+
+		await ctrl.waitFor([State.ACCEPT]);
+	});
+
+	test('\'whole range\' isn\'t updated for edits outside whole range #4346', async function () {
+
+		editor.setSelection(new Range(3, 1, 3, 1));
+
+		const d = interactiveEditorService.addProvider({
+			debugName: 'Unit Test',
+			prepareInteractiveEditorSession() {
+				return {
+					id: Math.random(),
+					wholeRange: new Range(3, 1, 3, 3)
+				};
+			},
+			provideResponse(session, request) {
+				return {
+					type: InteractiveEditorResponseType.EditorEdit,
+					id: Math.random(),
+					edits: [{
+						range: new Range(1, 1, 1, 1), // EDIT happens outside of whole range
+						text: `${request.prompt}\n${request.prompt}`
+					}]
+				};
+			}
+		});
+		store.add(d);
+		ctrl = instaService.createInstance(TestController, editor);
+		ctrl.run({ message: 'Hello', autoSend: false });
+
+		await ctrl.waitFor(TestController.INIT_SEQUENCE);
+
+		const session = interactiveEditorSessionService.getSession(editor, editor.getModel()!.uri);
+		assert.ok(session);
+		assert.deepStrictEqual(session.wholeRange.value, new Range(3, 1, 3, 12));
+
+		ctrl.accept();
+
+		await ctrl.waitFor([State.MAKE_REQUEST, State.APPLY_RESPONSE, State.SHOW_RESPONSE, State.WAIT_FOR_INPUT]);
+
+		assert.deepStrictEqual(session.wholeRange.value, new Range(1, 1, 4, 12));
+	});
 });