Skip to content

Commit 9209da7

Browse files
rebornixrebornix
committed
Ensure escape discard/accept changes properly.
1 parent fe89a8a commit 9209da7

File tree

4 files changed

+74
-16
lines changed

4 files changed

+74
-16
lines changed

‎src/vs/workbench/contrib/notebook/browser/controller/chat/cellChatActions.ts‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@ import { InputFocusedContextKey } from 'vs/platform/contextkey/common/contextkey
1414
import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation';
1515
import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
1616
import { CTX_INLINE_CHAT_FOCUSED, CTX_INLINE_CHAT_HAS_PROVIDER, CTX_INLINE_CHAT_INNER_CURSOR_FIRST, CTX_INLINE_CHAT_INNER_CURSOR_LAST, CTX_INLINE_CHAT_LAST_RESPONSE_TYPE, CTX_INLINE_CHAT_RESPONSE_TYPES, InlineChatResponseFeedbackKind, InlineChatResponseTypes } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
17-
import { CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_NOTEBOOK_CHAT_HAS_ACTIVE_REQUEST, MENU_CELL_CHAT_INPUT, MENU_CELL_CHAT_WIDGET, MENU_CELL_CHAT_WIDGET_FEEDBACK, MENU_CELL_CHAT_WIDGET_STATUS } from 'vs/workbench/contrib/notebook/browser/controller/chat/notebookChatContext';
17+
import { CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_NOTEBOOK_CHAT_HAS_ACTIVE_REQUEST, CTX_NOTEBOOK_CHAT_USER_DID_EDIT, MENU_CELL_CHAT_INPUT, MENU_CELL_CHAT_WIDGET, MENU_CELL_CHAT_WIDGET_FEEDBACK, MENU_CELL_CHAT_WIDGET_STATUS } from 'vs/workbench/contrib/notebook/browser/controller/chat/notebookChatContext';
1818
import { NotebookChatController } from 'vs/workbench/contrib/notebook/browser/controller/chat/notebookChatController';
1919
import { INotebookActionContext, INotebookCellActionContext, NotebookAction, NotebookCellAction, getEditorFromArgsOrActivePane } from 'vs/workbench/contrib/notebook/browser/controller/coreActions';
2020
import { CellEditState } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
2121
import { CellKind, NOTEBOOK_EDITOR_CURSOR_BOUNDARY, NotebookSetting } from 'vs/workbench/contrib/notebook/common/notebookCommon';
22-
import { NOTEBOOK_CELL_LIST_FOCUSED, NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED } from 'vs/workbench/contrib/notebook/common/notebookContextKeys';
22+
import { NOTEBOOK_EDITOR_EDITABLE, NOTEBOOK_EDITOR_FOCUSED } from 'vs/workbench/contrib/notebook/common/notebookContextKeys';
2323

2424

2525
registerAction2(class extends NotebookAction {
@@ -222,11 +222,18 @@ registerAction2(class extends NotebookAction {
222222
shortTitle: localize('apply2', 'Accept'),
223223
icon: Codicon.check,
224224
tooltip: localize('apply3', 'Accept Changes'),
225-
keybinding: {
226-
when: ContextKeyExpr.and(CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_INLINE_CHAT_FOCUSED),
227-
weight: KeybindingWeight.EditorContrib + 10,
228-
primary: KeyMod.CtrlCmd | KeyCode.Enter,
229-
},
225+
keybinding: [
226+
{
227+
when: ContextKeyExpr.and(CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_INLINE_CHAT_FOCUSED),
228+
weight: KeybindingWeight.EditorContrib + 10,
229+
primary: KeyMod.CtrlCmd | KeyCode.Enter,
230+
},
231+
{
232+
when: ContextKeyExpr.and(CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_INLINE_CHAT_FOCUSED, CTX_NOTEBOOK_CHAT_USER_DID_EDIT),
233+
weight: KeybindingWeight.EditorCore + 10,
234+
primary: KeyCode.Escape
235+
}
236+
],
230237
menu: [
231238
{
232239
id: MENU_CELL_CHAT_WIDGET_STATUS,
@@ -251,7 +258,7 @@ registerAction2(class extends NotebookAction {
251258
title: localize('discard', 'Discard'),
252259
icon: Codicon.discard,
253260
keybinding: {
254-
when: ContextKeyExpr.and(CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_INLINE_CHAT_FOCUSED, NOTEBOOK_CELL_LIST_FOCUSED),
261+
when: ContextKeyExpr.and(CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_INLINE_CHAT_FOCUSED, CTX_NOTEBOOK_CHAT_USER_DID_EDIT.negate()),
255262
weight: KeybindingWeight.EditorContrib,
256263
primary: KeyCode.Escape
257264
},

‎src/vs/workbench/contrib/notebook/browser/controller/chat/notebookChatContext.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { RawContextKey } from 'vs/platform/contextkey/common/contextkey';
99

1010
export const CTX_NOTEBOOK_CELL_CHAT_FOCUSED = new RawContextKey<boolean>('notebookCellChatFocused', false, localize('notebookCellChatFocused', "Whether the cell chat editor is focused"));
1111
export const CTX_NOTEBOOK_CHAT_HAS_ACTIVE_REQUEST = new RawContextKey<boolean>('notebookChatHasActiveRequest', false, localize('notebookChatHasActiveRequest', "Whether the cell chat editor has an active request"));
12+
export const CTX_NOTEBOOK_CHAT_USER_DID_EDIT = new RawContextKey<boolean>('notebookChatUserDidEdit', false, localize('notebookChatUserDidEdit', "Whether the user did changes ontop of the notebook cell chat"));
1213
export const MENU_CELL_CHAT_INPUT = MenuId.for('cellChatInput');
1314
export const MENU_CELL_CHAT_WIDGET = MenuId.for('cellChatWidget');
1415
export const MENU_CELL_CHAT_WIDGET_STATUS = MenuId.for('cellChatWidget.status');

‎src/vs/workbench/contrib/notebook/browser/controller/chat/notebookChatController.ts‎

Lines changed: 55 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,12 @@ import { IInlineChatMessageAppender, InlineChatWidget } from 'vs/workbench/contr
4040
import { asProgressiveEdit, performAsyncTextEdit } from 'vs/workbench/contrib/inlineChat/browser/utils';
4141
import { CTX_INLINE_CHAT_LAST_RESPONSE_TYPE, EditMode, IInlineChatProgressItem, IInlineChatRequest, InlineChatResponseFeedbackKind, InlineChatResponseType } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
4242
import { insertCell, runDeleteAction } from 'vs/workbench/contrib/notebook/browser/controller/cellOperations';
43-
import { CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_NOTEBOOK_CHAT_HAS_ACTIVE_REQUEST, MENU_CELL_CHAT_INPUT, MENU_CELL_CHAT_WIDGET, MENU_CELL_CHAT_WIDGET_FEEDBACK, MENU_CELL_CHAT_WIDGET_STATUS } from 'vs/workbench/contrib/notebook/browser/controller/chat/notebookChatContext';
43+
import { CTX_NOTEBOOK_CELL_CHAT_FOCUSED, CTX_NOTEBOOK_CHAT_HAS_ACTIVE_REQUEST, CTX_NOTEBOOK_CHAT_USER_DID_EDIT, MENU_CELL_CHAT_INPUT, MENU_CELL_CHAT_WIDGET, MENU_CELL_CHAT_WIDGET_FEEDBACK, MENU_CELL_CHAT_WIDGET_STATUS } from 'vs/workbench/contrib/notebook/browser/controller/chat/notebookChatContext';
4444
import { INotebookEditor, INotebookEditorContribution, INotebookViewZone, ScrollToRevealBehavior } from 'vs/workbench/contrib/notebook/browser/notebookBrowser';
4545
import { registerNotebookContribution } from 'vs/workbench/contrib/notebook/browser/notebookEditorExtensions';
4646
import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModelImpl';
4747
import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon';
48+
import { INotebookExecutionStateService, NotebookExecutionType } from 'vs/workbench/contrib/notebook/common/notebookExecutionStateService';
4849

4950

5051

@@ -100,10 +101,21 @@ class NotebookChatWidget extends Disposable implements INotebookViewZone {
100101
this.inlineChatWidget.focus();
101102
}
102103

103-
async getEditingCellEditor() {
104+
getEditingCell() {
105+
return this._editingCell;
106+
}
107+
108+
async getOrCreateEditingCell(): Promise<{ cell: CellViewModel; editor: IActiveCodeEditor } | undefined> {
104109
if (this._editingCell) {
105110
await this._notebookEditor.focusNotebookCell(this._editingCell, 'editor');
106-
return this._notebookEditor.activeCodeEditor;
111+
if (this._notebookEditor.activeCodeEditor?.hasModel()) {
112+
return {
113+
cell: this._editingCell,
114+
editor: this._notebookEditor.activeCodeEditor
115+
};
116+
} else {
117+
return undefined;
118+
}
107119
}
108120

109121
if (!this._notebookEditor.hasModel()) {
@@ -117,7 +129,14 @@ class NotebookChatWidget extends Disposable implements INotebookViewZone {
117129
}
118130

119131
await this._notebookEditor.focusNotebookCell(this._editingCell, 'editor', { revealBehavior: ScrollToRevealBehavior.firstLine });
120-
return this._notebookEditor.activeCodeEditor;
132+
if (this._notebookEditor.activeCodeEditor?.hasModel()) {
133+
return {
134+
cell: this._editingCell,
135+
editor: this._notebookEditor.activeCodeEditor
136+
};
137+
}
138+
139+
return undefined;
121140
}
122141

123142
async discardChange() {
@@ -160,6 +179,8 @@ export class NotebookChatController extends Disposable implements INotebookEdito
160179
private _activeSession?: Session;
161180
private readonly _ctxHasActiveRequest: IContextKey<boolean>;
162181
private readonly _ctxCellWidgetFocused: IContextKey<boolean>;
182+
private readonly _ctxUserDidEdit: IContextKey<boolean>;
183+
private readonly _userEditingDisposables = this._register(new DisposableStore());
163184
private readonly _ctxLastResponseType: IContextKey<undefined | InlineChatResponseType>;
164185
private _widget: NotebookChatWidget | undefined;
165186
private _widgetDisposableStore = this._register(new DisposableStore());
@@ -174,12 +195,14 @@ export class NotebookChatController extends Disposable implements INotebookEdito
174195
@IInlineChatSavingService private readonly _inlineChatSavingService: IInlineChatSavingService,
175196
@IModelService private readonly _modelService: IModelService,
176197
@ILanguageService private readonly _languageService: ILanguageService,
198+
@INotebookExecutionStateService private _executionStateService: INotebookExecutionStateService,
177199

178200
) {
179201
super();
180202
this._ctxHasActiveRequest = CTX_NOTEBOOK_CHAT_HAS_ACTIVE_REQUEST.bindTo(this._contextKeyService);
181203
this._ctxCellWidgetFocused = CTX_NOTEBOOK_CELL_CHAT_FOCUSED.bindTo(this._contextKeyService);
182204
this._ctxLastResponseType = CTX_INLINE_CHAT_LAST_RESPONSE_TYPE.bindTo(this._contextKeyService);
205+
this._ctxUserDidEdit = CTX_NOTEBOOK_CHAT_USER_DID_EDIT.bindTo(this._contextKeyService);
183206
}
184207

185208
run(index: number, input: string | undefined, autoSend: boolean | undefined): void {
@@ -270,8 +293,9 @@ export class NotebookChatController extends Disposable implements INotebookEdito
270293
this._languageService
271294
);
272295

296+
this._ctxCellWidgetFocused.set(true);
297+
273298
disposableTimeout(() => {
274-
this._ctxCellWidgetFocused.set(true);
275299
this._focusWidget();
276300
}, 0, this._store);
277301

@@ -476,6 +500,22 @@ export class NotebookChatController extends Disposable implements INotebookEdito
476500
});
477501
}
478502
}
503+
504+
this._userEditingDisposables.clear();
505+
// monitor user edits
506+
const editingCell = this._widget.getEditingCell();
507+
if (editingCell) {
508+
this._userEditingDisposables.add(editingCell.model.onDidChangeContent(() => this._updateUserEditingState()));
509+
this._userEditingDisposables.add(editingCell.model.onDidChangeLanguage(() => this._updateUserEditingState()));
510+
this._userEditingDisposables.add(editingCell.model.onDidChangeMetadata(() => this._updateUserEditingState()));
511+
this._userEditingDisposables.add(editingCell.model.onDidChangeInternalMetadata(() => this._updateUserEditingState()));
512+
this._userEditingDisposables.add(editingCell.model.onDidChangeOutputs(() => this._updateUserEditingState()));
513+
this._userEditingDisposables.add(this._executionStateService.onDidChangeExecution(e => {
514+
if (e.type === NotebookExecutionType.cell && e.affectsCell(editingCell.uri)) {
515+
this._updateUserEditingState();
516+
}
517+
}));
518+
}
479519
}
480520
} catch (e) {
481521
response = new ErrorResponse(e);
@@ -519,12 +559,14 @@ export class NotebookChatController extends Disposable implements INotebookEdito
519559
assertType(this._strategy);
520560
assertType(this._widget);
521561

522-
const editor = await this._widget.getEditingCellEditor();
562+
const editingCell = await this._widget.getOrCreateEditingCell();
523563

524-
if (!editor || !editor.hasModel()) {
564+
if (!editingCell) {
525565
return;
526566
}
527567

568+
const editor = editingCell.editor;
569+
528570
const moreMinimalEdits = await this._editorWorkerService.computeMoreMinimalEdits(editor.getModel().uri, edits);
529571
// this._log('edits from PROVIDER and after making them MORE MINIMAL', this._activeSession.provider.debugName, edits, moreMinimalEdits);
530572

@@ -551,6 +593,10 @@ export class NotebookChatController extends Disposable implements INotebookEdito
551593
}
552594
}
553595

596+
private _updateUserEditingState() {
597+
this._ctxUserDidEdit.set(true);
598+
}
599+
554600
async acceptSession() {
555601
assertType(this._activeSession);
556602
assertType(this._strategy);
@@ -615,6 +661,7 @@ export class NotebookChatController extends Disposable implements INotebookEdito
615661
discard() {
616662
this._strategy?.cancel();
617663
this._widget?.discardChange();
664+
this.dismiss();
618665
}
619666

620667
async feedbackLast(kind: InlineChatResponseFeedbackKind) {
@@ -627,6 +674,7 @@ export class NotebookChatController extends Disposable implements INotebookEdito
627674

628675
dismiss() {
629676
this._ctxCellWidgetFocused.set(false);
677+
this._ctxUserDidEdit.set(false);
630678
this._sessionCtor?.cancel();
631679
this._sessionCtor = undefined;
632680
this._widget?.dispose();

‎src/vs/workbench/contrib/notebook/browser/controller/editActions.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { IDialogService, IConfirmationResult } from 'vs/platform/dialogs/common/
3232
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
3333
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
3434
import { InlineChatController } from 'vs/workbench/contrib/inlineChat/browser/inlineChatController';
35+
import { CTX_INLINE_CHAT_FOCUSED } from 'vs/workbench/contrib/inlineChat/common/inlineChat';
3536

3637

3738
const CLEAR_ALL_CELLS_OUTPUTS_COMMAND_ID = 'notebook.clearAllCellsOutputs';
@@ -85,7 +86,8 @@ registerAction2(class EditCellAction extends NotebookCellAction {
8586

8687
const quitEditCondition = ContextKeyExpr.and(
8788
NOTEBOOK_EDITOR_FOCUSED,
88-
InputFocusedContext
89+
InputFocusedContext,
90+
CTX_INLINE_CHAT_FOCUSED.toNegated()
8991
);
9092
registerAction2(class QuitEditCellAction extends NotebookCellAction {
9193
constructor() {

0 commit comments

Comments
 (0)