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 3f46f698ce22f..9c6c2e1991b83 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 059a4928862c6..10a75a4490422 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 ee2be0b1bcc24..16c8268848bd8 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 4a18e62f25636..4cfaa3c07eead 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 d9e8a1112a0d2..c649d96c78cff 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 | 134 ++++++++++++------ 2 files changed, 90 insertions(+), 45 deletions(-) diff --git a/src/vs/platform/list/browser/listService.ts b/src/vs/platform/list/browser/listService.ts index 6857531cb6681..317af0fdf3a43 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 4cfaa3c07eead..437721a69fc18 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 - }); - - // 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 }); + // 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)); + 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 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 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 }, @@ -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 437721a69fc18..229662787263a 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 | 143 ++++++++---------- .../browser/preview/bulkEditPreview.ts | 1 - 6 files changed, 65 insertions(+), 83 deletions(-) diff --git a/extensions/git/src/commands.ts b/extensions/git/src/commands.ts index 9c6c2e1991b83..3f46f698ce22f 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 10a75a4490422..059a4928862c6 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 317af0fdf3a43..6857531cb6681 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 16c8268848bd8..ee2be0b1bcc24 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 229662787263a..5716aa6765947 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); + 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; + + 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('fileElement.edit : ', fileElement.edit); + console.log('leftResource : ', JSON.stringify(leftResource)); 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 }); + 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 { - 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; - } - 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 }); + 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 c649d96c78cff..d9e8a1112a0d2 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 5716aa6765947..9f7077d24ac2c 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 938edfad4b33d..349c26b5ee389 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 129d91151b1ad..54818d55c3597 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 9f7077d24ac2c..7a8faba7ea4b4 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 263668667598c..9361ee7e0efbf 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 349c26b5ee389..9a19e5e5669ea 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 54818d55c3597..5619b08b2cd5f 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 7a8faba7ea4b4..b072acb88e073 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 9361ee7e0efbf..21b1d64a6fac1 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 | 102 ++++++++---------- 1 file changed, 44 insertions(+), 58 deletions(-) diff --git a/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts b/src/vs/workbench/contrib/bulkEdit/browser/preview/bulkEditPane.ts index b072acb88e073..49cef9c8c8ae9 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) { - - 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; + 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) } + }); + + } 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 49cef9c8c8ae9..3a9b9ebab13ae 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 3a9b9ebab13ae..2d8031cbe778d 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 9a19e5e5669ea..e2392cc8b4a1e 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 5619b08b2cd5f..b8547c7d347b7 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 bf6785d967516..322582ad33c76 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 2d8031cbe778d..ff0cc489d2808 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 21b1d64a6fac1..be2fab9317566 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 1be2060f98c72..19b51a652c7fa 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 be2fab9317566..d32ed95ec3d06 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 907fd6e15fe81..f7dae8fc3e019 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 e78363dc2296d..bab0d9e9613b6 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 e2392cc8b4a1e..8923fc2024e50 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 b8547c7d347b7..b9a0287e63caf 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 322582ad33c76..283a89b9c8576 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 ff0cc489d2808..21b1e44e809f6 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 d32ed95ec3d06..4ef87792095e4 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 bab0d9e9613b6..e78363dc2296d 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 19b51a652c7fa..1be2060f98c72 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 b9a0287e63caf..31cf913dfeb54 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 ffd9e3240dd8f..552cb7c06a3be 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 283a89b9c8576..bf6785d967516 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 21b1e44e809f6..f22ba75b048fe 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 4ef87792095e4..e047d5595b966 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 31cf913dfeb54..6bb3d5f918512 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 552cb7c06a3be..ffd9e3240dd8f 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 f22ba75b048fe..db1d7174f531f 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 e047d5595b966..1270bc7e977df 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 8923fc2024e50..a970708463467 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 6bb3d5f918512..381e48b089fe8 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 1270bc7e977df..151f887f0e97c 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 a970708463467..4fed15c8da2a3 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 381e48b089fe8..3aedbd4cc57d8 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 db1d7174f531f..913f8bdf1366c 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 151f887f0e97c..a01863e45edb1 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 | 27 ++++++++++++------- .../bulkEdit/browser/preview/bulkEditPane.ts | 4 +-- .../browser/multiDiffEditor.ts | 22 +++++---------- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts b/src/vs/editor/browser/widget/multiDiffEditorWidget/multiDiffEditorWidget.ts index a1405574d341e..dcc315e5b7d4f 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 3aedbd4cc57d8..26ad0f1b16426 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 interface IMultiDiffEditorOptions extends ITextEditorOptions { + viewState?: IMultiDiffEditorOptionsViewState; +} -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 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 913f8bdf1366c..8739a79bbbb85 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 7a7aa309a9eaa..ed582ca29b635 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 dcc315e5b7d4f..60adf436d1502 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 26ad0f1b16426..e423974d4fb70 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 60adf436d1502..5a869f83bd2f7 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 e423974d4fb70..ca4e240c1dace 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 ed582ca29b635..bb8bdc7c16e53 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 {