From 867f35c00dc0bcd34bbbdcc8fdf6472b460fc37a Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Thu, 8 Feb 2024 17:03:56 +0100 Subject: [PATCH 01/20] wip --- extensions/git/src/commands.ts | 1 + .../standalone/browser/standaloneEditor.ts | 1 + src/vs/workbench/api/common/extHostSCM.ts | 1 + .../bulkEdit/browser/preview/bulkEditPane.ts | 44 ++++++++++++++++--- .../browser/preview/bulkEditPreview.ts | 1 + 5 files changed, 41 insertions(+), 7 deletions(-) diff --git a/extensions/git/src/commands.ts b/extensions/git/src/commands.ts index 3f46f698ce22..9c6c2e1991b8 100644 --- a/extensions/git/src/commands.ts +++ b/extensions/git/src/commands.ts @@ -3780,6 +3780,7 @@ export class CommandCenter { resources.push(toMultiFileDiffEditorUris(change, stashFirstParentCommit, modifiedUriRef)); } + // Command which is executed in order to open the multi diff editor, uring the passed in URI, the given title and the resources which are the modified resource and the original resource commands.executeCommand('_workbench.openMultiDiffEditor', { multiDiffSourceUri, title, resources }); } diff --git a/src/vs/editor/standalone/browser/standaloneEditor.ts b/src/vs/editor/standalone/browser/standaloneEditor.ts index 059a4928862c..10a75a449042 100644 --- a/src/vs/editor/standalone/browser/standaloneEditor.ts +++ b/src/vs/editor/standalone/browser/standaloneEditor.ts @@ -100,6 +100,7 @@ export function createDiffEditor(domElement: HTMLElement, options?: IStandaloneD return instantiationService.createInstance(StandaloneDiffEditor2, domElement, options); } +// There is also a function which creates a multi file diff editor, but maybe we do not need it export function createMultiFileDiffEditor(domElement: HTMLElement, override?: IEditorOverrideServices) { const instantiationService = StandaloneServices.initialize(override || {}); return new MultiDiffEditorWidget(domElement, {}, instantiationService); diff --git a/src/vs/workbench/api/common/extHostSCM.ts b/src/vs/workbench/api/common/extHostSCM.ts index ee2be0b1bcc2..16c8268848bd 100644 --- a/src/vs/workbench/api/common/extHostSCM.ts +++ b/src/vs/workbench/api/common/extHostSCM.ts @@ -416,6 +416,7 @@ class ExtHostSourceControlResourceGroup implements vscode.SourceControlResourceG private _sourceControlHandle: number, private _id: string, private _label: string, + // The following appears to be adding multi diff editor support public readonly multiDiffEditorEnableViewChanges: boolean, private readonly _extension: IExtensionDescription, ) { } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 4a18e62f2563..4cfaa3c07eea 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -38,6 +38,7 @@ import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService'; import { ButtonBar } from 'vs/base/browser/ui/button/button'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; +import { ICommandService } from 'vs/platform/commands/common/commands'; const enum State { Data = 'data', @@ -79,6 +80,7 @@ export class BulkEditPane extends ViewPane { @IDialogService private readonly _dialogService: IDialogService, @IContextMenuService private readonly _contextMenuService: IContextMenuService, @IStorageService private readonly _storageService: IStorageService, + @ICommandService private readonly commandService: ICommandService, @IContextKeyService contextKeyService: IContextKeyService, @IViewDescriptorService viewDescriptorService: IViewDescriptorService, @IKeybindingService keybindingService: IKeybindingService, @@ -223,6 +225,7 @@ export class BulkEditPane extends ViewPane { return Boolean(this._currentInput); } + // Presumably the method where we set the data to show in the tree refactors view private async _setTreeInput(input: BulkFileOperations) { const viewState = this._treeViewStates.get(this._treeDataSource.groupByFile); @@ -316,6 +319,7 @@ export class BulkEditPane extends ViewPane { } } + // In this function, we actually open the element as an editor, and this is where we could open a multi file diff editor private async _openElementAsEditor(e: IOpenEvent): Promise { const options: Mutable = { ...e.editorOptions }; @@ -333,9 +337,15 @@ export class BulkEditPane extends ViewPane { return; } + console.log('options : ', JSON.stringify(options)); + const previewUri = this._currentProvider!.asPreviewUri(fileElement.edit.uri); if (fileElement.edit.type & BulkFileOperationType.Delete) { + + console.log('fileElement.edit : ', fileElement.edit); + console.log('previewUri : ', JSON.stringify(previewUri)); + // delete -> show single editor this._editorService.openEditor({ label: localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)), @@ -343,7 +353,17 @@ export class BulkEditPane extends ViewPane { options }); + // const label = localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)); + // const uri = fileElement.edit.newUri ?? fileElement.edit.uri; + // const resources: { originalUri: Uri | undefined; modifiedUri: Uri | undefined }[] = [{ + // originalUri: fileElement.edit.uri, + // modifiedUri: fileElement.edit.newUri + // }]; + // await commands.executeCommand('_workbench.openMultiDiffEditor', { uri, label, resources }); + } else { + console.log('fileElement.edit ; ', fileElement.edit); + // rename, create, edits -> show diff editr let leftResource: URI | undefined; try { @@ -367,13 +387,23 @@ export class BulkEditPane extends ViewPane { label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); } - this._editorService.openEditor({ - original: { resource: leftResource }, - modified: { resource: previewUri }, - label, - description: this._labelService.getUriLabel(dirname(leftResource), { relative: true }), - options - }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); + console.log('leftResource : ', JSON.stringify(leftResource)); + console.log('previewUri : ', JSON.stringify(previewUri)); + + // this._editorService.openEditor({ + // original: { resource: leftResource }, + // modified: { resource: previewUri }, + // label, + // description: this._labelService.getUriLabel(dirname(leftResource), { relative: true }), + // options + // }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); + + const uri = previewUri; + const resources: { originalUri: URI | undefined; modifiedUri: URI | undefined }[] = [{ + originalUri: leftResource, + modifiedUri: previewUri + }]; + this.commandService.executeCommand('_workbench.openMultiDiffEditor', { uri, label, resources }); } } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts index d9e8a1112a0d..c649d96c78cf 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts @@ -307,6 +307,7 @@ export class BulkFileOperations { return result; } + // Getting the edits for a specific file getFileEdits(uri: URI): ISingleEditOperation[] { for (const file of this.fileOperations) { From 3fa90909560116c71afd8b8eeda3947c85d67e5e Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Thu, 8 Feb 2024 17:23:47 +0100 Subject: [PATCH 02/20] adding support so that the multi file diff editor shows for all of the bulk operations at the same time --- src/vs/platform/list/browser/listService.ts | 1 + .../bulkEdit/browser/preview/bulkEditPane.ts | 128 ++++++++++++------ 2 files changed, 87 insertions(+), 42 deletions(-) diff --git a/src/vs/platform/list/browser/listService.ts b/src/vs/platform/list/browser/listService.ts index 6857531cb668..317af0fdf3a4 100644 --- a/src/vs/platform/list/browser/listService.ts +++ b/src/vs/platform/list/browser/listService.ts @@ -770,6 +770,7 @@ abstract class ResourceNavigator extends Disposable { this._open(element, preserveFocus, pinned, sideBySide, browserEvent); } + // We want to actually retrieve all of the elements, not just the element at hand private _open(element: T | undefined, preserveFocus: boolean, pinned: boolean, sideBySide: boolean, browserEvent?: UIEvent): void { if (!element) { return; diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 4cfaa3c07eea..437721a69fc1 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -145,6 +145,7 @@ export class BulkEditPane extends ViewPane { ); this._disposables.add(this._tree.onContextMenu(this._onContextMenu, this)); + // Thing is when the tree is clicked, we want actually to show all of the files inside of the multi diff editor this._disposables.add(this._tree.onDidOpen(e => this._openElementAsEditor(e))); // buttons @@ -339,56 +340,53 @@ export class BulkEditPane extends ViewPane { console.log('options : ', JSON.stringify(options)); - const previewUri = this._currentProvider!.asPreviewUri(fileElement.edit.uri); - if (fileElement.edit.type & BulkFileOperationType.Delete) { + const previewUri = this._currentProvider!.asPreviewUri(fileElement.edit.uri); console.log('fileElement.edit : ', fileElement.edit); console.log('previewUri : ', JSON.stringify(previewUri)); // delete -> show single editor - this._editorService.openEditor({ - label: localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)), - resource: previewUri, - options - }); + // this._editorService.openEditor({ + // label: localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)), + // resource: previewUri, + // options + // }); - // const label = localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)); - // const uri = fileElement.edit.newUri ?? fileElement.edit.uri; - // const resources: { originalUri: Uri | undefined; modifiedUri: Uri | undefined }[] = [{ - // originalUri: fileElement.edit.uri, - // modifiedUri: fileElement.edit.newUri - // }]; - // await commands.executeCommand('_workbench.openMultiDiffEditor', { uri, label, resources }); + const uri = previewUri; + + const label = localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)); + const resources: { originalUri: URI | undefined; modifiedUri: URI | undefined }[] = [{ + originalUri: undefined, + modifiedUri: previewUri + }]; + this.commandService.executeCommand('_workbench.openMultiDiffEditor', { uri, label, resources }); } else { console.log('fileElement.edit ; ', fileElement.edit); // rename, create, edits -> show diff editr - let leftResource: URI | undefined; - try { - (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); - leftResource = fileElement.edit.uri; - } catch { - leftResource = BulkEditPreviewProvider.emptyPreview; - } + // let leftResource: URI | undefined; + // try { + // (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); + // leftResource = fileElement.edit.uri; + // } catch { + // leftResource = BulkEditPreviewProvider.emptyPreview; + // } - let typeLabel: string | undefined; - if (fileElement.edit.type & BulkFileOperationType.Rename) { - typeLabel = localize('rename', "rename"); - } else if (fileElement.edit.type & BulkFileOperationType.Create) { - typeLabel = localize('create', "create"); - } + // let typeLabel: string | undefined; + // if (fileElement.edit.type & BulkFileOperationType.Rename) { + // typeLabel = localize('rename', "rename"); + // } else if (fileElement.edit.type & BulkFileOperationType.Create) { + // typeLabel = localize('create', "create"); + // } - let label: string; - if (typeLabel) { - label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); - } else { - label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); - } - - console.log('leftResource : ', JSON.stringify(leftResource)); - console.log('previewUri : ', JSON.stringify(previewUri)); + // let label: string; + // if (typeLabel) { + // label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); + // } else { + // label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); + // } // this._editorService.openEditor({ // original: { resource: leftResource }, @@ -398,12 +396,58 @@ export class BulkEditPane extends ViewPane { // options // }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); - const uri = previewUri; - const resources: { originalUri: URI | undefined; modifiedUri: URI | undefined }[] = [{ - originalUri: leftResource, - modifiedUri: previewUri - }]; - this.commandService.executeCommand('_workbench.openMultiDiffEditor', { uri, label, resources }); + // Perhaps a better way to access the parent instead of accessing through the parent of the file element + + let bulkFileOperations: BulkFileOperations | undefined = undefined; + let currentParent = fileElement.parent; + while (true) { + if (currentParent instanceof BulkFileOperations) { + bulkFileOperations = currentParent; + break; + } + currentParent = currentParent.parent; + } + const allBulkFileOperations = bulkFileOperations.fileOperations; + console.log('allBulkFileOperations : ', allBulkFileOperations); + + const resources: { originalUri: URI | undefined; modifiedUri: URI | undefined }[] = []; + + for (const operation of allBulkFileOperations) { + const previewUri = this._currentProvider!.asPreviewUri(operation.textEdits[0].textEdit.resource); + + let leftResource: URI | undefined; + try { + (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); + leftResource = fileElement.edit.uri; + } catch { + leftResource = BulkEditPreviewProvider.emptyPreview; + } + + let typeLabel: string | undefined; + if (fileElement.edit.type & BulkFileOperationType.Rename) { + typeLabel = localize('rename', "rename"); + } else if (fileElement.edit.type & BulkFileOperationType.Create) { + typeLabel = localize('create', "create"); + } + + let label: string; + if (typeLabel) { + label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); + } else { + label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); + } + + console.log('leftResource : ', JSON.stringify(leftResource)); + console.log('previewUri : ', JSON.stringify(previewUri)); + + resources.push({ + originalUri: leftResource, + modifiedUri: previewUri + }); + } + + const refactorSourceUri = URI.from({ scheme: 'refactor-preview' }); + this.commandService.executeCommand('_workbench.openMultiDiffEditor', { refactorSourceUri, label: 'Refactor Preview', resources }); } } From d40fd3baa2b280c3efc9432ebf07c57fb4aa1a84 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Thu, 8 Feb 2024 17:32:06 +0100 Subject: [PATCH 03/20] adding notes --- .../bulkEdit/browser/preview/bulkEditPane.ts | 44 ++----------------- 1 file changed, 4 insertions(+), 40 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 437721a69fc1..229662787263 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -346,13 +346,6 @@ export class BulkEditPane extends ViewPane { console.log('fileElement.edit : ', fileElement.edit); console.log('previewUri : ', JSON.stringify(previewUri)); - // delete -> show single editor - // this._editorService.openEditor({ - // label: localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)), - // resource: previewUri, - // options - // }); - const uri = previewUri; const label = localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)); @@ -365,39 +358,6 @@ export class BulkEditPane extends ViewPane { } else { console.log('fileElement.edit ; ', fileElement.edit); - // rename, create, edits -> show diff editr - // let leftResource: URI | undefined; - // try { - // (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); - // leftResource = fileElement.edit.uri; - // } catch { - // leftResource = BulkEditPreviewProvider.emptyPreview; - // } - - // let typeLabel: string | undefined; - // if (fileElement.edit.type & BulkFileOperationType.Rename) { - // typeLabel = localize('rename', "rename"); - // } else if (fileElement.edit.type & BulkFileOperationType.Create) { - // typeLabel = localize('create', "create"); - // } - - // let label: string; - // if (typeLabel) { - // label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); - // } else { - // label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); - // } - - // this._editorService.openEditor({ - // original: { resource: leftResource }, - // modified: { resource: previewUri }, - // label, - // description: this._labelService.getUriLabel(dirname(leftResource), { relative: true }), - // options - // }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); - - // Perhaps a better way to access the parent instead of accessing through the parent of the file element - let bulkFileOperations: BulkFileOperations | undefined = undefined; let currentParent = fileElement.parent; while (true) { @@ -446,6 +406,10 @@ export class BulkEditPane extends ViewPane { }); } + // Issues with current implementation + // 1. Each time, this creates a new multi diff editor, we want it to reshow the same multi diff editor if there is one + // 2. The file naming does not look correct in the multi diff editor, there is a bug somewhere + // 3. Currently I am accessing the parent of the file element and showing all of the files, but we want to jump to the correct location when clicking on the multi diff editor const refactorSourceUri = URI.from({ scheme: 'refactor-preview' }); this.commandService.executeCommand('_workbench.openMultiDiffEditor', { refactorSourceUri, label: 'Refactor Preview', resources }); } From 5e537b389ed2a30afd0beea99e019adbc317ae74 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Fri, 9 Feb 2024 10:32:23 +0100 Subject: [PATCH 04/20] polishing the code --- extensions/git/src/commands.ts | 1 - .../standalone/browser/standaloneEditor.ts | 1 - src/vs/platform/list/browser/listService.ts | 1 - src/vs/workbench/api/common/extHostSCM.ts | 1 - .../bulkEdit/browser/preview/bulkEditPane.ts | 151 ++++++++---------- .../browser/preview/bulkEditPreview.ts | 1 - 6 files changed, 69 insertions(+), 87 deletions(-) diff --git a/extensions/git/src/commands.ts b/extensions/git/src/commands.ts index 9c6c2e1991b8..3f46f698ce22 100644 --- a/extensions/git/src/commands.ts +++ b/extensions/git/src/commands.ts @@ -3780,7 +3780,6 @@ export class CommandCenter { resources.push(toMultiFileDiffEditorUris(change, stashFirstParentCommit, modifiedUriRef)); } - // Command which is executed in order to open the multi diff editor, uring the passed in URI, the given title and the resources which are the modified resource and the original resource commands.executeCommand('_workbench.openMultiDiffEditor', { multiDiffSourceUri, title, resources }); } diff --git a/src/vs/editor/standalone/browser/standaloneEditor.ts b/src/vs/editor/standalone/browser/standaloneEditor.ts index 10a75a449042..059a4928862c 100644 --- a/src/vs/editor/standalone/browser/standaloneEditor.ts +++ b/src/vs/editor/standalone/browser/standaloneEditor.ts @@ -100,7 +100,6 @@ export function createDiffEditor(domElement: HTMLElement, options?: IStandaloneD return instantiationService.createInstance(StandaloneDiffEditor2, domElement, options); } -// There is also a function which creates a multi file diff editor, but maybe we do not need it export function createMultiFileDiffEditor(domElement: HTMLElement, override?: IEditorOverrideServices) { const instantiationService = StandaloneServices.initialize(override || {}); return new MultiDiffEditorWidget(domElement, {}, instantiationService); diff --git a/src/vs/platform/list/browser/listService.ts b/src/vs/platform/list/browser/listService.ts index 317af0fdf3a4..6857531cb668 100644 --- a/src/vs/platform/list/browser/listService.ts +++ b/src/vs/platform/list/browser/listService.ts @@ -770,7 +770,6 @@ abstract class ResourceNavigator extends Disposable { this._open(element, preserveFocus, pinned, sideBySide, browserEvent); } - // We want to actually retrieve all of the elements, not just the element at hand private _open(element: T | undefined, preserveFocus: boolean, pinned: boolean, sideBySide: boolean, browserEvent?: UIEvent): void { if (!element) { return; diff --git a/src/vs/workbench/api/common/extHostSCM.ts b/src/vs/workbench/api/common/extHostSCM.ts index 16c8268848bd..ee2be0b1bcc2 100644 --- a/src/vs/workbench/api/common/extHostSCM.ts +++ b/src/vs/workbench/api/common/extHostSCM.ts @@ -416,7 +416,6 @@ class ExtHostSourceControlResourceGroup implements vscode.SourceControlResourceG private _sourceControlHandle: number, private _id: string, private _label: string, - // The following appears to be adding multi diff editor support public readonly multiDiffEditorEnableViewChanges: boolean, private readonly _extension: IExtensionDescription, ) { } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 229662787263..5716aa676594 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -11,7 +11,6 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IThemeService } from 'vs/platform/theme/common/themeService'; import { localize } from 'vs/nls'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; import { BulkEditPreviewProvider, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; import { ILabelService } from 'vs/platform/label/common/label'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; @@ -24,7 +23,7 @@ import { IContextKeyService, RawContextKey, IContextKey } from 'vs/platform/cont import { IViewletViewOptions } from 'vs/workbench/browser/parts/views/viewsViewlet'; import { ResourceLabels, IResourceLabelsContainer } from 'vs/workbench/browser/labels'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; -import { basename, dirname } from 'vs/base/common/resources'; +import { basename } from 'vs/base/common/resources'; import { MenuId } from 'vs/platform/actions/common/actions'; import { ITreeContextMenuEvent } from 'vs/base/browser/ui/tree/tree'; import { CancellationToken } from 'vs/base/common/cancellation'; @@ -38,7 +37,7 @@ import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService'; import { ButtonBar } from 'vs/base/browser/ui/button/button'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; -import { ICommandService } from 'vs/platform/commands/common/commands'; +import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; const enum State { Data = 'data', @@ -80,7 +79,6 @@ export class BulkEditPane extends ViewPane { @IDialogService private readonly _dialogService: IDialogService, @IContextMenuService private readonly _contextMenuService: IContextMenuService, @IStorageService private readonly _storageService: IStorageService, - @ICommandService private readonly commandService: ICommandService, @IContextKeyService contextKeyService: IContextKeyService, @IViewDescriptorService viewDescriptorService: IViewDescriptorService, @IKeybindingService keybindingService: IKeybindingService, @@ -145,8 +143,7 @@ export class BulkEditPane extends ViewPane { ); this._disposables.add(this._tree.onContextMenu(this._onContextMenu, this)); - // Thing is when the tree is clicked, we want actually to show all of the files inside of the multi diff editor - this._disposables.add(this._tree.onDidOpen(e => this._openElementAsEditor(e))); + this._disposables.add(this._tree.onDidOpen(e => this._openElementInMultiDiffEditor(e))); // buttons const buttonsContainer = document.createElement('div'); @@ -226,7 +223,6 @@ export class BulkEditPane extends ViewPane { return Boolean(this._currentInput); } - // Presumably the method where we set the data to show in the tree refactors view private async _setTreeInput(input: BulkFileOperations) { const viewState = this._treeViewStates.get(this._treeDataSource.groupByFile); @@ -320,8 +316,11 @@ export class BulkEditPane extends ViewPane { } } - // In this function, we actually open the element as an editor, and this is where we could open a multi file diff editor - private async _openElementAsEditor(e: IOpenEvent): Promise { + // Issues with current implementation + // 1. Each time, this creates a new multi diff editor, we want it to reshow the same multi diff editor if there is one + // 2. The file naming does not look correct in the multi diff editor, there is a bug somewhere + // 3. Currently I am accessing the parent of the file element and showing all of the files, but we want to jump to the correct location when clicking on the multi diff editor + private async _openElementInMultiDiffEditor(e: IOpenEvent): Promise { const options: Mutable = { ...e.editorOptions }; let fileElement: FileElement; @@ -338,81 +337,69 @@ export class BulkEditPane extends ViewPane { return; } - console.log('options : ', JSON.stringify(options)); - - if (fileElement.edit.type & BulkFileOperationType.Delete) { - const previewUri = this._currentProvider!.asPreviewUri(fileElement.edit.uri); - - console.log('fileElement.edit : ', fileElement.edit); - console.log('previewUri : ', JSON.stringify(previewUri)); - - const uri = previewUri; - - const label = localize('edt.title.del', "{0} (delete, refactor preview)", basename(fileElement.edit.uri)); - const resources: { originalUri: URI | undefined; modifiedUri: URI | undefined }[] = [{ - originalUri: undefined, - modifiedUri: previewUri - }]; - this.commandService.executeCommand('_workbench.openMultiDiffEditor', { uri, label, resources }); - - } else { - console.log('fileElement.edit ; ', fileElement.edit); - - let bulkFileOperations: BulkFileOperations | undefined = undefined; - let currentParent = fileElement.parent; - while (true) { - if (currentParent instanceof BulkFileOperations) { - bulkFileOperations = currentParent; - break; - } - currentParent = currentParent.parent; + let bulkFileOperations: BulkFileOperations | undefined = undefined; + let currentParent = fileElement.parent; + while (true) { + if (currentParent instanceof BulkFileOperations) { + bulkFileOperations = currentParent; + break; } - const allBulkFileOperations = bulkFileOperations.fileOperations; - console.log('allBulkFileOperations : ', allBulkFileOperations); - - const resources: { originalUri: URI | undefined; modifiedUri: URI | undefined }[] = []; - - for (const operation of allBulkFileOperations) { - const previewUri = this._currentProvider!.asPreviewUri(operation.textEdits[0].textEdit.resource); - - let leftResource: URI | undefined; - try { - (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); - leftResource = fileElement.edit.uri; - } catch { - leftResource = BulkEditPreviewProvider.emptyPreview; - } - - let typeLabel: string | undefined; - if (fileElement.edit.type & BulkFileOperationType.Rename) { - typeLabel = localize('rename', "rename"); - } else if (fileElement.edit.type & BulkFileOperationType.Create) { - typeLabel = localize('create', "create"); - } - - let label: string; - if (typeLabel) { - label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); - } else { - label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); - } - - console.log('leftResource : ', JSON.stringify(leftResource)); - console.log('previewUri : ', JSON.stringify(previewUri)); - - resources.push({ - originalUri: leftResource, - modifiedUri: previewUri - }); - } - - // Issues with current implementation - // 1. Each time, this creates a new multi diff editor, we want it to reshow the same multi diff editor if there is one - // 2. The file naming does not look correct in the multi diff editor, there is a bug somewhere - // 3. Currently I am accessing the parent of the file element and showing all of the files, but we want to jump to the correct location when clicking on the multi diff editor - const refactorSourceUri = URI.from({ scheme: 'refactor-preview' }); - this.commandService.executeCommand('_workbench.openMultiDiffEditor', { refactorSourceUri, label: 'Refactor Preview', resources }); + currentParent = currentParent.parent; } + const fileOperations = bulkFileOperations.fileOperations; + + const resources = []; + for (const operation of fileOperations) { + const previewUri = this._currentProvider!.asPreviewUri(operation.textEdits[0].textEdit.resource); + + let leftResource: URI | undefined; + try { + (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); + leftResource = fileElement.edit.uri; + } catch { + leftResource = BulkEditPreviewProvider.emptyPreview; + } + resources.push({ + originalUri: leftResource, + modifiedUri: previewUri + }); + + console.log('leftResource : ', JSON.stringify(leftResource)); + console.log('previewUri : ', JSON.stringify(previewUri)); + } + + let typeLabel: string | undefined; + if (fileElement.edit.type & BulkFileOperationType.Rename) { + typeLabel = localize('rename', "rename"); + } else if (fileElement.edit.type & BulkFileOperationType.Create) { + typeLabel = localize('create', "create"); + } + + let label: string; + if (typeLabel) { + label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); + } else { + label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); + } + + const multiDiffSource = URI.from({ scheme: 'refactor-preview', path: JSON.stringify(fileElement.edit.uri) }); + const description = 'Refactor Preview'; + + console.log('options : ', JSON.stringify(options)); + console.log('fileOperations : ', fileOperations); + console.log('fileElement.edit ; ', fileElement.edit); + console.log('multiDiffSource : ', multiDiffSource); + console.log('resources : ', resources); + console.log('label : ', label); + console.log('description : ', description); + + this._editorService.openEditor({ + multiDiffSource: multiDiffSource ? URI.revive(multiDiffSource) : undefined, + resources: resources?.map(r => ({ original: { resource: URI.revive(r.originalUri) }, modified: { resource: URI.revive(r.modifiedUri) } })), + label: label, + description: description, + options: options, + }); } private _onContextMenu(e: ITreeContextMenuEvent): void { diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts index c649d96c78cf..d9e8a1112a0d 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview.ts @@ -307,7 +307,6 @@ export class BulkFileOperations { return result; } - // Getting the edits for a specific file getFileEdits(uri: URI): ISingleEditOperation[] { for (const file of this.fileOperations) { From 914f33d15bbdfef34291083f49cf4c2f04c2eef7 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Fri, 9 Feb 2024 10:43:31 +0100 Subject: [PATCH 05/20] updating correctly the right resource too --- .../contrib/bulkEdit/browser/preview/bulkEditPane.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 5716aa676594..9f7077d24ac2 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -350,22 +350,22 @@ export class BulkEditPane extends ViewPane { const resources = []; for (const operation of fileOperations) { - const previewUri = this._currentProvider!.asPreviewUri(operation.textEdits[0].textEdit.resource); - let leftResource: URI | undefined; + let leftResource: URI | undefined = operation.textEdits[0].textEdit.resource; + let rightResource: URI | undefined = undefined; try { - (await this._textModelService.createModelReference(fileElement.edit.uri)).dispose(); - leftResource = fileElement.edit.uri; + (await this._textModelService.createModelReference(leftResource)).dispose(); + rightResource = this._currentProvider!.asPreviewUri(leftResource); } catch { leftResource = BulkEditPreviewProvider.emptyPreview; } resources.push({ originalUri: leftResource, - modifiedUri: previewUri + modifiedUri: rightResource }); console.log('leftResource : ', JSON.stringify(leftResource)); - console.log('previewUri : ', JSON.stringify(previewUri)); + console.log('rightResource : ', JSON.stringify(rightResource)); } let typeLabel: string | undefined; From 46054bc4377beb80277466c2a915e67e2138363b Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Fri, 9 Feb 2024 13:16:31 +0100 Subject: [PATCH 06/20] setting the height --- .../multiDiffEditorWidget.ts | 14 ++++++++- .../multiDiffEditorWidgetImpl.ts | 19 +++++++++++- .../bulkEdit/browser/preview/bulkEditPane.ts | 30 +++++++++++++++---- .../browser/multiDiffEditor.ts | 14 ++++++++- 4 files changed, 68 insertions(+), 9 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index 938edfad4b33..349c26b5ee38 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -8,7 +8,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { derived, derivedWithStore, observableValue, recomputeInitiallyAndOnChange } from 'vs/base/common/observable'; import { readHotReloadableExport } from 'vs/editor/browser/widget/diffEditor/utils'; import { IMultiDiffEditorModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/model'; -import { IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl, VirtualizedViewItem } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { MultiDiffEditorViewModel } from './multiDiffEditorViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import './colors'; @@ -44,6 +44,18 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } + public setScrollState(scrollState: { top?: number; left?: number }): void { + this._widgetImpl.get().setScrollState(scrollState); + } + + public getTopOfElement(index: number): number { + return this._widgetImpl.get().getTopOfElement(index); + } + + public viewItems(): readonly VirtualizedViewItem[] { + return this._widgetImpl.get().viewItems(); + } + public createViewModel(model: IMultiDiffEditorModel): MultiDiffEditorViewModel { return new MultiDiffEditorViewModel(model, this._instantiationService); } diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 129d91151b1a..54818d55c359 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -186,6 +186,21 @@ export class MultiDiffEditorWidgetImpl extends Disposable { this._scrollableElement.setScrollPosition({ scrollLeft: scrollState.left, scrollTop: scrollState.top }); } + public getTopOfElement(index: number): number { + console.log('index : ', index); + const viewItems = this._viewItems.get(); + console.log('viewItems : ', viewItems); + let top = 0; + for (let i = 0; i < index - 1; i++) { + top += viewItems[i].contentHeight.get() + this._spaceBetweenPx; + } + return top; + } + + public viewItems(): readonly VirtualizedViewItem[] { + return this._viewItems.get(); + } + public getViewState(): IMultiDiffEditorViewState { return { scrollState: { @@ -277,9 +292,10 @@ export interface IMultiDiffEditorViewState { interface IMultiDiffDocState { collapsed: boolean; selections?: ISelection[]; + uri?: URI; } -class VirtualizedViewItem extends Disposable { +export class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); public readonly contentHeight = derived(this, reader => @@ -336,6 +352,7 @@ class VirtualizedViewItem extends Disposable { return { collapsed: this.viewModel.collapsed.get(), selections: this.viewModel.lastTemplateData.get().selections, + uri: this.viewModel.originalUri }; } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 9f7077d24ac2..7a8faba7ea4b 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -11,7 +11,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IThemeService } from 'vs/platform/theme/common/themeService'; import { localize } from 'vs/nls'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { BulkEditPreviewProvider, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; +import { BulkEditPreviewProvider, BulkFileOperation, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; import { ILabelService } from 'vs/platform/label/common/label'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { URI } from 'vs/base/common/uri'; @@ -38,6 +38,7 @@ import { ButtonBar } from 'vs/base/browser/ui/button/button'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; +import { MultiDiffEditor } from 'vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor'; const enum State { Data = 'data', @@ -68,7 +69,8 @@ export class BulkEditPane extends ViewPane { private _currentResolve?: (edit?: ResourceEdit[]) => void; private _currentInput?: BulkFileOperations; private _currentProvider?: BulkEditPreviewProvider; - + private _multiDiffEditor?: MultiDiffEditor; + private _fileOperations?: BulkFileOperation[]; constructor( options: IViewletViewOptions, @@ -317,9 +319,9 @@ export class BulkEditPane extends ViewPane { } // Issues with current implementation - // 1. Each time, this creates a new multi diff editor, we want it to reshow the same multi diff editor if there is one - // 2. The file naming does not look correct in the multi diff editor, there is a bug somewhere // 3. Currently I am accessing the parent of the file element and showing all of the files, but we want to jump to the correct location when clicking on the multi diff editor + // Maybe we should somehow return the file operations directly instead of iterating upwards, the way that we currently do + // 4. Should jump instead to the part of the multi diff editor of interest, so no need to show it again, and can just scroll private async _openElementInMultiDiffEditor(e: IOpenEvent): Promise { const options: Mutable = { ...e.editorOptions }; @@ -348,6 +350,22 @@ export class BulkEditPane extends ViewPane { } const fileOperations = bulkFileOperations.fileOperations; + if (this._fileOperations === fileOperations) { + // Multi diff editor already open + const viewItems = this._multiDiffEditor?.viewItems(); + if (viewItems) { + const item = viewItems?.find(item => item.viewModel.originalUri?.toString() === fileElement.edit.uri.toString()); + if (item) { + const index = viewItems.indexOf(item); + const top = this._multiDiffEditor?.getTopOfElement(index); + console.log('top : ', top); + this._multiDiffEditor?.setScrollState({ top }); + } + } + return; + } + this._fileOperations = fileOperations; + const resources = []; for (const operation of fileOperations) { @@ -393,13 +411,13 @@ export class BulkEditPane extends ViewPane { console.log('label : ', label); console.log('description : ', description); - this._editorService.openEditor({ + this._multiDiffEditor = await this._editorService.openEditor({ multiDiffSource: multiDiffSource ? URI.revive(multiDiffSource) : undefined, resources: resources?.map(r => ({ original: { resource: URI.revive(r.originalUri) }, modified: { resource: URI.revive(r.modifiedUri) } })), label: label, description: description, options: options, - }); + }) as MultiDiffEditor; } private _onContextMenu(e: ITreeContextMenuEvent): void { diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 263668667598..9361ee7e0efb 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -24,7 +24,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { URI } from 'vs/base/common/uri'; import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorViewModel'; -import { IMultiDiffEditorViewState } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, VirtualizedViewItem } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; @@ -72,6 +72,18 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { await super.setInput(input, options, context, token); this._viewModel = await input.getViewModel(); From 23704c7fa53d5fd08d163de682a1423db89fc0a1 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Mon, 12 Feb 2024 15:51:28 +0100 Subject: [PATCH 07/20] using instead scrollTo method --- .../multiDiffEditorWidget.ts | 14 ++------ .../multiDiffEditorWidgetImpl.ts | 21 ++++-------- .../bulkEdit/browser/preview/bulkEditPane.ts | 33 ++++--------------- .../browser/multiDiffEditor.ts | 14 ++------ 4 files changed, 19 insertions(+), 63 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index 349c26b5ee38..9a19e5e5669e 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -8,7 +8,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { derived, derivedWithStore, observableValue, recomputeInitiallyAndOnChange } from 'vs/base/common/observable'; import { readHotReloadableExport } from 'vs/editor/browser/widget/diffEditor/utils'; import { IMultiDiffEditorModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/model'; -import { IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl, VirtualizedViewItem } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { MultiDiffEditorViewModel } from './multiDiffEditorViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import './colors'; @@ -44,16 +44,8 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public setScrollState(scrollState: { top?: number; left?: number }): void { - this._widgetImpl.get().setScrollState(scrollState); - } - - public getTopOfElement(index: number): number { - return this._widgetImpl.get().getTopOfElement(index); - } - - public viewItems(): readonly VirtualizedViewItem[] { - return this._widgetImpl.get().viewItems(); + public scrollTo(uri: URI): void { + this._widgetImpl.get().scrollTo(uri); } public createViewModel(model: IMultiDiffEditorModel): MultiDiffEditorViewModel { diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 54818d55c359..5619b08b2cd5 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -186,19 +186,14 @@ export class MultiDiffEditorWidgetImpl extends Disposable { this._scrollableElement.setScrollPosition({ scrollLeft: scrollState.left, scrollTop: scrollState.top }); } - public getTopOfElement(index: number): number { - console.log('index : ', index); + public scrollTo(originalUri: URI): void { const viewItems = this._viewItems.get(); - console.log('viewItems : ', viewItems); - let top = 0; - for (let i = 0; i < index - 1; i++) { - top += viewItems[i].contentHeight.get() + this._spaceBetweenPx; + const index = viewItems?.findIndex(item => item.viewModel.originalUri?.toString() === originalUri.toString()); + let scrollTop = 0; + for (let i = 0; i < index; i++) { + scrollTop += viewItems[i].contentHeight.get() + this._spaceBetweenPx; } - return top; - } - - public viewItems(): readonly VirtualizedViewItem[] { - return this._viewItems.get(); + this._scrollableElement.setScrollPosition({ scrollTop }); } public getViewState(): IMultiDiffEditorViewState { @@ -292,10 +287,9 @@ export interface IMultiDiffEditorViewState { interface IMultiDiffDocState { collapsed: boolean; selections?: ISelection[]; - uri?: URI; } -export class VirtualizedViewItem extends Disposable { +class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); public readonly contentHeight = derived(this, reader => @@ -352,7 +346,6 @@ export class VirtualizedViewItem extends Disposable { return { collapsed: this.viewModel.collapsed.get(), selections: this.viewModel.lastTemplateData.get().selections, - uri: this.viewModel.originalUri }; } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 7a8faba7ea4b..b072acb88e07 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -318,10 +318,6 @@ export class BulkEditPane extends ViewPane { } } - // Issues with current implementation - // 3. Currently I am accessing the parent of the file element and showing all of the files, but we want to jump to the correct location when clicking on the multi diff editor - // Maybe we should somehow return the file operations directly instead of iterating upwards, the way that we currently do - // 4. Should jump instead to the part of the multi diff editor of interest, so no need to show it again, and can just scroll private async _openElementInMultiDiffEditor(e: IOpenEvent): Promise { const options: Mutable = { ...e.editorOptions }; @@ -352,16 +348,7 @@ export class BulkEditPane extends ViewPane { if (this._fileOperations === fileOperations) { // Multi diff editor already open - const viewItems = this._multiDiffEditor?.viewItems(); - if (viewItems) { - const item = viewItems?.find(item => item.viewModel.originalUri?.toString() === fileElement.edit.uri.toString()); - if (item) { - const index = viewItems.indexOf(item); - const top = this._multiDiffEditor?.getTopOfElement(index); - console.log('top : ', top); - this._multiDiffEditor?.setScrollState({ top }); - } - } + this._multiDiffEditor?.scrollTo(fileElement.edit.uri); return; } this._fileOperations = fileOperations; @@ -381,9 +368,6 @@ export class BulkEditPane extends ViewPane { originalUri: leftResource, modifiedUri: rightResource }); - - console.log('leftResource : ', JSON.stringify(leftResource)); - console.log('rightResource : ', JSON.stringify(rightResource)); } let typeLabel: string | undefined; @@ -403,21 +387,16 @@ export class BulkEditPane extends ViewPane { const multiDiffSource = URI.from({ scheme: 'refactor-preview', path: JSON.stringify(fileElement.edit.uri) }); const description = 'Refactor Preview'; - console.log('options : ', JSON.stringify(options)); - console.log('fileOperations : ', fileOperations); - console.log('fileElement.edit ; ', fileElement.edit); - console.log('multiDiffSource : ', multiDiffSource); - console.log('resources : ', resources); - console.log('label : ', label); - console.log('description : ', description); - - this._multiDiffEditor = await this._editorService.openEditor({ + console.log('before open editor'); + this._multiDiffEditor = this._disposables.add(await this._editorService.openEditor({ multiDiffSource: multiDiffSource ? URI.revive(multiDiffSource) : undefined, resources: resources?.map(r => ({ original: { resource: URI.revive(r.originalUri) }, modified: { resource: URI.revive(r.modifiedUri) } })), label: label, description: description, options: options, - }) as MultiDiffEditor; + }) as MultiDiffEditor); + + } private _onContextMenu(e: ITreeContextMenuEvent): void { diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 9361ee7e0efb..21b1d64a6fac 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -24,7 +24,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { URI } from 'vs/base/common/uri'; import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorViewModel'; -import { IMultiDiffEditorViewState, VirtualizedViewItem } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; @@ -72,16 +72,8 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { From 4f727cb9852caf9bb037344216a8e36d2cc0be66 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Mon, 12 Feb 2024 17:29:32 +0100 Subject: [PATCH 08/20] polishing the code --- .../bulkEdit/browser/preview/bulkEditPane.ts | 100 ++++++++---------- 1 file changed, 43 insertions(+), 57 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index b072acb88e07..49cef9c8c8ae 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -11,7 +11,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IThemeService } from 'vs/platform/theme/common/themeService'; import { localize } from 'vs/nls'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { BulkEditPreviewProvider, BulkFileOperation, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; +import { BulkEditPreviewProvider, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; import { ILabelService } from 'vs/platform/label/common/label'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { URI } from 'vs/base/common/uri'; @@ -23,7 +23,6 @@ import { IContextKeyService, RawContextKey, IContextKey } from 'vs/platform/cont import { IViewletViewOptions } from 'vs/workbench/browser/parts/views/viewsViewlet'; import { ResourceLabels, IResourceLabelsContainer } from 'vs/workbench/browser/labels'; import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; -import { basename } from 'vs/base/common/resources'; import { MenuId } from 'vs/platform/actions/common/actions'; import { ITreeContextMenuEvent } from 'vs/base/browser/ui/tree/tree'; import { CancellationToken } from 'vs/base/common/cancellation'; @@ -39,6 +38,7 @@ import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { MultiDiffEditor } from 'vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor'; +import { IResourceDiffEditorInput } from 'vs/workbench/common/editor'; const enum State { Data = 'data', @@ -70,7 +70,6 @@ export class BulkEditPane extends ViewPane { private _currentInput?: BulkFileOperations; private _currentProvider?: BulkEditPreviewProvider; private _multiDiffEditor?: MultiDiffEditor; - private _fileOperations?: BulkFileOperation[]; constructor( options: IViewletViewOptions, @@ -104,6 +103,7 @@ export class BulkEditPane extends ViewPane { override dispose(): void { this._tree.dispose(); this._disposables.dispose(); + this._multiDiffEditor = undefined; super.dispose(); } @@ -268,6 +268,7 @@ export class BulkEditPane extends ViewPane { this._dialogService.warn(message).finally(() => this._done(false)); } + // Going through here to discard discard() { this._done(false); } @@ -320,6 +321,11 @@ export class BulkEditPane extends ViewPane { private async _openElementInMultiDiffEditor(e: IOpenEvent): Promise { + const fileOperations = this._currentInput?.fileOperations; + if (!fileOperations) { + return; + } + const options: Mutable = { ...e.editorOptions }; let fileElement: FileElement; if (e.element instanceof TextEditElement) { @@ -335,68 +341,48 @@ export class BulkEditPane extends ViewPane { return; } - let bulkFileOperations: BulkFileOperations | undefined = undefined; - let currentParent = fileElement.parent; - while (true) { - if (currentParent instanceof BulkFileOperations) { - bulkFileOperations = currentParent; - break; - } - currentParent = currentParent.parent; - } - const fileOperations = bulkFileOperations.fileOperations; - - if (this._fileOperations === fileOperations) { - // Multi diff editor already open - this._multiDiffEditor?.scrollTo(fileElement.edit.uri); + if (this._multiDiffEditor) { + // Multi diff editor already visible + this._multiDiffEditor.scrollTo(fileElement.edit.uri); return; } - this._fileOperations = fileOperations; - const resources = []; + const resources: IResourceDiffEditorInput[] = []; for (const operation of fileOperations) { + const operationUri = operation.uri; + const previewUri = this._currentProvider!.asPreviewUri(operationUri); + // delete -> show single editor + if (operation.type & BulkFileOperationType.Delete) { + resources.push({ + original: { resource: undefined }, + modified: { resource: URI.revive(previewUri) } + }); - let leftResource: URI | undefined = operation.textEdits[0].textEdit.resource; - let rightResource: URI | undefined = undefined; - try { - (await this._textModelService.createModelReference(leftResource)).dispose(); - rightResource = this._currentProvider!.asPreviewUri(leftResource); - } catch { - leftResource = BulkEditPreviewProvider.emptyPreview; + } else { + // rename, create, edits -> show diff editr + let leftResource: URI | undefined; + try { + (await this._textModelService.createModelReference(operationUri)).dispose(); + leftResource = operationUri; + } catch { + leftResource = BulkEditPreviewProvider.emptyPreview; + } + resources.push({ + original: { resource: URI.revive(leftResource) }, + modified: { resource: URI.revive(previewUri) } + }); } - resources.push({ - originalUri: leftResource, - modifiedUri: rightResource - }); } - - let typeLabel: string | undefined; - if (fileElement.edit.type & BulkFileOperationType.Rename) { - typeLabel = localize('rename', "rename"); - } else if (fileElement.edit.type & BulkFileOperationType.Create) { - typeLabel = localize('create', "create"); - } - - let label: string; - if (typeLabel) { - label = localize('edt.title.2', "{0} ({1}, refactor preview)", basename(fileElement.edit.uri), typeLabel); - } else { - label = localize('edt.title.1', "{0} (refactor preview)", basename(fileElement.edit.uri)); - } - const multiDiffSource = URI.from({ scheme: 'refactor-preview', path: JSON.stringify(fileElement.edit.uri) }); - const description = 'Refactor Preview'; - - console.log('before open editor'); - this._multiDiffEditor = this._disposables.add(await this._editorService.openEditor({ - multiDiffSource: multiDiffSource ? URI.revive(multiDiffSource) : undefined, - resources: resources?.map(r => ({ original: { resource: URI.revive(r.originalUri) }, modified: { resource: URI.revive(r.modifiedUri) } })), - label: label, - description: description, - options: options, - }) as MultiDiffEditor); - - + const label = 'Refactor Preview'; + this._multiDiffEditor = this._sessionDisposables.add( + await this._editorService.openEditor({ + multiDiffSource: multiDiffSource ? URI.revive(multiDiffSource) : undefined, + resources, + label: label, + description: label, + options: options, + }) as MultiDiffEditor); } private _onContextMenu(e: ITreeContextMenuEvent): void { From 7b0e8858b37d05e44907378e5e61fe5f524a198d Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Tue, 13 Feb 2024 09:35:47 +0100 Subject: [PATCH 09/20] polishing code --- .../bulkEdit/browser/preview/bulkEditPane.ts | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 49cef9c8c8ae..3a9b9ebab13a 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -98,6 +98,11 @@ export class BulkEditPane extends ViewPane { this._ctxHasCategories = BulkEditPane.ctxHasCategories.bindTo(contextKeyService); this._ctxGroupByFile = BulkEditPane.ctxGroupByFile.bindTo(contextKeyService); this._ctxHasCheckedChanges = BulkEditPane.ctxHasCheckedChanges.bindTo(contextKeyService); + this._disposables.add(this._editorService.onDidCloseEditor((e) => { + if (this._multiDiffEditor && e.editor === this._multiDiffEditor.input) { + this._multiDiffEditor = undefined; + } + })); } override dispose(): void { @@ -268,7 +273,6 @@ export class BulkEditPane extends ViewPane { this._dialogService.warn(message).finally(() => this._done(false)); } - // Going through here to discard discard() { this._done(false); } @@ -278,6 +282,10 @@ export class BulkEditPane extends ViewPane { this._currentInput = undefined; this._setState(State.Message); this._sessionDisposables.clear(); + if (this._multiDiffEditor && this._multiDiffEditor.input && this._multiDiffEditor.group) { + this._editorService.closeEditor({ editor: this._multiDiffEditor.input, groupId: this._multiDiffEditor.group.id }); + } + this._multiDiffEditor = undefined; } toggleChecked() { @@ -375,14 +383,14 @@ export class BulkEditPane extends ViewPane { } const multiDiffSource = URI.from({ scheme: 'refactor-preview', path: JSON.stringify(fileElement.edit.uri) }); const label = 'Refactor Preview'; - this._multiDiffEditor = this._sessionDisposables.add( + this._multiDiffEditor = await this._editorService.openEditor({ - multiDiffSource: multiDiffSource ? URI.revive(multiDiffSource) : undefined, + multiDiffSource: URI.revive(multiDiffSource), resources, label: label, description: label, options: options, - }) as MultiDiffEditor); + }) as MultiDiffEditor; } private _onContextMenu(e: ITreeContextMenuEvent): void { From 0776d4f78f4ec9b974990d5414e332ebd82232ba Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Tue, 13 Feb 2024 15:38:07 +0100 Subject: [PATCH 10/20] polishing the code --- .../bulkEdit/browser/preview/bulkEditPane.ts | 23 ++++++++----------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 3a9b9ebab13a..2d8031cbe778 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -99,7 +99,7 @@ export class BulkEditPane extends ViewPane { this._ctxGroupByFile = BulkEditPane.ctxGroupByFile.bindTo(contextKeyService); this._ctxHasCheckedChanges = BulkEditPane.ctxHasCheckedChanges.bindTo(contextKeyService); this._disposables.add(this._editorService.onDidCloseEditor((e) => { - if (this._multiDiffEditor && e.editor === this._multiDiffEditor.input) { + if (this._multiDiffEditor && e.editor === this._multiDiffEditor.input && e.groupId === this._multiDiffEditor.group?.id) { this._multiDiffEditor = undefined; } })); @@ -108,7 +108,6 @@ export class BulkEditPane extends ViewPane { override dispose(): void { this._tree.dispose(); this._disposables.dispose(); - this._multiDiffEditor = undefined; super.dispose(); } @@ -284,8 +283,8 @@ export class BulkEditPane extends ViewPane { this._sessionDisposables.clear(); if (this._multiDiffEditor && this._multiDiffEditor.input && this._multiDiffEditor.group) { this._editorService.closeEditor({ editor: this._multiDiffEditor.input, groupId: this._multiDiffEditor.group.id }); + this._multiDiffEditor = undefined; } - this._multiDiffEditor = undefined; } toggleChecked() { @@ -333,7 +332,6 @@ export class BulkEditPane extends ViewPane { if (!fileOperations) { return; } - const options: Mutable = { ...e.editorOptions }; let fileElement: FileElement; if (e.element instanceof TextEditElement) { @@ -381,16 +379,15 @@ export class BulkEditPane extends ViewPane { }); } } - const multiDiffSource = URI.from({ scheme: 'refactor-preview', path: JSON.stringify(fileElement.edit.uri) }); + const multiDiffSource = URI.from({ scheme: 'refactor-preview' }); const label = 'Refactor Preview'; - this._multiDiffEditor = - await this._editorService.openEditor({ - multiDiffSource: URI.revive(multiDiffSource), - resources, - label: label, - description: label, - options: options, - }) as MultiDiffEditor; + this._multiDiffEditor = await this._editorService.openEditor({ + multiDiffSource, + resources, + label, + description: label, + options, + }) as MultiDiffEditor; } private _onContextMenu(e: ITreeContextMenuEvent): void { From 5f33bf34ce74baeebfa231dfd3dc2f15a247ae5c Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Tue, 13 Feb 2024 17:51:04 +0100 Subject: [PATCH 11/20] wip --- .../multiDiffEditorWidget.ts | 4 +- .../multiDiffEditorWidgetImpl.ts | 32 ++++++++++-- src/vs/workbench/common/editor.ts | 5 ++ .../bulkEdit/browser/preview/bulkEditPane.ts | 49 +++++++++---------- .../browser/multiDiffEditor.ts | 4 +- .../services/editor/common/editorService.ts | 1 - 6 files changed, 61 insertions(+), 34 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index 9a19e5e5669e..e2392cc8b4a1 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -44,8 +44,8 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public scrollTo(uri: URI): void { - this._widgetImpl.get().scrollTo(uri); + public reveal(resource: { original: URI } | { modified: URI }, lineNumber: number): void { + this._widgetImpl.get().reveal(resource, lineNumber); } public createViewModel(model: IMultiDiffEditorModel): MultiDiffEditorViewModel { diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 5619b08b2cd5..b8547c7d347b 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -25,6 +25,7 @@ import { ISelection, Selection } from 'vs/editor/common/core/selection'; import { URI } from 'vs/base/common/uri'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; +import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; export class MultiDiffEditorWidgetImpl extends Disposable { private readonly _elements = h('div.monaco-component.multiDiffEditor', [ @@ -104,6 +105,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { private readonly _workbenchUIElementFactory: IWorkbenchUIElementFactory, @IContextKeyService private readonly _parentContextKeyService: IContextKeyService, @IInstantiationService private readonly _parentInstantiationService: IInstantiationService, + @IConfigurationService private readonly _configurationService: IConfigurationService, ) { super(); @@ -186,10 +188,20 @@ export class MultiDiffEditorWidgetImpl extends Disposable { this._scrollableElement.setScrollPosition({ scrollLeft: scrollState.left, scrollTop: scrollState.top }); } - public scrollTo(originalUri: URI): void { + public reveal(resource: IMultiDiffResource, lineNumber: number): void { const viewItems = this._viewItems.get(); - const index = viewItems?.findIndex(item => item.viewModel.originalUri?.toString() === originalUri.toString()); - let scrollTop = 0; + let searchCallback: (item: VirtualizedViewItem) => boolean; + if (isMultiDiffOriginalResourceUri(resource)) { + searchCallback = (item) => item.viewModel.originalUri?.toString() === resource.original.toString(); + } else { + searchCallback = (item) => item.viewModel.modifiedUri?.toString() === resource.modified.toString(); + } + const index = viewItems.findIndex(searchCallback); + const scrollTopWithinItem = (lineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); + // todo@aiday-mar: need to find the actual scroll top given the line number specific to the original or modified uri + // following does not neccessarily correspond to the appropriate scroll top within the editor + const maxScroll = viewItems[index].template.get()?.maxScroll.get().maxScroll; + let scrollTop = (maxScroll && scrollTopWithinItem < maxScroll) ? scrollTopWithinItem : 0; for (let i = 0; i < index; i++) { scrollTop += viewItems[i].contentHeight.get() + this._spaceBetweenPx; } @@ -289,6 +301,20 @@ interface IMultiDiffDocState { selections?: ISelection[]; } +interface IMultiDiffOriginalResource { + original: URI; +} + +interface IMultiDiffModifiedResource { + modified: URI; +} + +function isMultiDiffOriginalResourceUri(obj: any): obj is IMultiDiffOriginalResource { + return 'original' in obj && obj.original instanceof URI; +} + +type IMultiDiffResource = IMultiDiffOriginalResource | IMultiDiffModifiedResource; + class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index bf6785d96751..322582ad33c7 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -505,6 +505,11 @@ export interface IResourceMultiDiffEditorInput extends IBaseUntypedEditorInput { * If not set, the resources are dynamically derived from the {@link multiDiffSource}. */ readonly resources?: IResourceDiffEditorInput[]; + + /** + * Reveal the following resource on open + */ + readonly revealResource?: IResourceDiffEditorInput; } export type IResourceMergeEditorInputSide = (IResourceEditorInput | ITextResourceEditorInput) & { detail?: string }; diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 2d8031cbe778..ff0cc489d280 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -11,7 +11,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IThemeService } from 'vs/platform/theme/common/themeService'; import { localize } from 'vs/nls'; import { DisposableStore } from 'vs/base/common/lifecycle'; -import { BulkEditPreviewProvider, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; +import { BulkEditPreviewProvider, BulkFileOperation, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; import { ILabelService } from 'vs/platform/label/common/label'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { URI } from 'vs/base/common/uri'; @@ -36,8 +36,7 @@ import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService'; import { ButtonBar } from 'vs/base/browser/ui/button/button'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; -import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; -import { MultiDiffEditor } from 'vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor'; +import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; import { IResourceDiffEditorInput } from 'vs/workbench/common/editor'; const enum State { @@ -69,7 +68,8 @@ export class BulkEditPane extends ViewPane { private _currentResolve?: (edit?: ResourceEdit[]) => void; private _currentInput?: BulkFileOperations; private _currentProvider?: BulkEditPreviewProvider; - private _multiDiffEditor?: MultiDiffEditor; + private _fileOperations?: BulkFileOperation[]; + private _resources?: IResourceDiffEditorInput[]; constructor( options: IViewletViewOptions, @@ -98,11 +98,6 @@ export class BulkEditPane extends ViewPane { this._ctxHasCategories = BulkEditPane.ctxHasCategories.bindTo(contextKeyService); this._ctxGroupByFile = BulkEditPane.ctxGroupByFile.bindTo(contextKeyService); this._ctxHasCheckedChanges = BulkEditPane.ctxHasCheckedChanges.bindTo(contextKeyService); - this._disposables.add(this._editorService.onDidCloseEditor((e) => { - if (this._multiDiffEditor && e.editor === this._multiDiffEditor.input && e.groupId === this._multiDiffEditor.group?.id) { - this._multiDiffEditor = undefined; - } - })); } override dispose(): void { @@ -281,10 +276,6 @@ export class BulkEditPane extends ViewPane { this._currentInput = undefined; this._setState(State.Message); this._sessionDisposables.clear(); - if (this._multiDiffEditor && this._multiDiffEditor.input && this._multiDiffEditor.group) { - this._editorService.closeEditor({ editor: this._multiDiffEditor.input, groupId: this._multiDiffEditor.group.id }); - this._multiDiffEditor = undefined; - } } toggleChecked() { @@ -347,12 +338,26 @@ export class BulkEditPane extends ViewPane { return; } - if (this._multiDiffEditor) { - // Multi diff editor already visible - this._multiDiffEditor.scrollTo(fileElement.edit.uri); - return; + let resources: IResourceDiffEditorInput[]; + if (this._fileOperations === fileOperations && this._resources) { + resources = this._resources; + } else { + resources = await this._getResources(fileOperations); } + const revealResource = resources.find(r => r.original.resource!.toString() === fileElement.edit.uri.toString()); + const multiDiffSource = URI.from({ scheme: 'refactor-preview' }); + const label = 'Refactor Preview'; + this._editorService.openEditor({ + multiDiffSource, + revealResource, + resources, + label, + description: label, + options, + }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); + } + private async _getResources(fileOperations: BulkFileOperation[]): Promise { const resources: IResourceDiffEditorInput[] = []; for (const operation of fileOperations) { const operationUri = operation.uri; @@ -379,15 +384,7 @@ export class BulkEditPane extends ViewPane { }); } } - const multiDiffSource = URI.from({ scheme: 'refactor-preview' }); - const label = 'Refactor Preview'; - this._multiDiffEditor = await this._editorService.openEditor({ - multiDiffSource, - resources, - label, - description: label, - options, - }) as MultiDiffEditor; + return resources; } private _onContextMenu(e: ITreeContextMenuEvent): void { diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 21b1d64a6fac..be2fab931756 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -72,8 +72,8 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { diff --git a/src/vs/workbench/services/editor/common/editorService.ts b/src/vs/workbench/services/editor/common/editorService.ts index 1be2060f98c7..19b51a652c7f 100644 --- a/src/vs/workbench/services/editor/common/editorService.ts +++ b/src/vs/workbench/services/editor/common/editorService.ts @@ -236,7 +236,6 @@ export interface IEditorService { * Open an editor in an editor group. * * @param editor the editor to open - * @param options the options to use for the editor * @param group the target group. If unspecified, the editor will open in the currently * active group. Use `SIDE_GROUP` to open the editor in a new editor group to the side * of the currently active group. From cb5ae542533b19e7a79364902d21c123c145e7ca Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Wed, 14 Feb 2024 12:13:27 +0100 Subject: [PATCH 12/20] adding review changes --- .../browser/multiDiffEditor.ts | 12 ++++++++++ .../browser/multiDiffEditorInput.ts | 24 +++++++++++++++++-- .../browser/multiDiffSourceResolverService.ts | 8 +++++++ 3 files changed, 42 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index be2fab931756..d32ed95ec3d0 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -85,6 +85,18 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts index 907fd6e15fe8..f7dae8fc3e01 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditorInput.ts @@ -49,6 +49,10 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor resource.modified.resource, ); }), + input.revealResource ? new MultiDiffEditorItem( + input.revealResource.original.resource, + input.revealResource.modified.resource + ) : undefined, ); } @@ -60,7 +64,11 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor data.resources?.map(resource => new MultiDiffEditorItem( resource.originalUri ? URI.parse(resource.originalUri) : undefined, resource.modifiedUri ? URI.parse(resource.modifiedUri) : undefined, - )) + )), + data.revealResource ? new MultiDiffEditorItem( + data.revealResource.originalUri ? URI.parse(data.revealResource.originalUri) : undefined, + data.revealResource.modifiedUri ? URI.parse(data.revealResource.modifiedUri) : undefined, + ) : undefined ); } @@ -81,6 +89,7 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor public readonly multiDiffSource: URI, public readonly label: string | undefined, public readonly initialResources: readonly MultiDiffEditorItem[] | undefined, + public readonly initialResourceToReveal: MultiDiffEditorItem | undefined, @ITextModelService private readonly _textModelService: ITextModelService, @ITextResourceConfigurationService private readonly _textResourceConfigurationService: ITextResourceConfigurationService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @@ -106,6 +115,10 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor originalUri: resource.original?.toString(), modifiedUri: resource.modified?.toString(), })), + revealResource: this.initialResourceToReveal ? { + originalUri: this.initialResourceToReveal.original?.toString(), + modifiedUri: this.initialResourceToReveal.modified?.toString(), + } : undefined }; } @@ -224,7 +237,10 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor } if (otherInput instanceof MultiDiffEditorInput) { - return this.multiDiffSource.toString() === otherInput.multiDiffSource.toString(); + const initialResourcesEqual = (this.initialResourceToReveal && otherInput.initialResourceToReveal + && this.initialResourceToReveal.equals(otherInput.initialResourceToReveal)) + || (!this.initialResourceToReveal && !otherInput.initialResourceToReveal); + return (this.multiDiffSource.toString() === otherInput.multiDiffSource.toString()) && initialResourcesEqual; } return false; @@ -355,6 +371,10 @@ interface ISerializedMultiDiffEditorInput { originalUri: string | undefined; modifiedUri: string | undefined; }[] | undefined; + revealResource: { + originalUri: string | undefined; + modifiedUri: string | undefined; + } | undefined; } export class MultiDiffEditorSerializer implements IEditorSerializer { diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts index e78363dc2296..bab0d9e9613b 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts @@ -51,6 +51,14 @@ export class MultiDiffEditorItem { throw new BugIndicatingError('Invalid arguments'); } } + + equals(other: MultiDiffEditorItem): boolean { + if (this.original?.toString() === other.original?.toString() + && this.modified?.toString() === other.modified?.toString()) { + return true; + } + return false; + } } export class MultiDiffSourceResolverService implements IMultiDiffSourceResolverService { From 500401c8ac5afd54326a5addcdf092db3eedc311 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Wed, 14 Feb 2024 14:53:50 +0100 Subject: [PATCH 13/20] polishing the code --- .../multiDiffEditorWidget.ts | 4 +- .../multiDiffEditorWidgetImpl.ts | 2 +- src/vs/workbench/common/editor.ts | 2 +- .../bulkEdit/browser/preview/bulkEditPane.ts | 4 +- .../browser/multiDiffEditor.ts | 6 +-- .../browser/multiDiffEditorInput.ts | 43 ++++++++----------- .../browser/multiDiffSourceResolverService.ts | 8 ---- .../services/editor/common/editorService.ts | 1 + 8 files changed, 30 insertions(+), 40 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index e2392cc8b4a1..8923fc2024e5 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -8,7 +8,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { derived, derivedWithStore, observableValue, recomputeInitiallyAndOnChange } from 'vs/base/common/observable'; import { readHotReloadableExport } from 'vs/editor/browser/widget/diffEditor/utils'; import { IMultiDiffEditorModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/model'; -import { IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, IMultiDiffResource, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { MultiDiffEditorViewModel } from './multiDiffEditorViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import './colors'; @@ -44,7 +44,7 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public reveal(resource: { original: URI } | { modified: URI }, lineNumber: number): void { + public reveal(resource: IMultiDiffResource, lineNumber: number): void { this._widgetImpl.get().reveal(resource, lineNumber); } diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index b8547c7d347b..b9a0287e63ca 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -313,7 +313,7 @@ function isMultiDiffOriginalResourceUri(obj: any): obj is IMultiDiffOriginalReso return 'original' in obj && obj.original instanceof URI; } -type IMultiDiffResource = IMultiDiffOriginalResource | IMultiDiffModifiedResource; +export type IMultiDiffResource = IMultiDiffOriginalResource | IMultiDiffModifiedResource; class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index 322582ad33c7..283a89b9c857 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -507,7 +507,7 @@ export interface IResourceMultiDiffEditorInput extends IBaseUntypedEditorInput { readonly resources?: IResourceDiffEditorInput[]; /** - * Reveal the following resource on open + * Reveal the following resource on editor open */ readonly revealResource?: IResourceDiffEditorInput; } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index ff0cc489d280..21b1e44e809f 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -11,6 +11,7 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IThemeService } from 'vs/platform/theme/common/themeService'; import { localize } from 'vs/nls'; import { DisposableStore } from 'vs/base/common/lifecycle'; +import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; import { BulkEditPreviewProvider, BulkFileOperation, BulkFileOperations, BulkFileOperationType } from 'vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPreview'; import { ILabelService } from 'vs/platform/label/common/label'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; @@ -36,7 +37,6 @@ import { ResourceEdit } from 'vs/editor/browser/services/bulkEditService'; import { ButtonBar } from 'vs/base/browser/ui/button/button'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; -import { ACTIVE_GROUP, IEditorService, SIDE_GROUP } from 'vs/workbench/services/editor/common/editorService'; import { IResourceDiffEditorInput } from 'vs/workbench/common/editor'; const enum State { @@ -344,6 +344,8 @@ export class BulkEditPane extends ViewPane { } else { resources = await this._getResources(fileOperations); } + this._fileOperations = fileOperations; + this._resources = resources; const revealResource = resources.find(r => r.original.resource!.toString() === fileElement.edit.uri.toString()); const multiDiffSource = URI.from({ scheme: 'refactor-preview' }); const label = 'Refactor Preview'; diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index d32ed95ec3d0..4ef87792095e 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -24,7 +24,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { URI } from 'vs/base/common/uri'; import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorViewModel'; -import { IMultiDiffEditorViewState } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, IMultiDiffResource } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; @@ -72,7 +72,7 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { + return new MultiDiffEditorItem( + resource.original.resource, + resource.modified.resource, + ); + }; const multiDiffSource = input.multiDiffSource ?? URI.parse(`multi-diff-editor:${new Date().getMilliseconds().toString() + Math.random().toString()}`); return instantiationService.createInstance( MultiDiffEditorInput, multiDiffSource, input.label, - input.resources?.map(resource => { - return new MultiDiffEditorItem( - resource.original.resource, - resource.modified.resource, - ); - }), - input.revealResource ? new MultiDiffEditorItem( - input.revealResource.original.resource, - input.revealResource.modified.resource - ) : undefined, + input.resources?.map(toMultiDiffEditorItem), + input.revealResource ? toMultiDiffEditorItem(input.revealResource) : undefined, ); } public static fromSerialized(data: ISerializedMultiDiffEditorInput, instantiationService: IInstantiationService): MultiDiffEditorInput { + const toMultiDiffEditorItem = (resource: { originalUri: string | undefined; modifiedUri: string | undefined }): MultiDiffEditorItem => { + return new MultiDiffEditorItem( + resource.originalUri ? URI.parse(resource.originalUri) : undefined, + resource.modifiedUri ? URI.parse(resource.modifiedUri) : undefined, + ); + }; return instantiationService.createInstance( MultiDiffEditorInput, URI.parse(data.multiDiffSourceUri), data.label, - data.resources?.map(resource => new MultiDiffEditorItem( - resource.originalUri ? URI.parse(resource.originalUri) : undefined, - resource.modifiedUri ? URI.parse(resource.modifiedUri) : undefined, - )), - data.revealResource ? new MultiDiffEditorItem( - data.revealResource.originalUri ? URI.parse(data.revealResource.originalUri) : undefined, - data.revealResource.modifiedUri ? URI.parse(data.revealResource.modifiedUri) : undefined, - ) : undefined + data.resources?.map(toMultiDiffEditorItem), + data.revealResource ? toMultiDiffEditorItem(data.revealResource) : undefined ); } @@ -236,11 +234,8 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor return true; } - if (otherInput instanceof MultiDiffEditorInput) { - const initialResourcesEqual = (this.initialResourceToReveal && otherInput.initialResourceToReveal - && this.initialResourceToReveal.equals(otherInput.initialResourceToReveal)) - || (!this.initialResourceToReveal && !otherInput.initialResourceToReveal); - return (this.multiDiffSource.toString() === otherInput.multiDiffSource.toString()) && initialResourcesEqual; + if (otherInput instanceof MultiDiffEditorInput && !this.initialResourceToReveal && !otherInput.initialResourceToReveal) { + return this.multiDiffSource.toString() === otherInput.multiDiffSource.toString(); } return false; diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts index bab0d9e9613b..e78363dc2296 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffSourceResolverService.ts @@ -51,14 +51,6 @@ export class MultiDiffEditorItem { throw new BugIndicatingError('Invalid arguments'); } } - - equals(other: MultiDiffEditorItem): boolean { - if (this.original?.toString() === other.original?.toString() - && this.modified?.toString() === other.modified?.toString()) { - return true; - } - return false; - } } export class MultiDiffSourceResolverService implements IMultiDiffSourceResolverService { diff --git a/src/vs/workbench/services/editor/common/editorService.ts b/src/vs/workbench/services/editor/common/editorService.ts index 19b51a652c7f..1be2060f98c7 100644 --- a/src/vs/workbench/services/editor/common/editorService.ts +++ b/src/vs/workbench/services/editor/common/editorService.ts @@ -236,6 +236,7 @@ export interface IEditorService { * Open an editor in an editor group. * * @param editor the editor to open + * @param options the options to use for the editor * @param group the target group. If unspecified, the editor will open in the currently * active group. Use `SIDE_GROUP` to open the editor in a new editor group to the side * of the currently active group. From 4d349caa60bb386e0bd7b1ad26c41c009baa62cd Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Wed, 14 Feb 2024 16:53:15 +0100 Subject: [PATCH 14/20] changes from review --- .../multiDiffEditorWidgetImpl.ts | 52 +++++++++++++------ .../browser/bracketMatching.ts | 2 +- src/vs/workbench/common/editor.ts | 5 -- .../bulkEdit/browser/preview/bulkEditPane.ts | 27 ++++++---- .../browser/multiDiffEditor.ts | 23 ++++---- .../browser/multiDiffEditorInput.ts | 39 +++++--------- 6 files changed, 77 insertions(+), 71 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index b9a0287e63ca..31cf913dfeb5 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -191,17 +191,47 @@ export class MultiDiffEditorWidgetImpl extends Disposable { public reveal(resource: IMultiDiffResource, lineNumber: number): void { const viewItems = this._viewItems.get(); let searchCallback: (item: VirtualizedViewItem) => boolean; - if (isMultiDiffOriginalResourceUri(resource)) { + if ('original' in resource) { searchCallback = (item) => item.viewModel.originalUri?.toString() === resource.original.toString(); } else { searchCallback = (item) => item.viewModel.modifiedUri?.toString() === resource.modified.toString(); } const index = viewItems.findIndex(searchCallback); - const scrollTopWithinItem = (lineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); + /* + // How to find the size of the current editor window? + const scrollTopViewport = this._scrollableElement.getScrollPosition().scrollTop; + const scrollBottomViewport = scrollTopViewport + this._sizeObserver.height.get(); + + const scrollTopWithinViewport = (scrollTop: number) => { + return scrollTop >= scrollTopViewport && scrollTop <= scrollBottomViewport; + } + + console.log('scrollTopViewport', scrollTopViewport); + console.log('scrollBottomViewport', scrollBottomViewport); + + let scrollTopOfResource = 0; + for (let i = 0; i < index; i++) { + scrollTopOfResource += viewItems[i].contentHeight.get() + this._spaceBetweenPx; + } + const lineHeight = this._configurationService.getValue('editor.lineHeight'); + const scrollTopOfRange = scrollTopOfResource + (range.startLineNumber - 1) * lineHeight; + const scrollBottomOfRange = scrollTopOfResource + (range.endLineNumber) * lineHeight; + + console.log('scrollTopOfRange', scrollTopOfRange); + console.log('scrollBottomOfRange', scrollBottomOfRange); + + if (scrollTopWithinViewport(scrollTopOfRange) && scrollTopWithinViewport(scrollBottomOfRange)) { + // Early return because the range is already visible + return; + } + + // The range is not visible, hence jump to the top of the top of the range + this._scrollableElement.setScrollPosition({ scrollTop: scrollTopOfRange }); + */ + // todo@aiday-mar: need to find the actual scroll top given the line number specific to the original or modified uri // following does not neccessarily correspond to the appropriate scroll top within the editor - const maxScroll = viewItems[index].template.get()?.maxScroll.get().maxScroll; - let scrollTop = (maxScroll && scrollTopWithinItem < maxScroll) ? scrollTopWithinItem : 0; + let scrollTop = (lineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); for (let i = 0; i < index; i++) { scrollTop += viewItems[i].contentHeight.get() + this._spaceBetweenPx; } @@ -301,19 +331,7 @@ interface IMultiDiffDocState { selections?: ISelection[]; } -interface IMultiDiffOriginalResource { - original: URI; -} - -interface IMultiDiffModifiedResource { - modified: URI; -} - -function isMultiDiffOriginalResourceUri(obj: any): obj is IMultiDiffOriginalResource { - return 'original' in obj && obj.original instanceof URI; -} - -export type IMultiDiffResource = IMultiDiffOriginalResource | IMultiDiffModifiedResource; +export type IMultiDiffResource = { original: URI } | { modified: URI }; class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); diff --git a/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts b/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts index ffd9e3240dd8..552cb7c06a3b 100644 --- a/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts +++ b/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts @@ -216,7 +216,7 @@ export class BracketMatchingController extends Disposable implements IEditorCont } return new Selection(position.lineNumber, position.column, position.lineNumber, position.column); }); - + this._editor.revealLineInCenterIfOutsideViewport(); this._editor.setSelections(newSelections); this._editor.revealRange(newSelections[0]); } diff --git a/src/vs/workbench/common/editor.ts b/src/vs/workbench/common/editor.ts index 283a89b9c857..bf6785d96751 100644 --- a/src/vs/workbench/common/editor.ts +++ b/src/vs/workbench/common/editor.ts @@ -505,11 +505,6 @@ export interface IResourceMultiDiffEditorInput extends IBaseUntypedEditorInput { * If not set, the resources are dynamically derived from the {@link multiDiffSource}. */ readonly resources?: IResourceDiffEditorInput[]; - - /** - * Reveal the following resource on editor open - */ - readonly revealResource?: IResourceDiffEditorInput; } export type IResourceMergeEditorInputSide = (IResourceEditorInput | ITextResourceEditorInput) & { detail?: string }; diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 21b1e44e809f..f22ba75b048f 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -324,6 +324,8 @@ export class BulkEditPane extends ViewPane { return; } const options: Mutable = { ...e.editorOptions }; + console.log('options : ', options); + // todo@aiday-mar, we may not need the following options let fileElement: FileElement; if (e.element instanceof TextEditElement) { fileElement = e.element.parent; @@ -338,20 +340,18 @@ export class BulkEditPane extends ViewPane { return; } - let resources: IResourceDiffEditorInput[]; - if (this._fileOperations === fileOperations && this._resources) { - resources = this._resources; - } else { - resources = await this._getResources(fileOperations); - } - this._fileOperations = fileOperations; - this._resources = resources; - const revealResource = resources.find(r => r.original.resource!.toString() === fileElement.edit.uri.toString()); + const resources = await this._resolveResources(fileOperations); + options.viewState = { + revealData: { + resource: { original: fileElement.edit.uri }, + lineNumber: 1 + } + }; + console.log('options : ', options); const multiDiffSource = URI.from({ scheme: 'refactor-preview' }); const label = 'Refactor Preview'; this._editorService.openEditor({ multiDiffSource, - revealResource, resources, label, description: label, @@ -359,7 +359,10 @@ export class BulkEditPane extends ViewPane { }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); } - private async _getResources(fileOperations: BulkFileOperation[]): Promise { + private async _resolveResources(fileOperations: BulkFileOperation[]): Promise { + if (this._fileOperations === fileOperations && this._resources) { + return this._resources; + } const resources: IResourceDiffEditorInput[] = []; for (const operation of fileOperations) { const operationUri = operation.uri; @@ -386,6 +389,8 @@ export class BulkEditPane extends ViewPane { }); } } + this._fileOperations = fileOperations; + this._resources = resources; return resources; } diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 4ef87792095e..e047d5595b96 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -85,16 +85,19 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { - return new MultiDiffEditorItem( - resource.original.resource, - resource.modified.resource, - ); - }; const multiDiffSource = input.multiDiffSource ?? URI.parse(`multi-diff-editor:${new Date().getMilliseconds().toString() + Math.random().toString()}`); return instantiationService.createInstance( MultiDiffEditorInput, multiDiffSource, input.label, - input.resources?.map(toMultiDiffEditorItem), - input.revealResource ? toMultiDiffEditorItem(input.revealResource) : undefined, + input.resources?.map(resource => { + return new MultiDiffEditorItem( + resource.original.resource, + resource.modified.resource, + ); + }), ); } public static fromSerialized(data: ISerializedMultiDiffEditorInput, instantiationService: IInstantiationService): MultiDiffEditorInput { - const toMultiDiffEditorItem = (resource: { originalUri: string | undefined; modifiedUri: string | undefined }): MultiDiffEditorItem => { - return new MultiDiffEditorItem( - resource.originalUri ? URI.parse(resource.originalUri) : undefined, - resource.modifiedUri ? URI.parse(resource.modifiedUri) : undefined, - ); - }; return instantiationService.createInstance( MultiDiffEditorInput, URI.parse(data.multiDiffSourceUri), data.label, - data.resources?.map(toMultiDiffEditorItem), - data.revealResource ? toMultiDiffEditorItem(data.revealResource) : undefined + data.resources?.map(resource => new MultiDiffEditorItem( + resource.originalUri ? URI.parse(resource.originalUri) : undefined, + resource.modifiedUri ? URI.parse(resource.modifiedUri) : undefined, + )) ); } @@ -87,7 +81,6 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor public readonly multiDiffSource: URI, public readonly label: string | undefined, public readonly initialResources: readonly MultiDiffEditorItem[] | undefined, - public readonly initialResourceToReveal: MultiDiffEditorItem | undefined, @ITextModelService private readonly _textModelService: ITextModelService, @ITextResourceConfigurationService private readonly _textResourceConfigurationService: ITextResourceConfigurationService, @IInstantiationService private readonly _instantiationService: IInstantiationService, @@ -113,10 +106,6 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor originalUri: resource.original?.toString(), modifiedUri: resource.modified?.toString(), })), - revealResource: this.initialResourceToReveal ? { - originalUri: this.initialResourceToReveal.original?.toString(), - modifiedUri: this.initialResourceToReveal.modified?.toString(), - } : undefined }; } @@ -234,7 +223,7 @@ export class MultiDiffEditorInput extends EditorInput implements ILanguageSuppor return true; } - if (otherInput instanceof MultiDiffEditorInput && !this.initialResourceToReveal && !otherInput.initialResourceToReveal) { + if (otherInput instanceof MultiDiffEditorInput) { return this.multiDiffSource.toString() === otherInput.multiDiffSource.toString(); } @@ -366,10 +355,6 @@ interface ISerializedMultiDiffEditorInput { originalUri: string | undefined; modifiedUri: string | undefined; }[] | undefined; - revealResource: { - originalUri: string | undefined; - modifiedUri: string | undefined; - } | undefined; } export class MultiDiffEditorSerializer implements IEditorSerializer { From abaae7f726d29299d4125a739c0c744e015e823c Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Wed, 14 Feb 2024 17:23:13 +0100 Subject: [PATCH 15/20] adding code --- .../multiDiffEditorWidgetImpl.ts | 9 +++++++- .../browser/bracketMatching.ts | 2 +- .../bulkEdit/browser/preview/bulkEditPane.ts | 21 +++++++------------ .../browser/multiDiffEditor.ts | 16 +++++++------- 4 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 31cf913dfeb5..6bb3d5f91851 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -224,7 +224,6 @@ export class MultiDiffEditorWidgetImpl extends Disposable { // Early return because the range is already visible return; } - // The range is not visible, hence jump to the top of the top of the range this._scrollableElement.setScrollPosition({ scrollTop: scrollTopOfRange }); */ @@ -333,6 +332,14 @@ interface IMultiDiffDocState { export type IMultiDiffResource = { original: URI } | { modified: URI }; +export function isIMultiDiffResource(obj: any): obj is IMultiDiffResource { + if (('original' in obj && obj.original instanceof URI) + || ('modified' in obj && obj.modified instanceof URI)) { + return true; + } + return false; +} + class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); diff --git a/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts b/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts index 552cb7c06a3b..ffd9e3240dd8 100644 --- a/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts +++ b/src/vs/editor/contrib/bracketMatching/browser/bracketMatching.ts @@ -216,7 +216,7 @@ export class BracketMatchingController extends Disposable implements IEditorCont } return new Selection(position.lineNumber, position.column, position.lineNumber, position.column); }); - this._editor.revealLineInCenterIfOutsideViewport(); + this._editor.setSelections(newSelections); this._editor.revealRange(newSelections[0]); } diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index f22ba75b048f..db1d7174f531 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -323,39 +323,34 @@ export class BulkEditPane extends ViewPane { if (!fileOperations) { return; } - const options: Mutable = { ...e.editorOptions }; - console.log('options : ', options); - // todo@aiday-mar, we may not need the following options let fileElement: FileElement; if (e.element instanceof TextEditElement) { fileElement = e.element.parent; - options.selection = e.element.edit.textEdit.textEdit.range; - } else if (e.element instanceof FileElement) { fileElement = e.element; - options.selection = e.element.edit.textEdits[0]?.textEdit.textEdit.range; - } else { // invalid event return; } const resources = await this._resolveResources(fileOperations); - options.viewState = { - revealData: { - resource: { original: fileElement.edit.uri }, - lineNumber: 1 + const options: Mutable = { + ...e.editorOptions, + viewState: { + revealData: { + resource: { original: fileElement.edit.uri }, + lineNumber: 1 + } } }; - console.log('options : ', options); const multiDiffSource = URI.from({ scheme: 'refactor-preview' }); const label = 'Refactor Preview'; this._editorService.openEditor({ multiDiffSource, resources, label, - description: label, options, + description: label }, e.sideBySide ? SIDE_GROUP : ACTIVE_GROUP); } diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index e047d5595b96..1270bc7e977d 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -24,7 +24,7 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { URI } from 'vs/base/common/uri'; import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorViewModel'; -import { IMultiDiffEditorViewState, IMultiDiffResource } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, IMultiDiffResource, isIMultiDiffResource } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; @@ -93,12 +93,14 @@ export class MultiDiffEditor extends AbstractEditorWithViewState Date: Wed, 14 Feb 2024 17:37:50 +0100 Subject: [PATCH 16/20] polishing the code --- .../multiDiffEditorWidget.ts | 5 ++- .../multiDiffEditorWidgetImpl.ts | 39 ++----------------- .../browser/multiDiffEditor.ts | 5 ++- 3 files changed, 10 insertions(+), 39 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index 8923fc2024e5..a97070846346 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -18,6 +18,7 @@ import { Event } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; +import { Range } from 'vs/editor/common/core/range'; export class MultiDiffEditorWidget extends Disposable { private readonly _dimension = observableValue(this, undefined); @@ -44,8 +45,8 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public reveal(resource: IMultiDiffResource, lineNumber: number): void { - this._widgetImpl.get().reveal(resource, lineNumber); + public reveal(resource: IMultiDiffResource, range: Range): void { + this._widgetImpl.get().reveal(resource, range); } public createViewModel(model: IMultiDiffEditorModel): MultiDiffEditorViewModel { diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 6bb3d5f91851..381e48b089fe 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -26,6 +26,7 @@ import { URI } from 'vs/base/common/uri'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; +import { Range } from 'vs/editor/common/core/range'; export class MultiDiffEditorWidgetImpl extends Disposable { private readonly _elements = h('div.monaco-component.multiDiffEditor', [ @@ -188,7 +189,8 @@ export class MultiDiffEditorWidgetImpl extends Disposable { this._scrollableElement.setScrollPosition({ scrollLeft: scrollState.left, scrollTop: scrollState.top }); } - public reveal(resource: IMultiDiffResource, lineNumber: number): void { + // todo@aiday-mar need to reveal the range instead of just the start line number + public reveal(resource: IMultiDiffResource, range: Range): void { const viewItems = this._viewItems.get(); let searchCallback: (item: VirtualizedViewItem) => boolean; if ('original' in resource) { @@ -197,40 +199,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { searchCallback = (item) => item.viewModel.modifiedUri?.toString() === resource.modified.toString(); } const index = viewItems.findIndex(searchCallback); - /* - // How to find the size of the current editor window? - const scrollTopViewport = this._scrollableElement.getScrollPosition().scrollTop; - const scrollBottomViewport = scrollTopViewport + this._sizeObserver.height.get(); - - const scrollTopWithinViewport = (scrollTop: number) => { - return scrollTop >= scrollTopViewport && scrollTop <= scrollBottomViewport; - } - - console.log('scrollTopViewport', scrollTopViewport); - console.log('scrollBottomViewport', scrollBottomViewport); - - let scrollTopOfResource = 0; - for (let i = 0; i < index; i++) { - scrollTopOfResource += viewItems[i].contentHeight.get() + this._spaceBetweenPx; - } - const lineHeight = this._configurationService.getValue('editor.lineHeight'); - const scrollTopOfRange = scrollTopOfResource + (range.startLineNumber - 1) * lineHeight; - const scrollBottomOfRange = scrollTopOfResource + (range.endLineNumber) * lineHeight; - - console.log('scrollTopOfRange', scrollTopOfRange); - console.log('scrollBottomOfRange', scrollBottomOfRange); - - if (scrollTopWithinViewport(scrollTopOfRange) && scrollTopWithinViewport(scrollBottomOfRange)) { - // Early return because the range is already visible - return; - } - // The range is not visible, hence jump to the top of the top of the range - this._scrollableElement.setScrollPosition({ scrollTop: scrollTopOfRange }); - */ - - // todo@aiday-mar: need to find the actual scroll top given the line number specific to the original or modified uri - // following does not neccessarily correspond to the appropriate scroll top within the editor - let scrollTop = (lineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); + let scrollTop = (range.startLineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); for (let i = 0; i < index; i++) { scrollTop += viewItems[i].contentHeight.get() + this._spaceBetweenPx; } diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 1270bc7e977d..151f887f0e97 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -27,6 +27,7 @@ import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEdit import { IMultiDiffEditorViewState, IMultiDiffResource, isIMultiDiffResource } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; +import { Range } from 'vs/editor/common/core/range'; export class MultiDiffEditor extends AbstractEditorWithViewState { static readonly ID = 'multiDiffEditor'; @@ -72,8 +73,8 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { From d88ae06cede970c1e3ea216fd73346292a359f63 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Wed, 14 Feb 2024 17:50:33 +0100 Subject: [PATCH 17/20] IRange used --- .../widget/multiDiffEditorWidget/multiDiffEditorWidget.ts | 4 ++-- .../multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts | 4 ++-- .../contrib/bulkEdit/browser/preview/bulkEditPane.ts | 3 ++- .../contrib/multiDiffEditor/browser/multiDiffEditor.ts | 8 ++++---- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index a97070846346..4fed15c8da2a 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -18,7 +18,7 @@ import { Event } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; -import { Range } from 'vs/editor/common/core/range'; +import { IRange } from 'vs/editor/common/core/range'; export class MultiDiffEditorWidget extends Disposable { private readonly _dimension = observableValue(this, undefined); @@ -45,7 +45,7 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public reveal(resource: IMultiDiffResource, range: Range): void { + public reveal(resource: IMultiDiffResource, range: IRange): void { this._widgetImpl.get().reveal(resource, range); } diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 381e48b089fe..3aedbd4cc57d 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -26,7 +26,7 @@ import { URI } from 'vs/base/common/uri'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { Range } from 'vs/editor/common/core/range'; +import { IRange } from 'vs/editor/common/core/range'; export class MultiDiffEditorWidgetImpl extends Disposable { private readonly _elements = h('div.monaco-component.multiDiffEditor', [ @@ -190,7 +190,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { } // todo@aiday-mar need to reveal the range instead of just the start line number - public reveal(resource: IMultiDiffResource, range: Range): void { + public reveal(resource: IMultiDiffResource, range: IRange): void { const viewItems = this._viewItems.get(); let searchCallback: (item: VirtualizedViewItem) => boolean; if ('original' in resource) { diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index db1d7174f531..913f8bdf1366 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -38,6 +38,7 @@ import { ButtonBar } from 'vs/base/browser/ui/button/button'; import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; import { IResourceDiffEditorInput } from 'vs/workbench/common/editor'; +import { Range } from 'vs/editor/common/core/range'; const enum State { Data = 'data', @@ -339,7 +340,7 @@ export class BulkEditPane extends ViewPane { viewState: { revealData: { resource: { original: fileElement.edit.uri }, - lineNumber: 1 + range: new Range(1, 1, 1, 1) } } }; diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 151f887f0e97..a01863e45edb 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -27,7 +27,7 @@ import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEdit import { IMultiDiffEditorViewState, IMultiDiffResource, isIMultiDiffResource } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; -import { Range } from 'vs/editor/common/core/range'; +import { IRange, Range } from 'vs/editor/common/core/range'; export class MultiDiffEditor extends AbstractEditorWithViewState { static readonly ID = 'multiDiffEditor'; @@ -73,7 +73,7 @@ export class MultiDiffEditor extends AbstractEditorWithViewState Date: Fri, 16 Feb 2024 11:03:19 +0100 Subject: [PATCH 18/20] adding review changes --- .../multiDiffEditorWidget.ts | 7 ++--- .../multiDiffEditorWidgetImpl.ts | 29 ++++++++++++------- .../bulkEdit/browser/preview/bulkEditPane.ts | 4 +-- .../browser/multiDiffEditor.ts | 22 ++++---------- 4 files changed, 29 insertions(+), 33 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index a1405574d341..dcc315e5b7d4 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -8,7 +8,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { derived, derivedWithStore, observableValue, recomputeInitiallyAndOnChange } from 'vs/base/common/observable'; import { readHotReloadableExport } from 'vs/editor/browser/widget/diffEditor/utils'; import { IMultiDiffEditorModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/model'; -import { IMultiDiffEditorViewState, IMultiDiffResource, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorOptionRevealData, IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { MultiDiffEditorViewModel } from './multiDiffEditorViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import './colors'; @@ -18,7 +18,6 @@ import { Event } from 'vs/base/common/event'; import { URI } from 'vs/base/common/uri'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; -import { IRange } from 'vs/editor/common/core/range'; import { DiffEditorWidget } from 'vs/editor/browser/widget/diffEditor/diffEditorWidget'; export class MultiDiffEditorWidget extends Disposable { @@ -46,8 +45,8 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public reveal(resource: IMultiDiffResource, range: IRange): void { - this._widgetImpl.get().reveal(resource, range); + public reveal(revealData: IMultiDiffEditorOptionRevealData): void { + this._widgetImpl.get().reveal(revealData); } public createViewModel(model: IMultiDiffEditorModel): MultiDiffEditorViewModel { diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 3aedbd4cc57d..26ad0f1b1642 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -26,7 +26,8 @@ import { URI } from 'vs/base/common/uri'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; -import { IRange } from 'vs/editor/common/core/range'; +import { Range } from 'vs/editor/common/core/range'; +import { ITextEditorOptions } from 'vs/platform/editor/common/editor'; export class MultiDiffEditorWidgetImpl extends Disposable { private readonly _elements = h('div.monaco-component.multiDiffEditor', [ @@ -190,7 +191,8 @@ export class MultiDiffEditorWidgetImpl extends Disposable { } // todo@aiday-mar need to reveal the range instead of just the start line number - public reveal(resource: IMultiDiffResource, range: IRange): void { + public reveal(revealData: IMultiDiffEditorOptionRevealData): void { + const resource = revealData.resource; const viewItems = this._viewItems.get(); let searchCallback: (item: VirtualizedViewItem) => boolean; if ('original' in resource) { @@ -199,7 +201,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { searchCallback = (item) => item.viewModel.modifiedUri?.toString() === resource.modified.toString(); } const index = viewItems.findIndex(searchCallback); - let scrollTop = (range.startLineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); + let scrollTop = (revealData.range.startLineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); for (let i = 0; i < index; i++) { scrollTop += viewItems[i].contentHeight.get() + this._spaceBetweenPx; } @@ -299,16 +301,21 @@ interface IMultiDiffDocState { selections?: ISelection[]; } -export type IMultiDiffResource = { original: URI } | { modified: URI }; - -export function isIMultiDiffResource(obj: any): obj is IMultiDiffResource { - if (('original' in obj && obj.original instanceof URI) - || ('modified' in obj && obj.modified instanceof URI)) { - return true; - } - return false; +export interface IMultiDiffEditorOptions extends ITextEditorOptions { + viewState?: IMultiDiffEditorOptionsViewState; } +export interface IMultiDiffEditorOptionsViewState { + revealData?: IMultiDiffEditorOptionRevealData; +} + +export interface IMultiDiffEditorOptionRevealData { + resource: IMultiDiffResource; + range: Range; +} + +export type IMultiDiffResource = { original: URI } | { modified: URI }; + class VirtualizedViewItem extends Disposable { private readonly _templateRef = this._register(disposableObservableValue | undefined>(this, undefined)); diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index 913f8bdf1366..8739a79bbbb8 100644 --- a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts +++ b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts @@ -27,7 +27,6 @@ import { IDialogService } from 'vs/platform/dialogs/common/dialogs'; import { MenuId } from 'vs/platform/actions/common/actions'; import { ITreeContextMenuEvent } from 'vs/base/browser/ui/tree/tree'; import { CancellationToken } from 'vs/base/common/cancellation'; -import { ITextEditorOptions } from 'vs/platform/editor/common/editor'; import type { IAsyncDataTreeViewState } from 'vs/base/browser/ui/tree/asyncDataTree'; import { IStorageService, StorageScope, StorageTarget } from 'vs/platform/storage/common/storage'; import { IViewDescriptorService } from 'vs/workbench/common/views'; @@ -39,6 +38,7 @@ import { defaultButtonStyles } from 'vs/platform/theme/browser/defaultStyles'; import { Mutable } from 'vs/base/common/types'; import { IResourceDiffEditorInput } from 'vs/workbench/common/editor'; import { Range } from 'vs/editor/common/core/range'; +import { IMultiDiffEditorOptions } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; const enum State { Data = 'data', @@ -335,7 +335,7 @@ export class BulkEditPane extends ViewPane { } const resources = await this._resolveResources(fileOperations); - const options: Mutable = { + const options: Mutable = { ...e.editorOptions, viewState: { revealData: { diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index 7a7aa309a9ea..ed582ca29b63 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -8,7 +8,6 @@ import { CancellationToken } from 'vs/base/common/cancellation'; import { MultiDiffEditorWidget } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget'; import { IResourceLabel, IWorkbenchUIElementFactory } from 'vs/editor/browser/widget/multiDiffEditorWidget/workbenchUIElementFactory'; import { ITextResourceConfigurationService } from 'vs/editor/common/services/textResourceConfiguration'; -import { IEditorOptions } from 'vs/platform/editor/common/editor'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { InstantiationService } from 'vs/platform/instantiation/common/instantiationService'; import { IStorageService } from 'vs/platform/storage/common/storage'; @@ -24,10 +23,9 @@ import { IEditorGroupsService } from 'vs/workbench/services/editor/common/editor import { IEditorService } from 'vs/workbench/services/editor/common/editorService'; import { URI } from 'vs/base/common/uri'; import { MultiDiffEditorViewModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorViewModel'; -import { IMultiDiffEditorViewState, IMultiDiffResource, isIMultiDiffResource } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorOptions, IMultiDiffEditorViewState } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; -import { IRange, Range } from 'vs/editor/common/core/range'; export class MultiDiffEditor extends AbstractEditorWithViewState { static readonly ID = 'multiDiffEditor'; @@ -73,11 +71,7 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { + override async setInput(input: MultiDiffEditorInput, options: IMultiDiffEditorOptions | undefined, context: IEditorOpenContext, token: CancellationToken): Promise { await super.setInput(input, options, context, token); this._viewModel = await input.getViewModel(); this._multiDiffEditorWidget!.setViewModel(this._viewModel); @@ -89,20 +83,16 @@ export class MultiDiffEditor extends AbstractEditorWithViewState { From ac465646a9e89eea1bf23941299694716555a2b1 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Fri, 16 Feb 2024 11:12:15 +0100 Subject: [PATCH 19/20] removing the interface for revealData --- .../multiDiffEditorWidget/multiDiffEditorWidget.ts | 5 +++-- .../multiDiffEditorWidgetImpl.ts | 12 +++++------- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index dcc315e5b7d4..60adf436d150 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -8,7 +8,7 @@ import { Disposable } from 'vs/base/common/lifecycle'; import { derived, derivedWithStore, observableValue, recomputeInitiallyAndOnChange } from 'vs/base/common/observable'; import { readHotReloadableExport } from 'vs/editor/browser/widget/diffEditor/utils'; import { IMultiDiffEditorModel } from 'vs/editor/browser/widget/multiDiffEditorWidget/model'; -import { IMultiDiffEditorOptionRevealData, IMultiDiffEditorViewState, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; +import { IMultiDiffEditorViewState, IMultiDiffResource, MultiDiffEditorWidgetImpl } from 'vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl'; import { MultiDiffEditorViewModel } from './multiDiffEditorViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import './colors'; @@ -19,6 +19,7 @@ import { URI } from 'vs/base/common/uri'; import { IDiffEditor } from 'vs/editor/common/editorCommon'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { DiffEditorWidget } from 'vs/editor/browser/widget/diffEditor/diffEditorWidget'; +import { Range } from 'vs/editor/common/core/range'; export class MultiDiffEditorWidget extends Disposable { private readonly _dimension = observableValue(this, undefined); @@ -45,7 +46,7 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public reveal(revealData: IMultiDiffEditorOptionRevealData): void { + public reveal(revealData: { resource: IMultiDiffResource; range: Range }): void { this._widgetImpl.get().reveal(revealData); } diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index 26ad0f1b1642..e423974d4fb7 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -191,7 +191,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { } // todo@aiday-mar need to reveal the range instead of just the start line number - public reveal(revealData: IMultiDiffEditorOptionRevealData): void { + public reveal(revealData: { resource: IMultiDiffResource; range: Range }): void { const resource = revealData.resource; const viewItems = this._viewItems.get(); let searchCallback: (item: VirtualizedViewItem) => boolean; @@ -306,12 +306,10 @@ export interface IMultiDiffEditorOptions extends ITextEditorOptions { } export interface IMultiDiffEditorOptionsViewState { - revealData?: IMultiDiffEditorOptionRevealData; -} - -export interface IMultiDiffEditorOptionRevealData { - resource: IMultiDiffResource; - range: Range; + revealData?: { + resource: IMultiDiffResource; + range: Range; + }; } export type IMultiDiffResource = { original: URI } | { modified: URI }; From 0fd2b0ce5d941c8c24713076ec07c6f794a1d860 Mon Sep 17 00:00:00 2001 From: Aiday Marlen Kyzy Date: Fri, 16 Feb 2024 11:34:48 +0100 Subject: [PATCH 20/20] inlining the resource and the range --- .../widget/multiDiffEditorWidget/multiDiffEditorWidget.ts | 4 ++-- .../multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts | 5 ++--- .../contrib/multiDiffEditor/browser/multiDiffEditor.ts | 2 +- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index 60adf436d150..5a869f83bd2f 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts @@ -46,8 +46,8 @@ export class MultiDiffEditorWidget extends Disposable { this._register(recomputeInitiallyAndOnChange(this._widgetImpl)); } - public reveal(revealData: { resource: IMultiDiffResource; range: Range }): void { - this._widgetImpl.get().reveal(revealData); + public reveal(resource: IMultiDiffResource, range: Range): void { + this._widgetImpl.get().reveal(resource, range); } public createViewModel(model: IMultiDiffEditorModel): MultiDiffEditorViewModel { diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts index e423974d4fb7..ca4e240c1dac 100644 --- a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts +++ b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidgetImpl.ts @@ -191,8 +191,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { } // todo@aiday-mar need to reveal the range instead of just the start line number - public reveal(revealData: { resource: IMultiDiffResource; range: Range }): void { - const resource = revealData.resource; + public reveal(resource: IMultiDiffResource, range: Range): void { const viewItems = this._viewItems.get(); let searchCallback: (item: VirtualizedViewItem) => boolean; if ('original' in resource) { @@ -201,7 +200,7 @@ export class MultiDiffEditorWidgetImpl extends Disposable { searchCallback = (item) => item.viewModel.modifiedUri?.toString() === resource.modified.toString(); } const index = viewItems.findIndex(searchCallback); - let scrollTop = (revealData.range.startLineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); + let scrollTop = (range.startLineNumber - 1) * this._configurationService.getValue('editor.lineHeight'); for (let i = 0; i < index; i++) { scrollTop += viewItems[i].contentHeight.get() + this._spaceBetweenPx; } diff --git a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts index ed582ca29b63..bb8bdc7c16e5 100644 --- a/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts +++ b/src/vs/workbench/contrib/multiDiffEditor/browser/multiDiffEditor.ts @@ -92,7 +92,7 @@ export class MultiDiffEditor extends AbstractEditorWithViewState {