Skip to content

Commit 4aa873b

Browse files
alexdimaalexdima
committed
Introduce grouping for undo/redo elements (fixes microsoft#101789)
1 parent b33d560 commit 4aa873b

7 files changed

Lines changed: 123 additions & 30 deletions

File tree

‎src/vs/platform/undoRedo/common/undoRedo.ts‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ export class ResourceEditStackSnapshot {
6060
) { }
6161
}
6262

63+
export class UndoRedoGroup {
64+
private static _ID = 0;
65+
66+
public readonly id: number;
67+
68+
constructor() {
69+
this.id = UndoRedoGroup._ID++;
70+
}
71+
72+
public static None = new UndoRedoGroup();
73+
}
74+
6375
export interface IUndoRedoService {
6476
readonly _serviceBrand: undefined;
6577

@@ -79,7 +91,7 @@ export interface IUndoRedoService {
7991
* Add a new element to the `undo` stack.
8092
* This will destroy the `redo` stack.
8193
*/
82-
pushElement(element: IUndoRedoElement): void;
94+
pushElement(element: IUndoRedoElement, group?: UndoRedoGroup): void;
8395

8496
/**
8597
* Get the last pushed element for a resource.

‎src/vs/platform/undoRedo/common/undoRedoService.ts‎

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

66
import * as nls from 'vs/nls';
7-
import { IUndoRedoService, IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoElement, IPastFutureElements, ResourceEditStackSnapshot, UriComparisonKeyComputer, IResourceUndoRedoElement } from 'vs/platform/undoRedo/common/undoRedo';
7+
import { IUndoRedoService, IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoElement, IPastFutureElements, ResourceEditStackSnapshot, UriComparisonKeyComputer, IResourceUndoRedoElement, UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
88
import { URI } from 'vs/base/common/uri';
99
import { onUnexpectedError } from 'vs/base/common/errors';
1010
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
@@ -32,15 +32,17 @@ class ResourceStackElement {
3232
public readonly strResource: string;
3333
public readonly resourceLabels: string[];
3434
public readonly strResources: string[];
35+
public readonly groupId: number;
3536
public isValid: boolean;
3637

37-
constructor(actual: IUndoRedoElement, resourceLabel: string, strResource: string) {
38+
constructor(actual: IUndoRedoElement, resourceLabel: string, strResource: string, groupId: number) {
3839
this.actual = actual;
3940
this.label = actual.label;
4041
this.resourceLabel = resourceLabel;
4142
this.strResource = strResource;
4243
this.resourceLabels = [this.resourceLabel];
4344
this.strResources = [this.strResource];
45+
this.groupId = groupId;
4446
this.isValid = true;
4547
}
4648

@@ -124,14 +126,16 @@ class WorkspaceStackElement {
124126

125127
public readonly resourceLabels: string[];
126128
public readonly strResources: string[];
129+
public readonly groupId: number;
127130
public removedResources: RemovedResources | null;
128131
public invalidatedResources: RemovedResources | null;
129132

130-
constructor(actual: IWorkspaceUndoRedoElement, resourceLabels: string[], strResources: string[]) {
133+
constructor(actual: IWorkspaceUndoRedoElement, resourceLabels: string[], strResources: string[], groupId: number) {
131134
this.actual = actual;
132135
this.label = actual.label;
133136
this.resourceLabels = resourceLabels;
134137
this.strResources = strResources;
138+
this.groupId = groupId;
135139
this.removedResources = null;
136140
this.invalidatedResources = null;
137141
}
@@ -349,6 +353,13 @@ class ResourceEditStack {
349353
return this._past[this._past.length - 1];
350354
}
351355

356+
public getSecondClosestPastElement(): StackElement | null {
357+
if (this._past.length < 2) {
358+
return null;
359+
}
360+
return this._past[this._past.length - 2];
361+
}
362+
352363
public getClosestFutureElement(): StackElement | null {
353364
if (this._future.length === 0) {
354365
return null;
@@ -482,11 +493,11 @@ export class UndoRedoService implements IUndoRedoService {
482493
console.log(str.join('\n'));
483494
}
484495

485-
public pushElement(element: IUndoRedoElement): void {
496+
public pushElement(element: IUndoRedoElement, group: UndoRedoGroup = UndoRedoGroup.None): void {
486497
if (element.type === UndoRedoElementType.Resource) {
487498
const resourceLabel = getResourceLabel(element.resource);
488499
const strResource = this.getUriComparisonKey(element.resource);
489-
this._pushElement(new ResourceStackElement(element, resourceLabel, strResource));
500+
this._pushElement(new ResourceStackElement(element, resourceLabel, strResource, group.id));
490501
} else {
491502
const seen = new Set<string>();
492503
const resourceLabels: string[] = [];
@@ -504,9 +515,9 @@ export class UndoRedoService implements IUndoRedoService {
504515
}
505516

506517
if (resourceLabels.length === 1) {
507-
this._pushElement(new ResourceStackElement(element, resourceLabels[0], strResources[0]));
518+
this._pushElement(new ResourceStackElement(element, resourceLabels[0], strResources[0], group.id));
508519
} else {
509-
this._pushElement(new WorkspaceStackElement(element, resourceLabels, strResources));
520+
this._pushElement(new WorkspaceStackElement(element, resourceLabels, strResources, group.id));
510521
}
511522
}
512523
if (DEBUG) {
@@ -550,7 +561,7 @@ export class UndoRedoService implements IUndoRedoService {
550561
for (const _element of individualArr) {
551562
const resourceLabel = getResourceLabel(_element.resource);
552563
const strResource = this.getUriComparisonKey(_element.resource);
553-
const element = new ResourceStackElement(_element, resourceLabel, strResource);
564+
const element = new ResourceStackElement(_element, resourceLabel, strResource, 0);
554565
individualMap.set(element.strResource, element);
555566
}
556567

@@ -569,7 +580,7 @@ export class UndoRedoService implements IUndoRedoService {
569580
for (const _element of individualArr) {
570581
const resourceLabel = getResourceLabel(_element.resource);
571582
const strResource = this.getUriComparisonKey(_element.resource);
572-
const element = new ResourceStackElement(_element, resourceLabel, strResource);
583+
const element = new ResourceStackElement(_element, resourceLabel, strResource, 0);
573584
individualMap.set(element.strResource, element);
574585
}
575586

@@ -688,7 +699,7 @@ export class UndoRedoService implements IUndoRedoService {
688699
};
689700
}
690701

691-
private _safeInvokeWithLocks(element: StackElement, invoke: () => Promise<void> | void, editStackSnapshot: EditStackSnapshot, cleanup: IDisposable = Disposable.None): Promise<void> | void {
702+
private _safeInvokeWithLocks(element: StackElement, invoke: () => Promise<void> | void, editStackSnapshot: EditStackSnapshot, cleanup: IDisposable, continuation: () => Promise<void> | void): Promise<void> | void {
692703
const releaseLocks = this._acquireLocks(editStackSnapshot);
693704

694705
let result: Promise<void> | void;
@@ -706,6 +717,7 @@ export class UndoRedoService implements IUndoRedoService {
706717
() => {
707718
releaseLocks();
708719
cleanup.dispose();
720+
return continuation();
709721
},
710722
(err) => {
711723
releaseLocks();
@@ -717,6 +729,7 @@ export class UndoRedoService implements IUndoRedoService {
717729
// result is void
718730
releaseLocks();
719731
cleanup.dispose();
732+
return continuation();
720733
}
721734
}
722735

@@ -864,9 +877,34 @@ export class UndoRedoService implements IUndoRedoService {
864877
return this._confirmAndExecuteWorkspaceUndo(strResource, element, affectedEditStacks);
865878
}
866879

880+
private _isPartOfUndoGroup(element: WorkspaceStackElement): boolean {
881+
if (!element.groupId) {
882+
return false;
883+
}
884+
// check that there is at least another element with the same groupId ready to be undone
885+
for (const [, editStack] of this._editStacks) {
886+
const pastElement = editStack.getClosestPastElement();
887+
if (!pastElement) {
888+
continue;
889+
}
890+
if (pastElement === element) {
891+
const secondPastElement = editStack.getSecondClosestPastElement();
892+
if (secondPastElement && secondPastElement.groupId === element.groupId) {
893+
// there is another element with the same group id in the same stack!
894+
return true;
895+
}
896+
}
897+
if (pastElement.groupId === element.groupId) {
898+
// there is another element with the same group id in another stack!
899+
return true;
900+
}
901+
}
902+
return false;
903+
}
904+
867905
private async _confirmAndExecuteWorkspaceUndo(strResource: string, element: WorkspaceStackElement, editStackSnapshot: EditStackSnapshot): Promise<void> {
868906

869-
if (element.canSplit()) {
907+
if (element.canSplit() && !this._isPartOfUndoGroup(element)) {
870908
// this element can be split
871909

872910
const result = await this._dialogService.show(
@@ -920,7 +958,7 @@ export class UndoRedoService implements IUndoRedoService {
920958
for (const editStack of editStackSnapshot.editStacks) {
921959
editStack.moveBackward(element);
922960
}
923-
return this._safeInvokeWithLocks(element, () => element.actual.undo(), editStackSnapshot, cleanup);
961+
return this._safeInvokeWithLocks(element, () => element.actual.undo(), editStackSnapshot, cleanup, () => this._continueUndoInGroup(element.groupId));
924962
}
925963

926964
private _resourceUndo(editStack: ResourceEditStack, element: ResourceStackElement): Promise<void> | void {
@@ -939,10 +977,26 @@ export class UndoRedoService implements IUndoRedoService {
939977
}
940978
return this._invokeResourcePrepare(element, (cleanup) => {
941979
editStack.moveBackward(element);
942-
return this._safeInvokeWithLocks(element, () => element.actual.undo(), new EditStackSnapshot([editStack]), cleanup);
980+
return this._safeInvokeWithLocks(element, () => element.actual.undo(), new EditStackSnapshot([editStack]), cleanup, () => this._continueUndoInGroup(element.groupId));
943981
});
944982
}
945983

984+
private _continueUndoInGroup(groupId: number): Promise<void> | void {
985+
if (!groupId) {
986+
return;
987+
}
988+
// find another element with the same groupId ready to be undone
989+
for (const [strResource, editStack] of this._editStacks) {
990+
const pastElement = editStack.getClosestPastElement();
991+
if (!pastElement) {
992+
continue;
993+
}
994+
if (pastElement.groupId === groupId) {
995+
return this.undo(strResource);
996+
}
997+
}
998+
}
999+
9461000
public undo(resource: URI | string): Promise<void> | void {
9471001
const strResource = typeof resource === 'string' ? resource : this.getUriComparisonKey(resource);
9481002
if (!this._editStacks.has(strResource)) {
@@ -1097,7 +1151,7 @@ export class UndoRedoService implements IUndoRedoService {
10971151
for (const editStack of editStackSnapshot.editStacks) {
10981152
editStack.moveForward(element);
10991153
}
1100-
return this._safeInvokeWithLocks(element, () => element.actual.redo(), editStackSnapshot, cleanup);
1154+
return this._safeInvokeWithLocks(element, () => element.actual.redo(), editStackSnapshot, cleanup, () => this._continueRedoInGroup(element.groupId));
11011155
}
11021156

11031157
private _resourceRedo(editStack: ResourceEditStack, element: ResourceStackElement): Promise<void> | void {
@@ -1117,10 +1171,26 @@ export class UndoRedoService implements IUndoRedoService {
11171171

11181172
return this._invokeResourcePrepare(element, (cleanup) => {
11191173
editStack.moveForward(element);
1120-
return this._safeInvokeWithLocks(element, () => element.actual.redo(), new EditStackSnapshot([editStack]), cleanup);
1174+
return this._safeInvokeWithLocks(element, () => element.actual.redo(), new EditStackSnapshot([editStack]), cleanup, () => this._continueRedoInGroup(element.groupId));
11211175
});
11221176
}
11231177

1178+
private _continueRedoInGroup(groupId: number): Promise<void> | void {
1179+
if (!groupId) {
1180+
return;
1181+
}
1182+
// find another element with the same groupId ready to be redone
1183+
for (const [strResource, editStack] of this._editStacks) {
1184+
const futureElement = editStack.getClosestFutureElement();
1185+
if (!futureElement) {
1186+
continue;
1187+
}
1188+
if (futureElement.groupId === groupId) {
1189+
return this.redo(strResource);
1190+
}
1191+
}
1192+
}
1193+
11241194
public redo(resource: URI | string): Promise<void> | void {
11251195
const strResource = typeof resource === 'string' ? resource : this.getUriComparisonKey(resource);
11261196
if (!this._editStacks.has(strResource)) {

‎src/vs/platform/undoRedo/test/common/undoRedoService.test.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import * as assert from 'assert';
77
import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService';
88
import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService';
99
import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
10-
import { UndoRedoElementType, IUndoRedoElement } from 'vs/platform/undoRedo/common/undoRedo';
10+
import { UndoRedoElementType, IUndoRedoElement, UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
1111
import { URI } from 'vs/base/common/uri';
1212
import { mock } from 'vs/base/test/common/mock';
1313
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
@@ -208,4 +208,9 @@ suite('UndoRedoService', () => {
208208
assert.ok(service.getLastElement(resource2) === element1);
209209

210210
});
211+
212+
test('UndoRedoGroup.None uses id 0', () => {
213+
assert.equal(UndoRedoGroup.None, 0);
214+
});
215+
211216
});

‎src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { URI } from 'vs/base/common/uri';
99
import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService';
1010
import { WorkspaceEditMetadata } from 'vs/editor/common/modes';
1111
import { IProgress } from 'vs/platform/progress/common/progress';
12+
import { UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
1213
import { ICellEditOperation } from 'vs/workbench/contrib/notebook/common/notebookCommon';
1314
import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService';
1415
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
@@ -28,6 +29,7 @@ export class ResourceNotebookCellEdit extends ResourceEdit {
2829
export class BulkCellEdits {
2930

3031
constructor(
32+
undoRedoGroup: UndoRedoGroup,
3133
private readonly _progress: IProgress<void>,
3234
private readonly _edits: ResourceNotebookCellEdit[],
3335
@INotebookService private readonly _notebookService: INotebookService,

‎src/vs/workbench/contrib/bulkEdit/browser/bulkEditService.ts‎

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
1616
import { BulkTextEdits } from 'vs/workbench/contrib/bulkEdit/browser/bulkTextEdits';
1717
import { BulkFileEdits } from 'vs/workbench/contrib/bulkEdit/browser/bulkFileEdits';
1818
import { BulkCellEdits, ResourceNotebookCellEdit } from 'vs/workbench/contrib/bulkEdit/browser/bulkCellEdits';
19+
import { UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
1920

2021
class BulkEdit {
2122

@@ -60,38 +61,39 @@ class BulkEdit {
6061
this._progress.report({ total: this._edits.length });
6162
const progress: IProgress<void> = { report: _ => this._progress.report({ increment: 1 }) };
6263

64+
const undoRedoGroup = new UndoRedoGroup();
6365

6466
let index = 0;
6567
for (let range of ranges) {
6668
const group = this._edits.slice(index, index + range);
6769
if (group[0] instanceof ResourceFileEdit) {
68-
await this._performFileEdits(<ResourceFileEdit[]>group, progress);
70+
await this._performFileEdits(<ResourceFileEdit[]>group, undoRedoGroup, progress);
6971
} else if (group[0] instanceof ResourceTextEdit) {
70-
await this._performTextEdits(<ResourceTextEdit[]>group, progress);
72+
await this._performTextEdits(<ResourceTextEdit[]>group, undoRedoGroup, progress);
7173
} else if (group[0] instanceof ResourceNotebookCellEdit) {
72-
await this._performCellEdits(<ResourceNotebookCellEdit[]>group, progress);
74+
await this._performCellEdits(<ResourceNotebookCellEdit[]>group, undoRedoGroup, progress);
7375
} else {
7476
console.log('UNKNOWN EDIT');
7577
}
7678
index = index + range;
7779
}
7880
}
7981

80-
private async _performFileEdits(edits: ResourceFileEdit[], progress: IProgress<void>) {
82+
private async _performFileEdits(edits: ResourceFileEdit[], undoRedoGroup: UndoRedoGroup, progress: IProgress<void>) {
8183
this._logService.debug('_performFileEdits', JSON.stringify(edits));
82-
const model = this._instaService.createInstance(BulkFileEdits, this._label || localize('workspaceEdit', "Workspace Edit"), progress, edits);
84+
const model = this._instaService.createInstance(BulkFileEdits, this._label || localize('workspaceEdit', "Workspace Edit"), undoRedoGroup, progress, edits);
8385
await model.apply();
8486
}
8587

86-
private async _performTextEdits(edits: ResourceTextEdit[], progress: IProgress<void>): Promise<void> {
88+
private async _performTextEdits(edits: ResourceTextEdit[], undoRedoGroup: UndoRedoGroup, progress: IProgress<void>): Promise<void> {
8789
this._logService.debug('_performTextEdits', JSON.stringify(edits));
88-
const model = this._instaService.createInstance(BulkTextEdits, this._label || localize('workspaceEdit', "Workspace Edit"), this._editor, progress, edits);
90+
const model = this._instaService.createInstance(BulkTextEdits, this._label || localize('workspaceEdit', "Workspace Edit"), this._editor, undoRedoGroup, progress, edits);
8991
await model.apply();
9092
}
9193

92-
private async _performCellEdits(edits: ResourceNotebookCellEdit[], progress: IProgress<void>): Promise<void> {
94+
private async _performCellEdits(edits: ResourceNotebookCellEdit[], undoRedoGroup: UndoRedoGroup, progress: IProgress<void>): Promise<void> {
9395
this._logService.debug('_performCellEdits', JSON.stringify(edits));
94-
const model = this._instaService.createInstance(BulkCellEdits, progress, edits);
96+
const model = this._instaService.createInstance(BulkCellEdits, undoRedoGroup, progress, edits);
9597
await model.apply();
9698
}
9799
}

‎src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import { IFileService, FileSystemProviderCapabilities } from 'vs/platform/files/
99
import { IProgress } from 'vs/platform/progress/common/progress';
1010
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
1111
import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
12-
import { IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo';
12+
import { IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoService, UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
1313
import { URI } from 'vs/base/common/uri';
1414
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
1515
import { ILogService } from 'vs/platform/log/common/log';
@@ -147,6 +147,7 @@ export class BulkFileEdits {
147147

148148
constructor(
149149
private readonly _label: string,
150+
private readonly _undoRedoGroup: UndoRedoGroup,
150151
private readonly _progress: IProgress<void>,
151152
private readonly _edits: ResourceFileEdit[],
152153
@IInstantiationService private readonly _instaService: IInstantiationService,
@@ -176,6 +177,6 @@ export class BulkFileEdits {
176177
}
177178
}
178179

179-
this._undoRedoService.pushElement(new FileUndoRedoElement(this._label, undoOperations));
180+
this._undoRedoService.pushElement(new FileUndoRedoElement(this._label, undoOperations), this._undoRedoGroup);
180181
}
181182
}

0 commit comments

Comments
 (0)