Skip to content

Commit

Permalink
Introduce grouping for undo/redo elements (fixes #101789)
Browse files Browse the repository at this point in the history
  • Loading branch information
@alexdima
alexdima committed Sep 23, 2020
1 parent b33d560 commit 4aa873b
Show file tree
Hide file tree
Showing 7 changed files with 123 additions and 30 deletions.
14 changes: 13 additions & 1 deletion src/vs/platform/undoRedo/common/undoRedo.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,18 @@ export class ResourceEditStackSnapshot {
) { }
}

export class UndoRedoGroup {
private static _ID = 0;

public readonly id: number;

constructor() {
this.id = UndoRedoGroup._ID++;
}

public static None = new UndoRedoGroup();
}

export interface IUndoRedoService {
readonly _serviceBrand: undefined;

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

/**
* Get the last pushed element for a resource.
Expand Down
100 changes: 85 additions & 15 deletions src/vs/platform/undoRedo/common/undoRedoService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import * as nls from 'vs/nls';
import { IUndoRedoService, IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoElement, IPastFutureElements, ResourceEditStackSnapshot, UriComparisonKeyComputer, IResourceUndoRedoElement } from 'vs/platform/undoRedo/common/undoRedo';
import { IUndoRedoService, IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoElement, IPastFutureElements, ResourceEditStackSnapshot, UriComparisonKeyComputer, IResourceUndoRedoElement, UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
import { URI } from 'vs/base/common/uri';
import { onUnexpectedError } from 'vs/base/common/errors';
import { registerSingleton } from 'vs/platform/instantiation/common/extensions';
Expand Down Expand Up @@ -32,15 +32,17 @@ class ResourceStackElement {
public readonly strResource: string;
public readonly resourceLabels: string[];
public readonly strResources: string[];
public readonly groupId: number;
public isValid: boolean;

constructor(actual: IUndoRedoElement, resourceLabel: string, strResource: string) {
constructor(actual: IUndoRedoElement, resourceLabel: string, strResource: string, groupId: number) {
this.actual = actual;
this.label = actual.label;
this.resourceLabel = resourceLabel;
this.strResource = strResource;
this.resourceLabels = [this.resourceLabel];
this.strResources = [this.strResource];
this.groupId = groupId;
this.isValid = true;
}

Expand Down Expand Up @@ -124,14 +126,16 @@ class WorkspaceStackElement {

public readonly resourceLabels: string[];
public readonly strResources: string[];
public readonly groupId: number;
public removedResources: RemovedResources | null;
public invalidatedResources: RemovedResources | null;

constructor(actual: IWorkspaceUndoRedoElement, resourceLabels: string[], strResources: string[]) {
constructor(actual: IWorkspaceUndoRedoElement, resourceLabels: string[], strResources: string[], groupId: number) {
this.actual = actual;
this.label = actual.label;
this.resourceLabels = resourceLabels;
this.strResources = strResources;
this.groupId = groupId;
this.removedResources = null;
this.invalidatedResources = null;
}
Expand Down Expand Up @@ -349,6 +353,13 @@ class ResourceEditStack {
return this._past[this._past.length - 1];
}

public getSecondClosestPastElement(): StackElement | null {
if (this._past.length < 2) {
return null;
}
return this._past[this._past.length - 2];
}

public getClosestFutureElement(): StackElement | null {
if (this._future.length === 0) {
return null;
Expand Down Expand Up @@ -482,11 +493,11 @@ export class UndoRedoService implements IUndoRedoService {
console.log(str.join('\n'));
}

public pushElement(element: IUndoRedoElement): void {
public pushElement(element: IUndoRedoElement, group: UndoRedoGroup = UndoRedoGroup.None): void {
if (element.type === UndoRedoElementType.Resource) {
const resourceLabel = getResourceLabel(element.resource);
const strResource = this.getUriComparisonKey(element.resource);
this._pushElement(new ResourceStackElement(element, resourceLabel, strResource));
this._pushElement(new ResourceStackElement(element, resourceLabel, strResource, group.id));
} else {
const seen = new Set<string>();
const resourceLabels: string[] = [];
Expand All @@ -504,9 +515,9 @@ export class UndoRedoService implements IUndoRedoService {
}

if (resourceLabels.length === 1) {
this._pushElement(new ResourceStackElement(element, resourceLabels[0], strResources[0]));
this._pushElement(new ResourceStackElement(element, resourceLabels[0], strResources[0], group.id));
} else {
this._pushElement(new WorkspaceStackElement(element, resourceLabels, strResources));
this._pushElement(new WorkspaceStackElement(element, resourceLabels, strResources, group.id));
}
}
if (DEBUG) {
Expand Down Expand Up @@ -550,7 +561,7 @@ export class UndoRedoService implements IUndoRedoService {
for (const _element of individualArr) {
const resourceLabel = getResourceLabel(_element.resource);
const strResource = this.getUriComparisonKey(_element.resource);
const element = new ResourceStackElement(_element, resourceLabel, strResource);
const element = new ResourceStackElement(_element, resourceLabel, strResource, 0);
individualMap.set(element.strResource, element);
}

Expand All @@ -569,7 +580,7 @@ export class UndoRedoService implements IUndoRedoService {
for (const _element of individualArr) {
const resourceLabel = getResourceLabel(_element.resource);
const strResource = this.getUriComparisonKey(_element.resource);
const element = new ResourceStackElement(_element, resourceLabel, strResource);
const element = new ResourceStackElement(_element, resourceLabel, strResource, 0);
individualMap.set(element.strResource, element);
}

Expand Down Expand Up @@ -688,7 +699,7 @@ export class UndoRedoService implements IUndoRedoService {
};
}

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

let result: Promise<void> | void;
Expand All @@ -706,6 +717,7 @@ export class UndoRedoService implements IUndoRedoService {
() => {
releaseLocks();
cleanup.dispose();
return continuation();
},
(err) => {
releaseLocks();
Expand All @@ -717,6 +729,7 @@ export class UndoRedoService implements IUndoRedoService {
// result is void
releaseLocks();
cleanup.dispose();
return continuation();
}
}

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

private _isPartOfUndoGroup(element: WorkspaceStackElement): boolean {
if (!element.groupId) {
return false;
}
// check that there is at least another element with the same groupId ready to be undone
for (const [, editStack] of this._editStacks) {
const pastElement = editStack.getClosestPastElement();
if (!pastElement) {
continue;
}
if (pastElement === element) {
const secondPastElement = editStack.getSecondClosestPastElement();
if (secondPastElement && secondPastElement.groupId === element.groupId) {
// there is another element with the same group id in the same stack!
return true;
}
}
if (pastElement.groupId === element.groupId) {
// there is another element with the same group id in another stack!
return true;
}
}
return false;
}

private async _confirmAndExecuteWorkspaceUndo(strResource: string, element: WorkspaceStackElement, editStackSnapshot: EditStackSnapshot): Promise<void> {

if (element.canSplit()) {
if (element.canSplit() && !this._isPartOfUndoGroup(element)) {
// this element can be split

const result = await this._dialogService.show(
Expand Down Expand Up @@ -920,7 +958,7 @@ export class UndoRedoService implements IUndoRedoService {
for (const editStack of editStackSnapshot.editStacks) {
editStack.moveBackward(element);
}
return this._safeInvokeWithLocks(element, () => element.actual.undo(), editStackSnapshot, cleanup);
return this._safeInvokeWithLocks(element, () => element.actual.undo(), editStackSnapshot, cleanup, () => this._continueUndoInGroup(element.groupId));
}

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

private _continueUndoInGroup(groupId: number): Promise<void> | void {
if (!groupId) {
return;
}
// find another element with the same groupId ready to be undone
for (const [strResource, editStack] of this._editStacks) {
const pastElement = editStack.getClosestPastElement();
if (!pastElement) {
continue;
}
if (pastElement.groupId === groupId) {
return this.undo(strResource);
}
}
}

public undo(resource: URI | string): Promise<void> | void {
const strResource = typeof resource === 'string' ? resource : this.getUriComparisonKey(resource);
if (!this._editStacks.has(strResource)) {
Expand Down Expand Up @@ -1097,7 +1151,7 @@ export class UndoRedoService implements IUndoRedoService {
for (const editStack of editStackSnapshot.editStacks) {
editStack.moveForward(element);
}
return this._safeInvokeWithLocks(element, () => element.actual.redo(), editStackSnapshot, cleanup);
return this._safeInvokeWithLocks(element, () => element.actual.redo(), editStackSnapshot, cleanup, () => this._continueRedoInGroup(element.groupId));
}

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

return this._invokeResourcePrepare(element, (cleanup) => {
editStack.moveForward(element);
return this._safeInvokeWithLocks(element, () => element.actual.redo(), new EditStackSnapshot([editStack]), cleanup);
return this._safeInvokeWithLocks(element, () => element.actual.redo(), new EditStackSnapshot([editStack]), cleanup, () => this._continueRedoInGroup(element.groupId));
});
}

private _continueRedoInGroup(groupId: number): Promise<void> | void {
if (!groupId) {
return;
}
// find another element with the same groupId ready to be redone
for (const [strResource, editStack] of this._editStacks) {
const futureElement = editStack.getClosestFutureElement();
if (!futureElement) {
continue;
}
if (futureElement.groupId === groupId) {
return this.redo(strResource);
}
}
}

public redo(resource: URI | string): Promise<void> | void {
const strResource = typeof resource === 'string' ? resource : this.getUriComparisonKey(resource);
if (!this._editStacks.has(strResource)) {
Expand Down
7 changes: 6 additions & 1 deletion src/vs/platform/undoRedo/test/common/undoRedoService.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as assert from 'assert';
import { UndoRedoService } from 'vs/platform/undoRedo/common/undoRedoService';
import { TestDialogService } from 'vs/platform/dialogs/test/common/testDialogService';
import { TestNotificationService } from 'vs/platform/notification/test/common/testNotificationService';
import { UndoRedoElementType, IUndoRedoElement } from 'vs/platform/undoRedo/common/undoRedo';
import { UndoRedoElementType, IUndoRedoElement, UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
import { URI } from 'vs/base/common/uri';
import { mock } from 'vs/base/test/common/mock';
import { IDialogService } from 'vs/platform/dialogs/common/dialogs';
Expand Down Expand Up @@ -208,4 +208,9 @@ suite('UndoRedoService', () => {
assert.ok(service.getLastElement(resource2) === element1);

});

test('UndoRedoGroup.None uses id 0', () => {
assert.equal(UndoRedoGroup.None, 0);
});

});
2 changes: 2 additions & 0 deletions src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { URI } from 'vs/base/common/uri';
import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService';
import { WorkspaceEditMetadata } from 'vs/editor/common/modes';
import { IProgress } from 'vs/platform/progress/common/progress';
import { UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
import { ICellEditOperation } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
Expand All @@ -28,6 +29,7 @@ export class ResourceNotebookCellEdit extends ResourceEdit {
export class BulkCellEdits {

constructor(
undoRedoGroup: UndoRedoGroup,
private readonly _progress: IProgress<void>,
private readonly _edits: ResourceNotebookCellEdit[],
@INotebookService private readonly _notebookService: INotebookService,
Expand Down
20 changes: 11 additions & 9 deletions src/vs/workbench/contrib/bulkEdit/browser/bulkEditService.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti
import { BulkTextEdits } from 'vs/workbench/contrib/bulkEdit/browser/bulkTextEdits';
import { BulkFileEdits } from 'vs/workbench/contrib/bulkEdit/browser/bulkFileEdits';
import { BulkCellEdits, ResourceNotebookCellEdit } from 'vs/workbench/contrib/bulkEdit/browser/bulkCellEdits';
import { UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';

class BulkEdit {

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

const undoRedoGroup = new UndoRedoGroup();

let index = 0;
for (let range of ranges) {
const group = this._edits.slice(index, index + range);
if (group[0] instanceof ResourceFileEdit) {
await this._performFileEdits(<ResourceFileEdit[]>group, progress);
await this._performFileEdits(<ResourceFileEdit[]>group, undoRedoGroup, progress);
} else if (group[0] instanceof ResourceTextEdit) {
await this._performTextEdits(<ResourceTextEdit[]>group, progress);
await this._performTextEdits(<ResourceTextEdit[]>group, undoRedoGroup, progress);
} else if (group[0] instanceof ResourceNotebookCellEdit) {
await this._performCellEdits(<ResourceNotebookCellEdit[]>group, progress);
await this._performCellEdits(<ResourceNotebookCellEdit[]>group, undoRedoGroup, progress);
} else {
console.log('UNKNOWN EDIT');
}
index = index + range;
}
}

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

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

private async _performCellEdits(edits: ResourceNotebookCellEdit[], progress: IProgress<void>): Promise<void> {
private async _performCellEdits(edits: ResourceNotebookCellEdit[], undoRedoGroup: UndoRedoGroup, progress: IProgress<void>): Promise<void> {
this._logService.debug('_performCellEdits', JSON.stringify(edits));
const model = this._instaService.createInstance(BulkCellEdits, progress, edits);
const model = this._instaService.createInstance(BulkCellEdits, undoRedoGroup, progress, edits);
await model.apply();
}
}
Expand Down
5 changes: 3 additions & 2 deletions src/vs/workbench/contrib/bulkEdit/browser/bulkFileEdits.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { IFileService, FileSystemProviderCapabilities } from 'vs/platform/files/
import { IProgress } from 'vs/platform/progress/common/progress';
import { IConfigurationService } from 'vs/platform/configuration/common/configuration';
import { IWorkingCopyFileService } from 'vs/workbench/services/workingCopy/common/workingCopyFileService';
import { IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoService } from 'vs/platform/undoRedo/common/undoRedo';
import { IWorkspaceUndoRedoElement, UndoRedoElementType, IUndoRedoService, UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
import { URI } from 'vs/base/common/uri';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { ILogService } from 'vs/platform/log/common/log';
Expand Down Expand Up @@ -147,6 +147,7 @@ export class BulkFileEdits {

constructor(
private readonly _label: string,
private readonly _undoRedoGroup: UndoRedoGroup,
private readonly _progress: IProgress<void>,
private readonly _edits: ResourceFileEdit[],
@IInstantiationService private readonly _instaService: IInstantiationService,
Expand Down Expand Up @@ -176,6 +177,6 @@ export class BulkFileEdits {
}
}

this._undoRedoService.pushElement(new FileUndoRedoElement(this._label, undoOperations));
this._undoRedoService.pushElement(new FileUndoRedoElement(this._label, undoOperations), this._undoRedoGroup);
}
}
Loading

0 comments on commit 4aa873b

Please sign in to comment.