Skip to content

Commit 3e4ed47

Browse files
jriekenjrieken
authored
Merge pull request microsoft#182619 from microsoft/joh/stiff-crane
joh/stiff crane
2 parents 1236c13 + 4775bd5 commit 3e4ed47

File tree

5 files changed

+72
-67
lines changed

5 files changed

+72
-67
lines changed

‎src/vs/workbench/contrib/interactiveEditor/browser/interactiveEditorActions.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { EditorAction2 } from 'vs/editor/browser/editorExtensions';
1010
import { EmbeddedCodeEditorWidget, EmbeddedDiffEditorWidget } from 'vs/editor/browser/widget/embeddedCodeEditorWidget';
1111
import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
1212
import { InteractiveEditorController, InteractiveEditorRunOptions } from 'vs/workbench/contrib/interactiveEditor/browser/interactiveEditorController';
13-
import { CTX_INTERACTIVE_EDITOR_FOCUSED, CTX_INTERACTIVE_EDITOR_HAS_ACTIVE_REQUEST, CTX_INTERACTIVE_EDITOR_HAS_PROVIDER, CTX_INTERACTIVE_EDITOR_INNER_CURSOR_FIRST, CTX_INTERACTIVE_EDITOR_INNER_CURSOR_LAST, CTX_INTERACTIVE_EDITOR_EMPTY, CTX_INTERACTIVE_EDITOR_OUTER_CURSOR_POSITION, CTX_INTERACTIVE_EDITOR_VISIBLE, MENU_INTERACTIVE_EDITOR_WIDGET, MENU_INTERACTIVE_EDITOR_WIDGET_DISCARD, MENU_INTERACTIVE_EDITOR_WIDGET_STATUS, CTX_INTERACTIVE_EDITOR_LAST_FEEDBACK, CTX_INTERACTIVE_EDITOR_INLNE_DIFF, CTX_INTERACTIVE_EDITOR_EDIT_MODE, EditMode, CTX_INTERACTIVE_EDITOR_LAST_RESPONSE_TYPE, MENU_INTERACTIVE_EDITOR_WIDGET_MARKDOWN_MESSAGE, CTX_INTERACTIVE_EDITOR_MESSAGE_CROP_STATE, CTX_INTERACTIVE_EDITOR_DOCUMENT_CHANGED, CTX_INTERACTIVE_EDITOR_DID_EDIT } from 'vs/workbench/contrib/interactiveEditor/common/interactiveEditor';
13+
import { CTX_INTERACTIVE_EDITOR_FOCUSED, CTX_INTERACTIVE_EDITOR_HAS_ACTIVE_REQUEST, CTX_INTERACTIVE_EDITOR_HAS_PROVIDER, CTX_INTERACTIVE_EDITOR_INNER_CURSOR_FIRST, CTX_INTERACTIVE_EDITOR_INNER_CURSOR_LAST, CTX_INTERACTIVE_EDITOR_EMPTY, CTX_INTERACTIVE_EDITOR_OUTER_CURSOR_POSITION, CTX_INTERACTIVE_EDITOR_VISIBLE, MENU_INTERACTIVE_EDITOR_WIDGET, MENU_INTERACTIVE_EDITOR_WIDGET_DISCARD, MENU_INTERACTIVE_EDITOR_WIDGET_STATUS, CTX_INTERACTIVE_EDITOR_LAST_FEEDBACK, CTX_INTERACTIVE_EDITOR_SHOWING_DIFF, CTX_INTERACTIVE_EDITOR_EDIT_MODE, EditMode, CTX_INTERACTIVE_EDITOR_LAST_RESPONSE_TYPE, MENU_INTERACTIVE_EDITOR_WIDGET_MARKDOWN_MESSAGE, CTX_INTERACTIVE_EDITOR_MESSAGE_CROP_STATE, CTX_INTERACTIVE_EDITOR_DOCUMENT_CHANGED, CTX_INTERACTIVE_EDITOR_DID_EDIT } from 'vs/workbench/contrib/interactiveEditor/common/interactiveEditor';
1414
import { localize } from 'vs/nls';
1515
import { IAction2Options } from 'vs/platform/actions/common/actions';
1616
import { IClipboardService } from 'vs/platform/clipboard/common/clipboardService';
@@ -374,22 +374,22 @@ export class ToggleInlineDiff extends AbstractInteractiveEditorAction {
374374

375375
constructor() {
376376
super({
377-
id: 'interactiveEditor.toggleInlineDiff',
378-
title: localize('toggleInlineDiff', 'Toggle Inline Diff'),
377+
id: 'interactiveEditor.toggleDiff',
378+
title: localize('toggleDiff', 'Toggle Diff'),
379379
icon: Codicon.diff,
380380
precondition: CTX_INTERACTIVE_EDITOR_VISIBLE,
381-
toggled: CTX_INTERACTIVE_EDITOR_INLNE_DIFF,
381+
toggled: CTX_INTERACTIVE_EDITOR_SHOWING_DIFF,
382382
menu: {
383383
id: MENU_INTERACTIVE_EDITOR_WIDGET_STATUS,
384-
when: ContextKeyExpr.and(CTX_INTERACTIVE_EDITOR_EDIT_MODE.isEqualTo(EditMode.Live), CTX_INTERACTIVE_EDITOR_LAST_RESPONSE_TYPE.notEqualsTo('message')),
385-
group: '1_main',
386-
order: 1
384+
when: ContextKeyExpr.and(CTX_INTERACTIVE_EDITOR_EDIT_MODE.notEqualsTo(EditMode.Preview), CTX_INTERACTIVE_EDITOR_DID_EDIT),
385+
group: '0_main',
386+
order: 10
387387
}
388388
});
389389
}
390390

391391
override runInteractiveEditorCommand(_accessor: ServicesAccessor, ctrl: InteractiveEditorController): void {
392-
ctrl.toggleInlineDiff();
392+
ctrl.toggleDiff();
393393
}
394394
}
395395

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -131,8 +131,8 @@ export class InteractiveEditorController implements IEditorContribution {
131131
}
132132

133133
dispose(): void {
134+
this._finishExistingSession();
134135
this._store.dispose();
135-
this.cancelSession();
136136
}
137137

138138
getId(): string {
@@ -145,7 +145,6 @@ export class InteractiveEditorController implements IEditorContribution {
145145
if (this._accessibilityService.isScreenReaderOptimized() && editModeValue === editMode.defaultValue) {
146146
// By default, use preview mode for screen reader users
147147
editModeValue = EditMode.Preview;
148-
this._configurationService.updateValue('interactiveEditor.editMode', EditMode.Preview);
149148
}
150149
return editModeValue!;
151150
}
@@ -156,19 +155,19 @@ export class InteractiveEditorController implements IEditorContribution {
156155

157156
async run(options: InteractiveEditorRunOptions | undefined): Promise<void> {
158157
this._logService.trace('[IE] session starting');
159-
await this._finishOrCancel();
158+
await this._finishExistingSession();
160159

161160
await this._nextState(State.CREATE_SESSION, { ...options });
162161
this._logService.trace('[IE] session done or paused');
163162
}
164163

165-
private async _finishOrCancel(): Promise<void> {
164+
private async _finishExistingSession(): Promise<void> {
166165
if (this._activeSession) {
167166
if (this._activeSession.editMode === EditMode.Preview) {
168-
this._logService.trace('[IE] an EXISTING session is active, cancelling first');
167+
this._logService.trace('[IE] finishing existing session, using CANCEL', this._activeSession.editMode);
169168
await this.cancelSession();
170169
} else {
171-
this._logService.trace('[IE] an EXISTING session is active, finishing first');
170+
this._logService.trace('[IE] finishing existing session, using APPLY', this._activeSession.editMode);
172171
await this.applyChanges();
173172
}
174173
}
@@ -292,7 +291,7 @@ export class InteractiveEditorController implements IEditorContribution {
292291
// cancel all sibling sessions
293292
for (const editor of editors) {
294293
if (editor !== this._editor) {
295-
InteractiveEditorController.get(editor)?._finishOrCancel();
294+
InteractiveEditorController.get(editor)?._finishExistingSession();
296295
}
297296
}
298297
break;
@@ -507,7 +506,7 @@ export class InteractiveEditorController implements IEditorContribution {
507506

508507
try {
509508
this._ignoreModelContentChanged = true;
510-
await this._strategy.renderChanges(response, this._activeSession.lastTextModelChanges);
509+
await this._strategy.renderChanges(response);
511510
} finally {
512511
this._ignoreModelContentChanged = false;
513512
}
@@ -561,8 +560,8 @@ export class InteractiveEditorController implements IEditorContribution {
561560
}
562561
}
563562

564-
toggleInlineDiff(): void {
565-
this._strategy?.toggleInlineDiff();
563+
toggleDiff(): void {
564+
this._strategy?.toggleDiff();
566565
}
567566

568567
focus(): void {

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

Lines changed: 16 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Dimension, h } from 'vs/base/browser/dom';
7-
import { DisposableStore, MutableDisposable } from 'vs/base/common/lifecycle';
7+
import { MutableDisposable } from 'vs/base/common/lifecycle';
88
import { assertType } from 'vs/base/common/types';
99
import { ICodeEditor, IDiffEditor } from 'vs/editor/browser/editorBrowser';
1010
import { EmbeddedCodeEditorWidget, EmbeddedDiffEditorWidget } from 'vs/editor/browser/widget/embeddedCodeEditorWidget';
@@ -30,6 +30,7 @@ import { TextEdit } from 'vs/editor/common/languages';
3030
import { FileKind } from 'vs/platform/files/common/files';
3131
import { IModelService } from 'vs/editor/common/services/model';
3232
import { EditOperation } from 'vs/editor/common/core/editOperation';
33+
import { Session } from 'vs/workbench/contrib/interactiveEditor/browser/interactiveEditorSession';
3334

3435
export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
3536

@@ -39,12 +40,12 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
3940

4041
private readonly _diffEditor: IDiffEditor;
4142
private readonly _inlineDiffDecorations: IEditorDecorationsCollection;
42-
private readonly _sessionStore = this._disposables.add(new DisposableStore());
4343
private _dim: Dimension | undefined;
44+
private _isVisible: boolean = false;
4445

4546
constructor(
4647
editor: ICodeEditor,
47-
private readonly _textModelv0: ITextModel,
48+
private readonly _session: Session,
4849
@IInstantiationService instantiationService: IInstantiationService,
4950
@IThemeService themeService: IThemeService,
5051
@ILogService private readonly _logService: ILogService,
@@ -80,7 +81,7 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
8081
modifiedEditor: { contributions: diffContributions }
8182
}, editor);
8283
this._disposables.add(this._diffEditor);
83-
this._diffEditor.setModel({ original: this._textModelv0, modified: editor.getModel() });
84+
this._diffEditor.setModel({ original: this._session.textModel0, modified: editor.getModel() });
8485

8586
const doStyle = () => {
8687
const theme = themeService.getColorTheme();
@@ -115,40 +116,31 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
115116

116117
// --- show / hide --------------------
117118

119+
get isVisible(): boolean {
120+
return this._isVisible;
121+
}
122+
118123
override hide(): void {
119124
this._cleanupFullDiff();
120125
this._cleanupInlineDiff();
121-
this._sessionStore.clear();
122126
super.hide();
127+
this._isVisible = false;
123128
}
124129

125130
override show(): void {
126131
throw new Error('not supported like this, use showDiff');
127132
}
128133

129-
showDiff(range: () => Range, changes: LineRangeMapping[]): void {
134+
showDiff(): void {
130135
assertType(this.editor.hasModel());
131-
this._sessionStore.clear();
132-
133-
this._sessionStore.add(this._diffEditor.onDidUpdateDiff(() => {
134-
const result = this._diffEditor.getDiffComputationResult();
135-
const hasFocus = this._diffEditor.hasTextFocus();
136-
this._updateFromChanges(range(), result?.changes2 ?? []);
137-
// TODO@jrieken find a better fix for this. this is the challenge:
138-
// the _doShowForChanges method invokes show of the zone widget which removes and adds the
139-
// zone and overlay parts. this dettaches and reattaches the dom nodes which means they lose
140-
// focus
141-
if (hasFocus) {
142-
this._diffEditor.focus();
143-
}
144-
}));
145-
this._updateFromChanges(range(), changes);
136+
this._updateFromChanges(this._session.wholeRange, this._session.lastTextModelChanges);
137+
this._isVisible = true;
146138
}
147139

148140
private _updateFromChanges(range: Range, changes: LineRangeMapping[]): void {
149141
assertType(this.editor.hasModel());
150142

151-
if (changes.length === 0 || this._textModelv0.getValueLength() === 0) {
143+
if (changes.length === 0 || this._session.textModel0.getValueLength() === 0) {
152144
// no change or changes to an empty file
153145
this._logService.debug('[IE] livePreview-mode: no diff');
154146
this.hide();
@@ -170,7 +162,7 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
170162
// --- inline diff
171163

172164
private _renderChangesWithInlineDiff(changes: LineRangeMapping[]) {
173-
const original = this._textModelv0;
165+
const original = this._session.textModel0;
174166

175167
const decorations: IModelDeltaDecoration[] = [];
176168

@@ -267,7 +259,7 @@ export class InteractiveEditorLivePreviewWidget extends ZoneWidget {
267259
originalLineRange = new LineRange(originalLineRange.startLineNumber, originalLineRange.endLineNumberExclusive + endDelta);
268260
}
269261

270-
const originalDiffHidden = invertLineRange(originalLineRange, this._textModelv0);
262+
const originalDiffHidden = invertLineRange(originalLineRange, this._session.textModel0);
271263
const modifiedDiffHidden = invertLineRange(modifiedLineRange, model);
272264

273265
return {

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

Lines changed: 38 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,11 @@ import { IDisposable } from 'vs/base/common/lifecycle';
88
import 'vs/css!./interactiveEditor';
99
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
1010
import { IBulkEditService } from 'vs/editor/browser/services/bulkEditService';
11+
import { StableEditorScrollState } from 'vs/editor/browser/stableEditorScroll';
1112
import { EditOperation, ISingleEditOperation } from 'vs/editor/common/core/editOperation';
1213
import { Position } from 'vs/editor/common/core/position';
1314
import { Range } from 'vs/editor/common/core/range';
1415
import { Selection } from 'vs/editor/common/core/selection';
15-
import { LineRangeMapping } from 'vs/editor/common/diff/linesDiffComputer';
1616
import { IEditorDecorationsCollection } from 'vs/editor/common/editorCommon';
1717
import { ICursorStateComputer, IModelDecorationOptions, IModelDeltaDecoration, IValidEditOperation } from 'vs/editor/common/model';
1818
import { IEditorWorkerService } from 'vs/editor/common/services/editorWorker';
@@ -24,7 +24,7 @@ import { InteractiveEditorFileCreatePreviewWidget, InteractiveEditorLivePreviewW
2424
import { EditResponse, Session } from 'vs/workbench/contrib/interactiveEditor/browser/interactiveEditorSession';
2525
import { InteractiveEditorWidget } from 'vs/workbench/contrib/interactiveEditor/browser/interactiveEditorWidget';
2626
import { getValueFromSnapshot } from 'vs/workbench/contrib/interactiveEditor/browser/utils';
27-
import { CTX_INTERACTIVE_EDITOR_INLNE_DIFF, CTX_INTERACTIVE_EDITOR_DOCUMENT_CHANGED } from 'vs/workbench/contrib/interactiveEditor/common/interactiveEditor';
27+
import { CTX_INTERACTIVE_EDITOR_SHOWING_DIFF, CTX_INTERACTIVE_EDITOR_DOCUMENT_CHANGED } from 'vs/workbench/contrib/interactiveEditor/common/interactiveEditor';
2828
import { IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService';
2929

3030
export abstract class EditModeStrategy {
@@ -39,9 +39,9 @@ export abstract class EditModeStrategy {
3939

4040
abstract makeChanges(response: EditResponse, edits: ISingleEditOperation[]): Promise<void>;
4141

42-
abstract renderChanges(response: EditResponse, changes: LineRangeMapping[]): Promise<void>;
42+
abstract renderChanges(response: EditResponse): Promise<void>;
4343

44-
abstract toggleInlineDiff(): void;
44+
abstract toggleDiff(): void;
4545
}
4646

4747
export class PreviewStrategy extends EditModeStrategy {
@@ -111,10 +111,10 @@ export class PreviewStrategy extends EditModeStrategy {
111111
// nothing to do
112112
}
113113

114-
override async renderChanges(response: EditResponse, changes: LineRangeMapping[]): Promise<void> {
114+
override async renderChanges(response: EditResponse): Promise<void> {
115115
if (response.localEdits.length > 0) {
116116
const edits = response.localEdits.map(edit => EditOperation.replace(Range.lift(edit.range), edit.text));
117-
this._widget.showEditsPreview(this._session.textModel0, edits, changes);
117+
this._widget.showEditsPreview(this._session.textModel0, edits, this._session.lastTextModelChanges);
118118
} else {
119119
this._widget.hideEditsPreview();
120120
}
@@ -126,7 +126,9 @@ export class PreviewStrategy extends EditModeStrategy {
126126
}
127127
}
128128

129-
toggleInlineDiff(): void { }
129+
toggleDiff(): void {
130+
// nothing to do
131+
}
130132
}
131133

132134
class InlineDiffDecorations {
@@ -202,7 +204,7 @@ export class LiveStrategy extends EditModeStrategy {
202204
private _inlineDiffEnabled: boolean = false;
203205

204206
private readonly _inlineDiffDecorations: InlineDiffDecorations;
205-
private readonly _ctxInlineDiff: IContextKey<boolean>;
207+
protected readonly _ctxShowingDiff: IContextKey<boolean>;
206208
private _lastResponse?: EditResponse;
207209
private _editCount: number = 0;
208210

@@ -218,25 +220,25 @@ export class LiveStrategy extends EditModeStrategy {
218220
) {
219221
super();
220222
this._inlineDiffDecorations = new InlineDiffDecorations(this._editor, this._inlineDiffEnabled);
221-
this._ctxInlineDiff = CTX_INTERACTIVE_EDITOR_INLNE_DIFF.bindTo(contextKeyService);
223+
this._ctxShowingDiff = CTX_INTERACTIVE_EDITOR_SHOWING_DIFF.bindTo(contextKeyService);
222224

223225
this._inlineDiffEnabled = _storageService.getBoolean(LiveStrategy._inlineDiffStorageKey, StorageScope.PROFILE, false);
224-
this._ctxInlineDiff.set(this._inlineDiffEnabled);
226+
this._ctxShowingDiff.set(this._inlineDiffEnabled);
225227
this._inlineDiffDecorations.visible = this._inlineDiffEnabled;
226228
}
227229

228230
override dispose(): void {
229231
this._inlineDiffEnabled = this._inlineDiffDecorations.visible;
230232
this._storageService.store(LiveStrategy._inlineDiffStorageKey, this._inlineDiffEnabled, StorageScope.PROFILE, StorageTarget.USER);
231233
this._inlineDiffDecorations.clear();
232-
this._ctxInlineDiff.reset();
234+
this._ctxShowingDiff.reset();
233235

234236
super.dispose();
235237
}
236238

237-
toggleInlineDiff(): void {
239+
toggleDiff(): void {
238240
this._inlineDiffEnabled = !this._inlineDiffEnabled;
239-
this._ctxInlineDiff.set(this._inlineDiffEnabled);
241+
this._ctxShowingDiff.set(this._inlineDiffEnabled);
240242
this._inlineDiffDecorations.visible = this._inlineDiffEnabled;
241243
this._storageService.store(LiveStrategy._inlineDiffStorageKey, this._inlineDiffEnabled, StorageScope.PROFILE, StorageTarget.USER);
242244
}
@@ -298,10 +300,10 @@ export class LiveStrategy extends EditModeStrategy {
298300
this._editor.executeEdits('interactive-editor-live', edits, ignoreInlineDiff ? undefined : cursorStateComputerAndInlineDiffCollection);
299301
}
300302

301-
override async renderChanges(response: EditResponse, textModel0Changes: LineRangeMapping[]) {
303+
override async renderChanges(response: EditResponse) {
302304

303305
this._inlineDiffDecorations.update();
304-
this._updateSummaryMessage(textModel0Changes);
306+
this._updateSummaryMessage();
305307

306308
if (response.singleCreateFileEdit) {
307309
this._widget.showCreatePreview(response.singleCreateFileEdit.uri, await Promise.all(response.singleCreateFileEdit.edits));
@@ -310,12 +312,10 @@ export class LiveStrategy extends EditModeStrategy {
310312
}
311313
}
312314

313-
protected _updateSummaryMessage(textModel0Changes: LineRangeMapping[]) {
315+
protected _updateSummaryMessage() {
314316
let linesChanged = 0;
315-
if (textModel0Changes) {
316-
for (const change of textModel0Changes) {
317-
linesChanged += change.changedLineCount;
318-
}
317+
for (const change of this._session.lastTextModelChanges) {
318+
linesChanged += change.changedLineCount;
319319
}
320320
let message: string;
321321
if (linesChanged === 0) {
@@ -346,7 +346,7 @@ export class LivePreviewStrategy extends LiveStrategy {
346346
) {
347347
super(session, editor, widget, contextKeyService, storageService, bulkEditService, editorWorkerService, instaService);
348348

349-
this._diffZone = instaService.createInstance(InteractiveEditorLivePreviewWidget, editor, session.textModel0);
349+
this._diffZone = instaService.createInstance(InteractiveEditorLivePreviewWidget, editor, session);
350350
this._previewZone = instaService.createInstance(InteractiveEditorFileCreatePreviewWidget, editor);
351351
}
352352

@@ -362,17 +362,31 @@ export class LivePreviewStrategy extends LiveStrategy {
362362
super.makeChanges(_response, edits, true);
363363
}
364364

365-
override async renderChanges(response: EditResponse, changes: LineRangeMapping[]) {
365+
override async renderChanges(response: EditResponse) {
366366

367-
this._diffZone.showDiff(() => this._session.wholeRange, changes);
368-
this._updateSummaryMessage(changes);
367+
this._diffZone.showDiff();
368+
this._updateSummaryMessage();
369+
this._ctxShowingDiff.set(true);
369370

370371
if (response.singleCreateFileEdit) {
371372
this._previewZone.showCreation(this._session.wholeRange, response.singleCreateFileEdit.uri, await Promise.all(response.singleCreateFileEdit.edits));
372373
} else {
373374
this._previewZone.hide();
374375
}
375376
}
377+
378+
override toggleDiff(): void {
379+
// TODO@jrieken should this be persisted like we do in live-mode?
380+
const scrollState = StableEditorScrollState.capture(this._editor);
381+
if (this._diffZone.isVisible) {
382+
this._diffZone.hide();
383+
this._ctxShowingDiff.set(false);
384+
} else {
385+
this._diffZone.showDiff();
386+
this._ctxShowingDiff.set(true);
387+
}
388+
scrollState.restore(this._editor);
389+
}
376390
}
377391

378392
function showSingleCreateFile(accessor: ServicesAccessor, edit: EditResponse) {

0 commit comments

Comments
 (0)