From 600a3639b846bc47487401ad7df27c05297338bb Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Fri, 26 Jul 2024 13:21:14 -0700 Subject: [PATCH] Simplify IChatWidgetContrib Remove onDidChangeInputState Better fix for microsoft/vscode-copilot#6957 --- .../contrib/chat/browser/chatInputPart.ts | 32 +++++++++---------- .../contrib/chat/browser/chatViewPane.ts | 19 +++++++---- .../contrib/chat/browser/chatWidget.ts | 18 ++--------- .../browser/contrib/chatContextAttachments.ts | 6 ---- .../browser/contrib/chatDynamicVariables.ts | 9 ------ 5 files changed, 30 insertions(+), 54 deletions(-) diff --git a/src/vs/workbench/contrib/chat/browser/chatInputPart.ts b/src/vs/workbench/contrib/chat/browser/chatInputPart.ts index a0e272e94af06..c98b9ee2e48ad 100644 --- a/src/vs/workbench/contrib/chat/browser/chatInputPart.ts +++ b/src/vs/workbench/contrib/chat/browser/chatInputPart.ts @@ -149,6 +149,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge // private readonly editorOptions: ChatEditorOptions, // TODO this should be used private readonly location: ChatAgentLocation, private readonly options: IChatInputPartOptions, + private readonly getInputState: () => any, @IChatWidgetHistoryService private readonly historyService: IChatWidgetHistoryService, @IModelService private readonly modelService: IModelService, @IInstantiationService private readonly instantiationService: IInstantiationService, @@ -234,11 +235,12 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge } showPreviousValue(): void { + const inputState = this.getInputState(); if (this.history.isAtEnd()) { - this.saveCurrentValue(); + this.saveCurrentValue(inputState); } else { - if (!this.history.has({ text: this._inputEditor.getValue(), state: this.history.current().state })) { - this.saveCurrentValue(); + if (!this.history.has({ text: this._inputEditor.getValue(), state: inputState })) { + this.saveCurrentValue(inputState); this.history.resetCursor(); } } @@ -247,11 +249,12 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge } showNextValue(): void { + const inputState = this.getInputState(); if (this.history.isAtEnd()) { return; } else { - if (!this.history.has({ text: this._inputEditor.getValue(), state: this.history.current().state })) { - this.saveCurrentValue(); + if (!this.history.has({ text: this._inputEditor.getValue(), state: inputState })) { + this.saveCurrentValue(inputState); this.history.resetCursor(); } } @@ -288,12 +291,12 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge this.inputEditor.setPosition({ lineNumber: 1, column: value.length + 1 }); if (!transient) { - this.saveCurrentValue(); + this.saveCurrentValue(this.getInputState()); } } - private saveCurrentValue(): void { - const newEntry = { text: this._inputEditor.getValue(), state: this.history.current().state }; + private saveCurrentValue(inputState: any): void { + const newEntry = { text: this._inputEditor.getValue(), state: inputState }; this.history.replaceLast(newEntry); } @@ -312,11 +315,13 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge async acceptInput(isUserQuery?: boolean): Promise { if (isUserQuery) { const userQuery = this._inputEditor.getValue(); - const entry: IChatHistoryEntry = { text: userQuery, state: this.history.current().state }; + const entry: IChatHistoryEntry = { text: userQuery, state: this.getInputState() }; this.history.replaceLast(entry); this.history.add({ text: '' }); } + // Clear attached context, fire event to clear input state, and clear the input editor + this._attachedContext.clear(); this._onDidLoadInputState.fire({}); if (this.accessibilityService.isScreenReaderOptimized() && isMacintosh) { this._acceptInputForVoiceover(); @@ -339,14 +344,6 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge this._inputEditor.focus(); } - clearContext(): void { - if (this._attachedContext.size > 0) { - const removed = Array.from(this._attachedContext); - this._attachedContext.clear(); - this._onDidChangeContext.fire({ removed }); - } - } - attachContext(overwrite: boolean, ...contentReferences: IChatRequestVariableEntry[]): void { const removed = []; if (overwrite) { @@ -638,6 +635,7 @@ export class ChatInputPart extends Disposable implements IHistoryNavigationWidge } saveState(): void { + this.saveCurrentValue(this.getInputState()); const inputHistory = [...this.history]; this.historyService.saveHistory(this.location, inputHistory); } diff --git a/src/vs/workbench/contrib/chat/browser/chatViewPane.ts b/src/vs/workbench/contrib/chat/browser/chatViewPane.ts index 80022029e4079..0cb92990dff02 100644 --- a/src/vs/workbench/contrib/chat/browser/chatViewPane.ts +++ b/src/vs/workbench/contrib/chat/browser/chatViewPane.ts @@ -99,7 +99,7 @@ export class ChatViewPane extends ViewPane { }; } - private updateModel(model?: IChatModel | undefined, viewState?: IViewPaneState): void { + private updateModel(model?: IChatModel | undefined): void { this.modelDisposables.clear(); model = model ?? (this.chatService.transferredSessionData?.sessionId @@ -109,7 +109,7 @@ export class ChatViewPane extends ViewPane { throw new Error('Could not start chat session'); } - this._widget.setModel(model, { ...(viewState ?? this.viewState) }); + this._widget.setModel(model, { ...this.viewState }); this.viewState.sessionId = model.sessionId; } @@ -179,7 +179,10 @@ export class ChatViewPane extends ViewPane { if (this.widget.viewModel) { this.chatService.clearSession(this.widget.viewModel.sessionId); } - this.updateModel(undefined, { ...this.viewState, inputValue: undefined }); + + // Grab the widget's latest view state because it will be loaded back into the widget + this.updateViewState(); + this.updateModel(undefined); } loadSession(sessionId: string): void { @@ -211,12 +214,16 @@ export class ChatViewPane extends ViewPane { // TODO multiple chat views will overwrite each other this._widget.saveState(); - const widgetViewState = this._widget.getViewState(); - this.viewState.inputValue = widgetViewState.inputValue; - this.viewState.inputState = widgetViewState.inputState; + this.updateViewState(); this.memento.saveMemento(); } super.saveState(); } + + private updateViewState(): void { + const widgetViewState = this._widget.getViewState(); + this.viewState.inputValue = widgetViewState.inputValue; + this.viewState.inputState = widgetViewState.inputState; + } } diff --git a/src/vs/workbench/contrib/chat/browser/chatWidget.ts b/src/vs/workbench/contrib/chat/browser/chatWidget.ts index c9c67a5129b0e..58e75ca258cd7 100644 --- a/src/vs/workbench/contrib/chat/browser/chatWidget.ts +++ b/src/vs/workbench/contrib/chat/browser/chatWidget.ts @@ -69,8 +69,6 @@ export interface IChatWidgetContrib extends IDisposable { */ getInputState?(): any; - onDidChangeInputState?: Event; - /** * Called with the result of getInputState when navigating input history. */ @@ -337,15 +335,6 @@ export class ChatWidget extends Disposable implements IChatWidget { return undefined; } }).filter(isDefined); - - this.contribs.forEach(c => { - if (c.onDidChangeInputState) { - this._register(c.onDidChangeInputState(() => { - const state = this.collectInputState(); - this.inputPart.updateState(state); - })); - } - }); } getContrib(id: string): T | undefined { @@ -586,7 +575,8 @@ export class ChatWidget extends Disposable implements IChatWidget { renderStyle: options?.renderStyle === 'minimal' ? 'compact' : options?.renderStyle, menus: { executeToolbar: MenuId.ChatExecute, ...this.viewOptions.menus }, editorOverflowWidgetsDomNode: this.viewOptions.editorOverflowWidgetsDomNode, - } + }, + () => this.collectInputState() )); this.inputPart.render(container, '', this); @@ -788,8 +778,6 @@ export class ChatWidget extends Disposable implements IChatWidget { if (result) { this.inputPart.acceptInput(isUserQuery); this._onDidSubmitAgent.fire({ agent: result.agent, slashCommand: result.slashCommand }); - this.inputPart.updateState(this.collectInputState()); - this.inputPart.clearContext(); result.responseCompletePromise.then(() => { const responses = this.viewModel?.getItems().filter(isResponseVM); const lastResponse = responses?.[responses.length - 1]; @@ -801,7 +789,6 @@ export class ChatWidget extends Disposable implements IChatWidget { return undefined; } - setContext(overwrite: boolean, ...contentReferences: IChatRequestVariableEntry[]) { this.inputPart.attachContext(overwrite, ...contentReferences); } @@ -964,7 +951,6 @@ export class ChatWidget extends Disposable implements IChatWidget { } getViewState(): IChatViewState { - this.inputPart.saveState(); return { inputValue: this.getInput(), inputState: this.collectInputState() }; } } diff --git a/src/vs/workbench/contrib/chat/browser/contrib/chatContextAttachments.ts b/src/vs/workbench/contrib/chat/browser/contrib/chatContextAttachments.ts index 8886027d53435..c19bc95305ff5 100644 --- a/src/vs/workbench/contrib/chat/browser/contrib/chatContextAttachments.ts +++ b/src/vs/workbench/contrib/chat/browser/contrib/chatContextAttachments.ts @@ -3,7 +3,6 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import { Emitter } from 'vs/base/common/event'; import { Disposable } from 'vs/base/common/lifecycle'; import { IChatWidget } from 'vs/workbench/contrib/chat/browser/chat'; import { ChatWidget, IChatWidgetContrib } from 'vs/workbench/contrib/chat/browser/chatWidget'; @@ -13,9 +12,6 @@ export class ChatContextAttachments extends Disposable implements IChatWidgetCon private _attachedContext = new Set(); - private readonly _onDidChangeInputState = this._register(new Emitter()); - readonly onDidChangeInputState = this._onDidChangeInputState.event; - public static readonly ID = 'chatContextAttachments'; get id() { @@ -66,13 +62,11 @@ export class ChatContextAttachments extends Disposable implements IChatWidgetCon } this.widget.setContext(overwrite, ...attachments); - this._onDidChangeInputState.fire(); } private _removeContext(attachments: IChatRequestVariableEntry[]) { if (attachments.length) { attachments.forEach(this._attachedContext.delete, this._attachedContext); - this._onDidChangeInputState.fire(); } } diff --git a/src/vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables.ts b/src/vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables.ts index 0b5712a797985..7ecf788e9761d 100644 --- a/src/vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables.ts +++ b/src/vs/workbench/contrib/chat/browser/contrib/chatDynamicVariables.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import { coalesce } from 'vs/base/common/arrays'; -import { Emitter } from 'vs/base/common/event'; import { IMarkdownString, MarkdownString } from 'vs/base/common/htmlContent'; import { Disposable } from 'vs/base/common/lifecycle'; import { basename } from 'vs/base/common/resources'; @@ -39,9 +38,6 @@ export class ChatDynamicVariableModel extends Disposable implements IChatWidgetC return ChatDynamicVariableModel.ID; } - private _onDidChangeInputState = this._register(new Emitter()); - readonly onDidChangeInputState = this._onDidChangeInputState.event; - constructor( private readonly widget: IChatWidget, @ILabelService private readonly labelService: ILabelService, @@ -81,10 +77,6 @@ export class ChatDynamicVariableModel extends Disposable implements IChatWidgetC return ref; })); - - if (didModifyState) { - this._onDidChangeInputState.fire(); - } }); this.updateDecorations(); @@ -107,7 +99,6 @@ export class ChatDynamicVariableModel extends Disposable implements IChatWidgetC addReference(ref: IDynamicVariable): void { this._variables.push(ref); this.updateDecorations(); - this._onDidChangeInputState.fire(); } private updateDecorations(): void {