From aaf202a046ac5cea26393fccfefb573eaae06bd8 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 7 Aug 2024 15:26:40 +1000 Subject: [PATCH 01/11] Hide unchanged cells --- .../notebook/browser/diff/diffComponents.ts | 125 ++++++++++++++-- .../browser/diff/diffElementOutputs.ts | 6 +- .../browser/diff/diffElementViewModel.ts | 84 +++++++++-- .../notebook/browser/diff/notebookDiff.css | 62 +++++++- .../browser/diff/notebookDiffActions.ts | 10 +- .../browser/diff/notebookDiffEditor.ts | 133 ++++++++---------- .../browser/diff/notebookDiffEditorBrowser.ts | 32 +++-- .../notebook/browser/diff/notebookDiffList.ts | 82 ++++++++--- .../browser/diff/notebookDiffOverviewRuler.ts | 12 +- 9 files changed, 415 insertions(+), 131 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts index 506524d445bc9..524bc898d72fe 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts @@ -7,8 +7,8 @@ import * as DOM from 'vs/base/browser/dom'; import { Disposable, DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; import { Schemas } from 'vs/base/common/network'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { DiffElementViewModelBase, getFormattedMetadataJSON, getFormattedOutputJSON, OutputComparison, outputEqual, OUTPUT_EDITOR_HEIGHT_MAGIC, PropertyFoldingState, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; -import { CellDiffSideBySideRenderTemplate, CellDiffSingleSideRenderTemplate, DiffSide, DIFF_CELL_MARGIN, INotebookTextDiffEditor, NOTEBOOK_DIFF_CELL_INPUT, NOTEBOOK_DIFF_CELL_PROPERTY, NOTEBOOK_DIFF_CELL_PROPERTY_EXPANDED } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; +import { DiffElementCellViewModelBase, getFormattedMetadataJSON, getFormattedOutputJSON, OutputComparison, outputEqual, OUTPUT_EDITOR_HEIGHT_MAGIC, PropertyFoldingState, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel, DiffElementPlaceholderViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { CellDiffSideBySideRenderTemplate, CellDiffSingleSideRenderTemplate, DiffSide, DIFF_CELL_MARGIN, INotebookTextDiffEditor, NOTEBOOK_DIFF_CELL_INPUT, NOTEBOOK_DIFF_CELL_PROPERTY, NOTEBOOK_DIFF_CELL_PROPERTY_EXPANDED, CellDiffPlaceholderRenderTemplate, IDiffCellMarginOverlay } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; import { CodeEditorWidget, ICodeEditorWidgetOptions } from 'vs/editor/browser/widget/codeEditor/codeEditorWidget'; import { IModelService } from 'vs/editor/common/services/model'; import { ILanguageService } from 'vs/editor/common/languages/language'; @@ -31,7 +31,7 @@ import { SuggestController } from 'vs/editor/contrib/suggest/browser/suggestCont import { MenuPreventer } from 'vs/workbench/contrib/codeEditor/browser/menuPreventer'; import { SelectionClipboardContributionID } from 'vs/workbench/contrib/codeEditor/browser/selectionClipboard'; import { TabCompletionController } from 'vs/workbench/contrib/snippets/browser/tabCompletion'; -import { renderIcon } from 'vs/base/browser/ui/iconLabel/iconLabels'; +import { renderIcon, renderLabelWithIcons } from 'vs/base/browser/ui/iconLabel/iconLabels'; import * as editorCommon from 'vs/editor/common/editorCommon'; import { ITextModelService } from 'vs/editor/common/services/resolverService'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; @@ -44,6 +44,8 @@ import { IAccessibilityService } from 'vs/platform/accessibility/common/accessib import { DiffEditorWidget } from 'vs/editor/browser/widget/diffEditor/diffEditorWidget'; import { ICommandService } from 'vs/platform/commands/common/commands'; import { DiffNestedCellViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffNestedCellViewModel'; +import { localize } from 'vs/nls'; +import { Emitter } from 'vs/base/common/event'; export function getOptimizedNestedCodeEditorWidgetOptions(): ICodeEditorWidgetOptions { return { @@ -59,6 +61,37 @@ export function getOptimizedNestedCodeEditorWidgetOptions(): ICodeEditorWidgetOp }; } +export class CellDiffPlaceholderElement extends Disposable { + constructor( + placeholder: DiffElementPlaceholderViewModel, + templateData: CellDiffPlaceholderRenderTemplate, + ) { + super(); + templateData.body.classList.remove('left', 'right', 'full'); + const text = (placeholder.hiddenCells.length === 1) ? + localize('hiddenCell', '{0} hidden cell', placeholder.hiddenCells.length) : + localize('hiddenCells', '{0} hidden cells', placeholder.hiddenCells.length); + templateData.placeholder.innerText = text; + + const handler = (e: MouseEvent) => { + if (e.button !== 0) { + return; + } + e.preventDefault(); + placeholder.showHiddenCells(); + }; + templateData.placeholder.addEventListener('dblclick', handler); + this._register({ + dispose: () => { + templateData.placeholder.removeEventListener('dblclick', handler); + } + }); + this._register(templateData.marginOverlay.onAction(() => { + placeholder.showHiddenCells(); + })); + templateData.marginOverlay.show(); + } +} class PropertyHeader extends Disposable { protected _foldingIndicator!: HTMLElement; @@ -69,14 +102,14 @@ class PropertyHeader extends Disposable { protected _propertyExpanded?: IContextKey; constructor( - readonly cell: DiffElementViewModelBase, + readonly cell: DiffElementCellViewModelBase, readonly propertyHeaderContainer: HTMLElement, readonly notebookEditor: INotebookTextDiffEditor, readonly accessor: { updateInfoRendering: (renderOutput: boolean) => void; - checkIfModified: (cell: DiffElementViewModelBase) => false | { reason: string | undefined }; - getFoldingState: (cell: DiffElementViewModelBase) => PropertyFoldingState; - updateFoldingState: (cell: DiffElementViewModelBase, newState: PropertyFoldingState) => void; + checkIfModified: (cell: DiffElementCellViewModelBase) => false | { reason: string | undefined }; + getFoldingState: (cell: DiffElementCellViewModelBase) => PropertyFoldingState; + updateFoldingState: (cell: DiffElementCellViewModelBase, newState: PropertyFoldingState) => void; unChangedLabel: string; changedLabel: string; prefix: string; @@ -271,7 +304,7 @@ abstract class AbstractElementRenderer extends Disposable { constructor( readonly notebookEditor: INotebookTextDiffEditor, - readonly cell: DiffElementViewModelBase, + readonly cell: DiffElementCellViewModelBase, readonly templateData: CellDiffSingleSideRenderTemplate | CellDiffSideBySideRenderTemplate, readonly style: 'left' | 'right' | 'full', protected readonly instantiationService: IInstantiationService, @@ -1363,6 +1396,17 @@ export class ModifiedElement extends AbstractElementRenderer { container.classList.remove('inserted', 'removed'); } + override buildBody(): void { + super.buildBody(); + if (this.cell.unchangedCells) { + this._register(this.templateData.marginOverlay.onAction(() => { + this.cell.hideUnchangedCells(); + })); + this.templateData.marginOverlay.show(); + } else { + this.templateData.marginOverlay.hide(); + } + } _disposeMetadata() { this.cell.metadataStatusHeight = 0; this.cell.metadataHeight = 0; @@ -1789,3 +1833,68 @@ export class ModifiedElement extends AbstractElementRenderer { super.dispose(); } } + + +export class CollapsedCellOverlayWidget extends Disposable implements IDiffCellMarginOverlay { + private readonly _nodes = DOM.h('div.diff-hidden-cells', [ + DOM.h('div.center@content', { style: { display: 'flex' } }, [ + DOM.$('a', { title: localize('showUnchangedCells', 'Show Unchanged Cells'), role: 'button' }, + ...renderLabelWithIcons('$(unfold)'))] + ), + ]); + + private readonly _action = this._register(new Emitter()); + public readonly onAction = this._action.event; + constructor( + container: HTMLElement + ) { + super(); + + this._nodes.root.style.display = 'none'; + container.appendChild(this._nodes.root); + this._register(DOM.addDisposableListener(this._nodes.content.children[0], 'click', () => { + this._action.fire(); + })); + } + public show() { + this._nodes.root.style.display = 'block'; + } + public hide() { + this._nodes.root.style.display = 'none'; + } + public override dispose() { + this.hide(); + super.dispose(); + } +} + +export class UnchangedCellOverlayWidget extends Disposable implements IDiffCellMarginOverlay { + private readonly _nodes = DOM.h('div.diff-hidden-cells', [ + DOM.h('div.center@content', { style: { display: 'flex' } }, [ + DOM.$('a', { title: localize('hideUnchangedCells', 'Hide Unchanged Cells'), role: 'button' }, + ...renderLabelWithIcons('$(fold)'))] + ), + ]); + + private readonly _action = this._register(new Emitter()); + public readonly onAction = this._action.event; + constructor( + container: HTMLElement + ) { + super(); + + this._nodes.root.style.display = 'none'; + container.appendChild(this._nodes.root); + this._register(DOM.addDisposableListener(this._nodes.content.children[0], 'click', () => this._action.fire())); + } + public show() { + this._nodes.root.style.display = 'block'; + } + public hide() { + this._nodes.root.style.display = 'none'; + } + public override dispose() { + this.hide(); + super.dispose(); + } +} diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementOutputs.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementOutputs.ts index ea94a04d2adae..4787cc9bcda70 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementOutputs.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementOutputs.ts @@ -6,7 +6,7 @@ import * as DOM from 'vs/base/browser/dom'; import * as nls from 'vs/nls'; import { Disposable, DisposableStore } from 'vs/base/common/lifecycle'; -import { DiffElementViewModelBase, SideBySideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { DiffElementCellViewModelBase, SideBySideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; import { DiffSide, INotebookTextDiffEditor } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; import { ICellOutputViewModel, IInsetRenderOutput, RenderOutputType } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; @@ -33,7 +33,7 @@ export class OutputElement extends Disposable { private _notebookTextModel: NotebookTextModel, private _notebookService: INotebookService, private _quickInputService: IQuickInputService, - private _diffElementViewModel: DiffElementViewModelBase, + private _diffElementViewModel: DiffElementCellViewModelBase, private _diffSide: DiffSide, private _nestedCell: DiffNestedCellViewModel, private _outputContainer: HTMLElement, @@ -228,7 +228,7 @@ export class OutputContainer extends Disposable { constructor( private _editor: INotebookTextDiffEditor, private _notebookTextModel: NotebookTextModel, - private _diffElementViewModel: DiffElementViewModelBase, + private _diffElementViewModel: DiffElementCellViewModelBase, private _nestedCellViewModel: DiffNestedCellViewModel, private _diffSide: DiffSide, private _outputContainer: HTMLElement, diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts index 52617b470fc1a..bd456b420abe5 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts @@ -33,16 +33,80 @@ interface ILayoutInfoDelta extends ILayoutInfoDelta0 { recomputeOutput?: boolean; } +export type IDiffElementViewModelBase = DiffElementCellViewModelBase | DiffElementPlaceholderViewModel; + export abstract class DiffElementViewModelBase extends Disposable { + protected _layoutInfoEmitter = this._register(new Emitter()); + onDidLayoutChange = this._layoutInfoEmitter.event; + constructor( + public readonly mainDocumentTextModel: INotebookTextModel, + public readonly editorEventDispatcher: NotebookDiffEditorEventDispatcher, + public readonly initData: { + metadataStatusHeight: number; + outputStatusHeight: number; + fontInfo: FontInfo | undefined; + } + ) { + super(); + + this._register(this.editorEventDispatcher.onDidChangeLayout(e => { + this._layoutInfoEmitter.fire({ outerWidth: true }); + })); + } + + abstract layoutChange(): void; + abstract getHeight(lineHeight: number): number; + abstract get totalHeight(): number; +} + +export class DiffElementPlaceholderViewModel extends DiffElementViewModelBase { + readonly type: 'placeholder' = 'placeholder'; + public hiddenCells: DiffElementCellViewModelBase[] = []; + protected _unfoldHiddenCells = this._register(new Emitter()); + onUnfoldHiddenCells = this._unfoldHiddenCells.event; + + constructor( + mainDocumentTextModel: INotebookTextModel, + editorEventDispatcher: NotebookDiffEditorEventDispatcher, + initData: { + metadataStatusHeight: number; + outputStatusHeight: number; + fontInfo: FontInfo | undefined; + } + ) { + super(mainDocumentTextModel, editorEventDispatcher, initData); + + } + get totalHeight() { + return 50; + } + getHeight(lineHeight: number): number { + return this.totalHeight; + } + override layoutChange(): void { + // + console.log('layout change in placeholder'); + } + showHiddenCells() { + this._unfoldHiddenCells.fire(); + } +} + +export abstract class DiffElementCellViewModelBase extends DiffElementViewModelBase { public cellFoldingState: PropertyFoldingState; public metadataFoldingState: PropertyFoldingState; public outputFoldingState: PropertyFoldingState; - protected _layoutInfoEmitter = this._register(new Emitter()); - onDidLayoutChange = this._layoutInfoEmitter.event; protected _stateChangeEmitter = this._register(new Emitter<{ renderOutput: boolean }>()); onDidStateChange = this._stateChangeEmitter.event; protected _layoutInfo!: IDiffElementLayoutInfo; + public unchangedCells?: number; + private _hideUnchangedCells = this._register(new Emitter()); + public onHideUnchangedCells = this._hideUnchangedCells.event; + + hideUnchangedCells() { + this._hideUnchangedCells.fire(); + } set rawOutputHeight(height: number) { this._layout({ rawOutputHeight: Math.min(OUTPUT_EDITOR_HEIGHT_MAGIC, height) }); } @@ -115,23 +179,27 @@ export abstract class DiffElementViewModelBase extends Disposable { return this._layoutInfo; } + get totalHeight() { + return this.layoutInfo.totalHeight; + } + private _sourceEditorViewState: editorCommon.ICodeEditorViewState | editorCommon.IDiffEditorViewState | null = null; private _outputEditorViewState: editorCommon.ICodeEditorViewState | editorCommon.IDiffEditorViewState | null = null; private _metadataEditorViewState: editorCommon.ICodeEditorViewState | editorCommon.IDiffEditorViewState | null = null; constructor( - readonly mainDocumentTextModel: INotebookTextModel, + mainDocumentTextModel: INotebookTextModel, readonly original: DiffNestedCellViewModel | undefined, readonly modified: DiffNestedCellViewModel | undefined, readonly type: 'unchanged' | 'insert' | 'delete' | 'modified', - readonly editorEventDispatcher: NotebookDiffEditorEventDispatcher, - readonly initData: { + editorEventDispatcher: NotebookDiffEditorEventDispatcher, + initData: { metadataStatusHeight: number; outputStatusHeight: number; fontInfo: FontInfo | undefined; } ) { - super(); + super(mainDocumentTextModel, editorEventDispatcher, initData); const editorHeight = this._estimateEditorHeight(initData.fontInfo); const cellStatusHeight = 25; this._layoutInfo = { @@ -385,7 +453,7 @@ export abstract class DiffElementViewModelBase extends Disposable { } } -export class SideBySideDiffElementViewModel extends DiffElementViewModelBase { +export class SideBySideDiffElementViewModel extends DiffElementCellViewModelBase { get originalDocument() { return this.otherDocumentTextModel; } @@ -543,7 +611,7 @@ export class SideBySideDiffElementViewModel extends DiffElementViewModelBase { } } -export class SingleSideDiffElementViewModel extends DiffElementViewModelBase { +export class SingleSideDiffElementViewModel extends DiffElementCellViewModelBase { get cellViewModel() { return this.type === 'insert' ? this.modified! : this.original!; } diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css index fbc0471192ee4..7faea525525bb 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css @@ -25,6 +25,11 @@ flex-direction: row; } +.notebook-text-diff-editor .cell-placeholder-body { + display: flex; + flex-direction: row; +} + .notebook-text-diff-editor .webview-cover { user-select: initial; -webkit-user-select: initial; @@ -299,7 +304,7 @@ width: 15px !important; } -.monaco-workbench .notebook-text-diff-editor > .monaco-list > .monaco-scrollable-element > .scrollbar.visible { +.monaco-workbench .notebook-text-diff-editor > .monaco-list > .monaco-scrollable-element > .scrollbar.visible { z-index: var(--z-index-notebook-scrollbar); cursor: default; } @@ -399,3 +404,58 @@ .notebook-text-diff-editor .cell-body .cell-diff-editor-container .source-container .monaco-editor .monaco-editor-background { background: var(--vscode-notebook-cellEditorBackground, var(--vscode-editor-background)); } + +/** Overlay to hide the unchanged cells */ +.notebook-text-diff-editor .cell-body.full div.diff-hidden-cells { + position: absolute; + left: 0; +} + +.notebook-text-diff-editor .cell-body.full div.diff-hidden-cells .center { + color: var(--vscode-diffEditor-unchangedRegionForeground); + overflow: hidden; + display: block; + white-space: nowrap; + + height: 24px; +} + +.notebook-text-diff-editor .cell-body.full div.diff-hidden-cells .center span.codicon { + vertical-align: middle; +} + +.notebook-text-diff-editor .cell-body.full div.diff-hidden-cells .center a:hover .codicon { + cursor: pointer; +} + +/** Overlay to unhide the unchanged cells */ +.notebook-text-diff-editor .cell-placeholder-body { + background: var(--vscode-diffEditor-unchangedRegionBackground); + color: var(--vscode-diffEditor-unchangedRegionForeground); + min-height: 24px; +} + +.notebook-text-diff-editor .cell-placeholder-body div.diff-hidden-cells .center { + overflow: hidden; + display: block; + text-overflow: ellipsis; + white-space: nowrap; + + height: 24px; + box-shadow: inset 0 -5px 5px -7px var(--vscode-diffEditor-unchangedRegionShadow), inset 0 5px 5px -7px var(--vscode-diffEditor-unchangedRegionShadow); +} + +.notebook-text-diff-editor .cell-placeholder-body .text { + /** Add a gap between text and the unfold icon */ + padding-left: 2px; +} + +.notebook-text-diff-editor .cell-placeholder-body div.diff-hidden-cells .center span.codicon, +.notebook-text-diff-editor .cell-placeholder-body .text { + vertical-align: middle; +} + +.notebook-text-diff-editor .cell-placeholder-body div.diff-hidden-cells .center a:hover .codicon { + cursor: pointer; + color: var(--vscode-editorLink-activeForeground) !important; +} diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffActions.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffActions.ts index d1ea8b55dd018..11b4d887ba980 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffActions.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffActions.ts @@ -10,7 +10,7 @@ import { IConfigurationService } from 'vs/platform/configuration/common/configur import { ContextKeyExpr, ContextKeyExpression } from 'vs/platform/contextkey/common/contextkey'; import { ServicesAccessor } from 'vs/platform/instantiation/common/instantiation'; import { ActiveEditorContext } from 'vs/workbench/common/contextkeys'; -import { DiffElementViewModelBase, SideBySideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { DiffElementCellViewModelBase, SideBySideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; import { INotebookTextDiffEditor, NOTEBOOK_DIFF_CELL_INPUT, NOTEBOOK_DIFF_CELL_PROPERTY, NOTEBOOK_DIFF_CELL_PROPERTY_EXPANDED } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; import { NotebookTextDiffEditor } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor'; import { NotebookDiffEditorInput } from 'vs/workbench/contrib/notebook/common/notebookDiffEditorInput'; @@ -109,7 +109,7 @@ registerAction2(class extends Action2 { } ); } - run(accessor: ServicesAccessor, context?: { cell: DiffElementViewModelBase }) { + run(accessor: ServicesAccessor, context?: { cell: DiffElementCellViewModelBase }) { if (!context) { return; } @@ -164,7 +164,7 @@ registerAction2(class extends Action2 { } ); } - run(accessor: ServicesAccessor, context?: { cell: DiffElementViewModelBase }) { + run(accessor: ServicesAccessor, context?: { cell: DiffElementCellViewModelBase }) { if (!context) { return; } @@ -189,7 +189,7 @@ registerAction2(class extends Action2 { } ); } - run(accessor: ServicesAccessor, context?: { cell: DiffElementViewModelBase }) { + run(accessor: ServicesAccessor, context?: { cell: DiffElementCellViewModelBase }) { if (!context) { return; } @@ -230,7 +230,7 @@ registerAction2(class extends Action2 { } ); } - run(accessor: ServicesAccessor, context?: { cell: DiffElementViewModelBase }) { + run(accessor: ServicesAccessor, context?: { cell: DiffElementCellViewModelBase }) { if (!context) { return; } diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index 590b3c0482690..4288e2f1faf54 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -14,9 +14,9 @@ import { getDefaultNotebookCreationOptions } from 'vs/workbench/contrib/notebook import { IEditorGroup } from 'vs/workbench/services/editor/common/editorGroupsService'; import { NotebookDiffEditorInput } from '../../common/notebookDiffEditorInput'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; -import { DiffElementViewModelBase, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { DiffElementCellViewModelBase, DiffElementPlaceholderViewModel, IDiffElementViewModelBase, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; -import { CellDiffSideBySideRenderer, CellDiffSingleSideRenderer, NotebookCellTextDiffListDelegate, NotebookTextDiffList } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffList'; +import { CellDiffPlaceholderRenderer, CellDiffSideBySideRenderer, CellDiffSingleSideRenderer, NotebookCellTextDiffListDelegate, NotebookTextDiffList } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffList'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; import { diffDiagonalFill, editorBackground, focusBorder, foreground } from 'vs/platform/theme/common/colorRegistry'; import { INotebookEditorWorkerService } from 'vs/workbench/contrib/notebook/common/services/notebookWorkerService'; @@ -94,14 +94,14 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD private _overviewRulerContainer!: HTMLElement; private _overviewRuler!: NotebookDiffOverviewRuler; private _dimension: DOM.Dimension | null = null; - private _diffElementViewModels: DiffElementViewModelBase[] = []; + private _diffElementViewModels: IDiffElementViewModelBase[] = []; private _list!: NotebookTextDiffList; private _modifiedWebview: BackLayerWebView | null = null; private _originalWebview: BackLayerWebView | null = null; private _webviewTransparentCover: HTMLElement | null = null; private _fontInfo: FontInfo | undefined; - private readonly _onMouseUp = this._register(new Emitter<{ readonly event: MouseEvent; readonly target: DiffElementViewModelBase }>()); + private readonly _onMouseUp = this._register(new Emitter<{ readonly event: MouseEvent; readonly target: IDiffElementViewModelBase }>()); public readonly onMouseUp = this._onMouseUp.event; private readonly _onDidScroll = this._register(new Emitter()); readonly onDidScroll: Event = this._onDidScroll.event; @@ -277,6 +277,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD const renderers = [ this.instantiationService.createInstance(CellDiffSingleSideRenderer, this), this.instantiationService.createInstance(CellDiffSideBySideRenderer, this), + this.instantiationService.createInstance(CellDiffPlaceholderRenderer, this), ]; this._listViewContainer = DOM.append(this._rootElement, DOM.$('.notebook-diff-list-view')); @@ -378,7 +379,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD this._overviewRuler = this._register(this.instantiationService.createInstance(NotebookDiffOverviewRuler, this, NotebookTextDiffEditor.ENTIRE_DIFF_OVERVIEW_WIDTH, this._overviewRulerContainer)); } - private _updateOutputsOffsetsInWebview(scrollTop: number, scrollHeight: number, activeWebview: BackLayerWebView, getActiveNestedCell: (diffElement: DiffElementViewModelBase) => DiffNestedCellViewModel | undefined, diffSide: DiffSide) { + private _updateOutputsOffsetsInWebview(scrollTop: number, scrollHeight: number, activeWebview: BackLayerWebView, getActiveNestedCell: (diffElement: DiffElementCellViewModelBase) => DiffNestedCellViewModel | undefined, diffSide: DiffSide) { activeWebview.element.style.height = `${scrollHeight}px`; if (activeWebview.insetMapping) { @@ -485,13 +486,13 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD } if (this._modifiedWebview) { - this._updateOutputsOffsetsInWebview(this._list.scrollTop, this._list.scrollHeight, this._modifiedWebview, (diffElement: DiffElementViewModelBase) => { + this._updateOutputsOffsetsInWebview(this._list.scrollTop, this._list.scrollHeight, this._modifiedWebview, (diffElement: DiffElementCellViewModelBase) => { return diffElement.modified; }, DiffSide.Modified); } if (this._originalWebview) { - this._updateOutputsOffsetsInWebview(this._list.scrollTop, this._list.scrollHeight, this._originalWebview, (diffElement: DiffElementViewModelBase) => { + this._updateOutputsOffsetsInWebview(this._list.scrollTop, this._list.scrollHeight, this._originalWebview, (diffElement: DiffElementCellViewModelBase) => { return diffElement.original; }, DiffSide.Original); } @@ -567,9 +568,6 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD this._setViewModel(viewModels); } - // this._diffElementViewModels = viewModels; - // this._list.splice(0, this._list.length, this._diffElementViewModels); - if (this._revealFirst && firstChangeIndex !== -1 && firstChangeIndex < this._list.length) { this._revealFirst = false; this._list.setFocus([firstChangeIndex]); @@ -581,17 +579,22 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD } } - private _isViewModelTheSame(viewModels: DiffElementViewModelBase[]) { + private _isViewModelTheSame(viewModels: IDiffElementViewModelBase[]) { let isSame = true; if (this._diffElementViewModels.length === viewModels.length) { for (let i = 0; i < viewModels.length; i++) { const a = this._diffElementViewModels[i]; const b = viewModels[i]; - - if (a.original?.textModel.getHashValue() !== b.original?.textModel.getHashValue() + if (a.type === 'placeholder' || b.type === 'placeholder') { + if (a.type !== b.type) { + isSame = false; + break; + } + } else if (a.original?.textModel.getHashValue() !== b.original?.textModel.getHashValue() || a.modified?.textModel.getHashValue() !== b.modified?.textModel.getHashValue()) { isSame = false; break; + } } } else { @@ -601,11 +604,39 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD return isSame; } - private _setViewModel(viewModels: DiffElementViewModelBase[]) { + private readonly viewModelStore = this._register(new DisposableStore()); + private _setViewModel(viewModels: DiffElementCellViewModelBase[]) { this._diffElementViewModels = viewModels; - this._list.splice(0, this._list.length, this._diffElementViewModels); + this.viewModelStore.clear(); + + const newViewModel: IDiffElementViewModelBase[] = []; + let placeholder: DiffElementPlaceholderViewModel | undefined = undefined; + viewModels.forEach((vm, index) => { + if (vm.type === 'unchanged') { + if (!placeholder) { + placeholder = new DiffElementPlaceholderViewModel(vm.mainDocumentTextModel, vm.editorEventDispatcher, vm.initData); + const placeholderItem = placeholder; + this.viewModelStore.add(placeholderItem.onUnfoldHiddenCells(() => { + this._list.splice(index, 1, placeholderItem.hiddenCells); + })); + vm.unchangedCells = 1; + this._register(vm.onHideUnchangedCells(() => { + this._list.splice(index, placeholderItem.hiddenCells.length, [placeholderItem]); + })); + } + placeholder.hiddenCells.push(vm); + } else { + if (placeholder) { + newViewModel.push(placeholder); + placeholder.hiddenCells[0].unchangedCells = placeholder.hiddenCells.length; + } + placeholder = undefined; + newViewModel.push(vm); + } + }); + this._list.splice(0, this._list.length, newViewModel); if (this.isOverviewRulerEnabled()) { - this._overviewRuler.updateViewModels(this._diffElementViewModels, this._eventDispatcher); + this._overviewRuler.updateViewModels(newViewModel, this._eventDispatcher); } } @@ -649,7 +680,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD static computeDiff(instantiationService: IInstantiationService, configurationService: IConfigurationService, model: INotebookDiffEditorModel, eventDispatcher: NotebookDiffEditorEventDispatcher, diffResult: INotebookDiffResult, fontInfo: FontInfo | undefined) { const cellChanges = diffResult.cellsDiff.changes; - const diffElementViewModels: DiffElementViewModelBase[] = []; + const diffElementViewModels: DiffElementCellViewModelBase[] = []; const originalModel = model.original.notebook; const modifiedModel = model.modified.notebook; let originalCellIndex = 0; @@ -729,7 +760,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD outputStatusHeight: number; fontInfo: FontInfo | undefined; }) { - const result: DiffElementViewModelBase[] = []; + const result: DiffElementCellViewModelBase[] = []; // modified cells const modifiedLen = Math.min(change.originalLength, change.modifiedLength); @@ -798,11 +829,11 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD }, 10); } - private pendingLayouts = new WeakMap(); + private pendingLayouts = new WeakMap(); - layoutNotebookCell(cell: DiffElementViewModelBase, height: number) { - const relayout = (cell: DiffElementViewModelBase, height: number) => { + layoutNotebookCell(cell: DiffElementCellViewModelBase, height: number) { + const relayout = (cell: DiffElementCellViewModelBase, height: number) => { this._list.updateElementHeight2(cell, height); }; @@ -896,7 +927,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD } } - createOutput(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: DiffNestedCellViewModel, output: IInsetRenderOutput, getOffset: () => number, diffSide: DiffSide): void { + createOutput(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: DiffNestedCellViewModel, output: IInsetRenderOutput, getOffset: () => number, diffSide: DiffSide): void { this._insetModifyQueueByOutputId.queue(output.source.model.outputId + (diffSide === DiffSide.Modified ? '-right' : 'left'), async () => { const activeWebview = diffSide === DiffSide.Modified ? this._modifiedWebview : this._originalWebview; if (!activeWebview) { @@ -933,7 +964,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD throw new Error('Not implemented'); } - removeInset(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: DiffNestedCellViewModel, displayOutput: ICellOutputViewModel, diffSide: DiffSide) { + removeInset(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: DiffNestedCellViewModel, displayOutput: ICellOutputViewModel, diffSide: DiffSide) { this._insetModifyQueueByOutputId.queue(displayOutput.model.outputId + (diffSide === DiffSide.Modified ? '-right' : 'left'), async () => { const activeWebview = diffSide === DiffSide.Modified ? this._modifiedWebview : this._originalWebview; if (!activeWebview) { @@ -948,7 +979,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD }); } - showInset(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: DiffNestedCellViewModel, displayOutput: ICellOutputViewModel, diffSide: DiffSide) { + showInset(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: DiffNestedCellViewModel, displayOutput: ICellOutputViewModel, diffSide: DiffSide) { this._insetModifyQueueByOutputId.queue(displayOutput.model.outputId + (diffSide === DiffSide.Modified ? '-right' : 'left'), async () => { const activeWebview = diffSide === DiffSide.Modified ? this._modifiedWebview : this._originalWebview; if (!activeWebview) { @@ -972,7 +1003,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD }); } - hideInset(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: DiffNestedCellViewModel, output: ICellOutputViewModel) { + hideInset(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: DiffNestedCellViewModel, output: ICellOutputViewModel) { this._modifiedWebview?.hideInset(output); this._originalWebview?.hideInset(output); } @@ -1027,56 +1058,6 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD }; } - getCellOutputLayoutInfo(nestedCell: DiffNestedCellViewModel) { - if (!this._model) { - throw new Error('Editor is not attached to model yet'); - } - const documentModel = CellUri.parse(nestedCell.uri); - if (!documentModel) { - throw new Error('Nested cell in the diff editor has wrong Uri'); - } - - const belongToOriginalDocument = this._model.original.notebook.uri.toString() === documentModel.notebook.toString(); - const viewModel = this._diffElementViewModels.find(element => { - const textModel = belongToOriginalDocument ? element.original : element.modified; - if (!textModel) { - return false; - } - - if (textModel.uri.toString() === nestedCell.uri.toString()) { - return true; - } - - return false; - }); - - if (!viewModel) { - throw new Error('Nested cell in the diff editor does not match any diff element'); - } - - if (viewModel.type === 'unchanged') { - return this.getLayoutInfo(); - } - - if (viewModel.type === 'insert' || viewModel.type === 'delete') { - return { - width: this._dimension!.width / 2, - height: this._dimension!.height / 2, - fontInfo: this.fontInfo - }; - } - - if (viewModel.checkIfOutputsModified()) { - return { - width: this._dimension!.width / 2, - height: this._dimension!.height / 2, - fontInfo: this.fontInfo - }; - } else { - return this.getLayoutInfo(); - } - } - layout(dimension: DOM.Dimension, _position: DOM.IDomPosition): void { this._rootElement.classList.toggle('mid-width', dimension.width < 1000 && dimension.width >= 600); this._rootElement.classList.toggle('narrow-width', dimension.width < 600); @@ -1136,4 +1117,6 @@ registerThemingParticipant((theme, collector) => { `); collector.addRule(`.notebook-text-diff-editor .cell-body { margin: ${DIFF_CELL_MARGIN}px; }`); + // We do not want a left margin, as we add an overlay for expanind the collapsed/hidden cells. + collector.addRule(`.notebook-text-diff-editor .cell-placeholder-body { margin: ${DIFF_CELL_MARGIN}px; margin-left:0px }`); }); diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts index 0de4d56b51133..4fda303c2f158 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts @@ -4,10 +4,10 @@ *--------------------------------------------------------------------------------------------*/ import { CellLayoutState, ICellOutputViewModel, ICommonCellInfo, IGenericCellViewModel, IInsetRenderOutput } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { DiffElementViewModelBase } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { DiffElementCellViewModelBase, IDiffElementViewModelBase } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; import { Event } from 'vs/base/common/event'; import { BareFontInfo } from 'vs/editor/common/config/fontInfo'; -import { DisposableStore } from 'vs/base/common/lifecycle'; +import { DisposableStore, IDisposable } from 'vs/base/common/lifecycle'; import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel'; import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditor/codeEditorWidget'; import { IMouseWheelEvent } from 'vs/base/browser/mouseEvent'; @@ -23,24 +23,24 @@ export enum DiffSide { } export interface IDiffCellInfo extends ICommonCellInfo { - diffElement: DiffElementViewModelBase; + diffElement: DiffElementCellViewModelBase; } export interface INotebookTextDiffEditor { notebookOptions: NotebookOptions; readonly textModel?: NotebookTextModel; - onMouseUp: Event<{ readonly event: MouseEvent; readonly target: DiffElementViewModelBase }>; + onMouseUp: Event<{ readonly event: MouseEvent; readonly target: IDiffElementViewModelBase }>; onDidScroll: Event; onDidDynamicOutputRendered: Event<{ cell: IGenericCellViewModel; output: ICellOutputViewModel }>; getOverflowContainerDomNode(): HTMLElement; getLayoutInfo(): NotebookLayoutInfo; getScrollTop(): number; getScrollHeight(): number; - layoutNotebookCell(cell: DiffElementViewModelBase, height: number): void; - createOutput(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: IDiffNestedCellViewModel, output: IInsetRenderOutput, getOffset: () => number, diffSide: DiffSide): void; - showInset(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: IDiffNestedCellViewModel, displayOutput: ICellOutputViewModel, diffSide: DiffSide): void; - removeInset(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: IDiffNestedCellViewModel, output: ICellOutputViewModel, diffSide: DiffSide): void; - hideInset(cellDiffViewModel: DiffElementViewModelBase, cellViewModel: IDiffNestedCellViewModel, output: ICellOutputViewModel): void; + layoutNotebookCell(cell: DiffElementCellViewModelBase, height: number): void; + createOutput(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: IDiffNestedCellViewModel, output: IInsetRenderOutput, getOffset: () => number, diffSide: DiffSide): void; + showInset(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: IDiffNestedCellViewModel, displayOutput: ICellOutputViewModel, diffSide: DiffSide): void; + removeInset(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: IDiffNestedCellViewModel, output: ICellOutputViewModel, diffSide: DiffSide): void; + hideInset(cellDiffViewModel: DiffElementCellViewModelBase, cellViewModel: IDiffNestedCellViewModel, output: ICellOutputViewModel): void; /** * Trigger the editor to scroll from scroll event programmatically */ @@ -66,6 +66,14 @@ export interface CellDiffCommonRenderTemplate { readonly bottomBorder: HTMLElement; } +export interface CellDiffPlaceholderRenderTemplate { + readonly container: HTMLElement; + readonly placeholder: HTMLElement; + readonly body: HTMLElement; + readonly marginOverlay: IDiffCellMarginOverlay; + readonly elementDisposables: DisposableStore; +} + export interface CellDiffSingleSideRenderTemplate extends CellDiffCommonRenderTemplate { readonly container: HTMLElement; readonly body: HTMLElement; @@ -81,6 +89,11 @@ export interface CellDiffSingleSideRenderTemplate extends CellDiffCommonRenderTe readonly outputInfoContainer: HTMLElement; } +export interface IDiffCellMarginOverlay extends IDisposable { + onAction: Event; + show(): void; + hide(): void; +} export interface CellDiffSideBySideRenderTemplate extends CellDiffCommonRenderTemplate { readonly container: HTMLElement; @@ -96,6 +109,7 @@ export interface CellDiffSideBySideRenderTemplate extends CellDiffCommonRenderTe readonly metadataInfoContainer: HTMLElement; readonly outputHeaderContainer: HTMLElement; readonly outputInfoContainer: HTMLElement; + readonly marginOverlay: IDiffCellMarginOverlay; } export interface IDiffElementLayoutInfo { diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts index 03d610fb9d6a9..2afc794459d7e 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts @@ -14,9 +14,9 @@ import { IInstantiationService } from 'vs/platform/instantiation/common/instanti import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { IListService, IWorkbenchListOptions, WorkbenchList } from 'vs/platform/list/browser/listService'; import { IThemeService } from 'vs/platform/theme/common/themeService'; -import { DiffElementViewModelBase, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; -import { CellDiffSideBySideRenderTemplate, CellDiffSingleSideRenderTemplate, DIFF_CELL_MARGIN, INotebookTextDiffEditor } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; -import { DeletedElement, getOptimizedNestedCodeEditorWidgetOptions, InsertElement, ModifiedElement } from 'vs/workbench/contrib/notebook/browser/diff/diffComponents'; +import { DiffElementPlaceholderViewModel, IDiffElementViewModelBase, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { CellDiffPlaceholderRenderTemplate, CellDiffSideBySideRenderTemplate, CellDiffSingleSideRenderTemplate, DIFF_CELL_MARGIN, INotebookTextDiffEditor } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; +import { CellDiffPlaceholderElement, CollapsedCellOverlayWidget, DeletedElement, getOptimizedNestedCodeEditorWidgetOptions, InsertElement, ModifiedElement, UnchangedCellOverlayWidget } from 'vs/workbench/contrib/notebook/browser/diff/diffComponents'; import { CodeEditorWidget } from 'vs/editor/browser/widget/codeEditor/codeEditorWidget'; import { DiffEditorWidget } from 'vs/editor/browser/widget/diffEditor/diffEditorWidget'; import { IMenuService, MenuItemAction } from 'vs/platform/actions/common/actions'; @@ -30,8 +30,9 @@ import { PixelRatio } from 'vs/base/browser/pixelRatio'; import { WorkbenchToolBar } from 'vs/platform/actions/browser/toolbar'; import { fixedDiffEditorOptions, fixedEditorOptions } from 'vs/workbench/contrib/notebook/browser/diff/diffCellEditorOptions'; import { IAccessibilityService } from 'vs/platform/accessibility/common/accessibility'; +import { localize } from 'vs/nls'; -export class NotebookCellTextDiffListDelegate implements IListVirtualDelegate { +export class NotebookCellTextDiffListDelegate implements IListVirtualDelegate { private readonly lineHeight: number; constructor( @@ -42,15 +43,15 @@ export class NotebookCellTextDiffListDelegate implements IListVirtualDelegate { + static readonly TEMPLATE_ID = 'cell_diff_placeholder'; + + constructor( + readonly notebookEditor: INotebookTextDiffEditor, + @IInstantiationService protected readonly instantiationService: IInstantiationService + ) { } + get templateId() { + return CellDiffPlaceholderRenderer.TEMPLATE_ID; + } + + renderTemplate(container: HTMLElement): CellDiffPlaceholderRenderTemplate { + const body = DOM.$('.cell-placeholder-body'); + DOM.append(container, body); + + const elementDisposables = new DisposableStore(); + const marginOverlay = new CollapsedCellOverlayWidget(body); + const contents = DOM.append(body, DOM.$('.contents')); + const placeholder = DOM.append(contents, DOM.$('span.text', { title: localize('notebook.diff.hiddenCells.expandAll', 'Double click to unfold') })); + + return { + body, + container, + placeholder, + marginOverlay, + elementDisposables + }; + } + + renderElement(element: DiffElementPlaceholderViewModel, index: number, templateData: CellDiffPlaceholderRenderTemplate, height: number | undefined): void { + templateData.body.classList.remove('left', 'right', 'full'); + templateData.elementDisposables.add(this.instantiationService.createInstance(CellDiffPlaceholderElement, element, templateData)); + } + + disposeTemplate(templateData: CellDiffPlaceholderRenderTemplate): void { + templateData.container.innerText = ''; + templateData.elementDisposables.dispose(); + } + + disposeElement(element: DiffElementPlaceholderViewModel, index: number, templateData: CellDiffPlaceholderRenderTemplate): void { + templateData.elementDisposables.clear(); } } + export class CellDiffSingleSideRenderer implements IListRenderer { static readonly TEMPLATE_ID = 'cell_diff_single'; @@ -215,7 +263,8 @@ export class CellDiffSideBySideRenderer implements IListRenderer extends MouseController { } } -export class NotebookTextDiffList extends WorkbenchList implements IDisposable, IStyleController { +export class NotebookTextDiffList extends WorkbenchList implements IDisposable, IStyleController { private styleElement?: HTMLStyleElement; get rowsContainer(): HTMLElement { @@ -315,21 +365,21 @@ export class NotebookTextDiffList extends WorkbenchList, - renderers: IListRenderer[], + delegate: IListVirtualDelegate, + renderers: IListRenderer[], contextKeyService: IContextKeyService, - options: IWorkbenchListOptions, + options: IWorkbenchListOptions, @IListService listService: IListService, @IConfigurationService configurationService: IConfigurationService, @IInstantiationService instantiationService: IInstantiationService) { super(listUser, container, delegate, renderers, options, contextKeyService, listService, configurationService, instantiationService); } - protected override createMouseController(options: IListOptions): MouseController { + protected override createMouseController(options: IListOptions): MouseController { return new NotebookMouseController(this); } - getCellViewScrollTop(element: DiffElementViewModelBase): number { + getCellViewScrollTop(element: IDiffElementViewModelBase): number { const index = this.indexOf(element); // if (index === undefined || index < 0 || index >= this.length) { // this._getViewIndexUpperBound(element); @@ -356,7 +406,7 @@ export class NotebookTextDiffList extends WorkbenchList; private readonly _overviewViewportDomElement: FastDomNode; - private _diffElementViewModels: DiffElementViewModelBase[] = []; + private _diffElementViewModels: IDiffElementViewModelBase[] = []; private _lanes = 2; private _insertColor: Color | null; @@ -94,7 +94,7 @@ export class NotebookDiffOverviewRuler extends Themable { this._layoutNow(); } - updateViewModels(elements: DiffElementViewModelBase[], eventDispatcher: NotebookDiffEditorEventDispatcher | undefined) { + updateViewModels(elements: IDiffElementViewModelBase[], eventDispatcher: NotebookDiffEditorEventDispatcher | undefined) { this._disposables.clear(); this._diffElementViewModels = elements; @@ -126,7 +126,7 @@ export class NotebookDiffOverviewRuler extends Themable { private _layoutNow() { const layoutInfo = this.notebookEditor.getLayoutInfo(); const height = layoutInfo.height; - const contentHeight = this._diffElementViewModels.map(view => view.layoutInfo.totalHeight).reduce((a, b) => a + b, 0); + const contentHeight = this._diffElementViewModels.map(view => view.totalHeight).reduce((a, b) => a + b, 0); const ratio = PixelRatio.getInstance(DOM.getWindow(this._domNode.domNode)).value; this._domNode.setWidth(this.width); this._domNode.setHeight(height); @@ -182,8 +182,7 @@ export class NotebookDiffOverviewRuler extends Themable { for (let i = 0; i < this._diffElementViewModels.length; i++) { const element = this._diffElementViewModels[i]; - const cellHeight = Math.round((element.layoutInfo.totalHeight / scrollHeight) * ratio * height); - + const cellHeight = Math.round((element.totalHeight / scrollHeight) * ratio * height); switch (element.type) { case 'insert': ctx.fillStyle = this._insertColorHex; @@ -203,6 +202,7 @@ export class NotebookDiffOverviewRuler extends Themable { break; } + currentFrom += cellHeight; } } From 4a8ef1b2446b6bfc80d1a940afd5d82edcdd26ac Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Wed, 7 Aug 2024 17:04:10 +1000 Subject: [PATCH 02/11] misc --- .../contrib/notebook/browser/diff/notebookDiff.css | 7 ++++++- .../contrib/notebook/browser/diff/notebookDiffList.ts | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css index 7faea525525bb..a2bd135ab5da5 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css @@ -409,6 +409,9 @@ .notebook-text-diff-editor .cell-body.full div.diff-hidden-cells { position: absolute; left: 0; + + font-size: 13px; + line-height: 14px; } .notebook-text-diff-editor .cell-body.full div.diff-hidden-cells .center { @@ -433,6 +436,9 @@ background: var(--vscode-diffEditor-unchangedRegionBackground); color: var(--vscode-diffEditor-unchangedRegionForeground); min-height: 24px; + + /* font-size: 13px; + line-height: 14px; */ } .notebook-text-diff-editor .cell-placeholder-body div.diff-hidden-cells .center { @@ -442,7 +448,6 @@ white-space: nowrap; height: 24px; - box-shadow: inset 0 -5px 5px -7px var(--vscode-diffEditor-unchangedRegionShadow), inset 0 5px 5px -7px var(--vscode-diffEditor-unchangedRegionShadow); } .notebook-text-diff-editor .cell-placeholder-body .text { diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts index 2afc794459d7e..3c7bdcb276a20 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts @@ -84,7 +84,7 @@ export class CellDiffPlaceholderRenderer implements IListRenderer Date: Wed, 7 Aug 2024 22:37:09 +1000 Subject: [PATCH 03/11] Wip --- .../notebook/browser/diff/diffComponents.ts | 20 ++++--------------- .../browser/diff/diffElementViewModel.ts | 13 ++++-------- .../notebook/browser/diff/notebookDiff.css | 3 --- .../notebook/browser/diff/notebookDiffList.ts | 1 - 4 files changed, 8 insertions(+), 29 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts index 524bc898d72fe..d10cf2de00894 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts @@ -73,22 +73,14 @@ export class CellDiffPlaceholderElement extends Disposable { localize('hiddenCells', '{0} hidden cells', placeholder.hiddenCells.length); templateData.placeholder.innerText = text; - const handler = (e: MouseEvent) => { + this._register(DOM.addDisposableListener(templateData.placeholder, 'dblclick', (e: MouseEvent) => { if (e.button !== 0) { return; } e.preventDefault(); placeholder.showHiddenCells(); - }; - templateData.placeholder.addEventListener('dblclick', handler); - this._register({ - dispose: () => { - templateData.placeholder.removeEventListener('dblclick', handler); - } - }); - this._register(templateData.marginOverlay.onAction(() => { - placeholder.showHiddenCells(); })); + this._register(templateData.marginOverlay.onAction(() => placeholder.showHiddenCells())); templateData.marginOverlay.show(); } } @@ -1399,9 +1391,7 @@ export class ModifiedElement extends AbstractElementRenderer { override buildBody(): void { super.buildBody(); if (this.cell.unchangedCells) { - this._register(this.templateData.marginOverlay.onAction(() => { - this.cell.hideUnchangedCells(); - })); + this._register(this.templateData.marginOverlay.onAction(() => this.cell.hideUnchangedCells())); this.templateData.marginOverlay.show(); } else { this.templateData.marginOverlay.hide(); @@ -1852,9 +1842,7 @@ export class CollapsedCellOverlayWidget extends Disposable implements IDiffCellM this._nodes.root.style.display = 'none'; container.appendChild(this._nodes.root); - this._register(DOM.addDisposableListener(this._nodes.content.children[0], 'click', () => { - this._action.fire(); - })); + this._register(DOM.addDisposableListener(this._nodes.content.children[0], 'click', () => this._action.fire())); } public show() { this._nodes.root.style.display = 'block'; diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts index bd456b420abe5..200ccdaccb86e 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts @@ -49,9 +49,7 @@ export abstract class DiffElementViewModelBase extends Disposable { ) { super(); - this._register(this.editorEventDispatcher.onDidChangeLayout(e => { - this._layoutInfoEmitter.fire({ outerWidth: true }); - })); + this._register(this.editorEventDispatcher.onDidChangeLayout(e => this._layoutInfoEmitter.fire({ outerWidth: true }))); } abstract layoutChange(): void; @@ -78,14 +76,13 @@ export class DiffElementPlaceholderViewModel extends DiffElementViewModelBase { } get totalHeight() { - return 50; + return 24; } - getHeight(lineHeight: number): number { + getHeight(_: number): number { return this.totalHeight; } override layoutChange(): void { // - console.log('layout change in placeholder'); } showHiddenCells() { this._unfoldHiddenCells.fire(); @@ -222,9 +219,7 @@ export abstract class DiffElementCellViewModelBase extends DiffElementViewModelB this.metadataFoldingState = PropertyFoldingState.Collapsed; this.outputFoldingState = PropertyFoldingState.Collapsed; - this._register(this.editorEventDispatcher.onDidChangeLayout(e => { - this._layoutInfoEmitter.fire({ outerWidth: true }); - })); + this._register(this.editorEventDispatcher.onDidChangeLayout(e => this._layoutInfoEmitter.fire({ outerWidth: true }))); } layoutChange() { diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css index a2bd135ab5da5..9004d5f6e120d 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiff.css @@ -436,9 +436,6 @@ background: var(--vscode-diffEditor-unchangedRegionBackground); color: var(--vscode-diffEditor-unchangedRegionForeground); min-height: 24px; - - /* font-size: 13px; - line-height: 14px; */ } .notebook-text-diff-editor .cell-placeholder-body div.diff-hidden-cells .center { diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts index 3c7bdcb276a20..c0f4fcd898a20 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffList.ts @@ -102,7 +102,6 @@ export class CellDiffPlaceholderRenderer implements IListRenderer Date: Thu, 8 Aug 2024 07:40:10 +1000 Subject: [PATCH 04/11] Misc --- .../notebook/browser/diff/diffComponents.ts | 26 ++++++++++++++----- .../browser/diff/diffElementViewModel.ts | 2 +- .../browser/diff/notebookDiffEditor.ts | 10 +++---- 3 files changed, 25 insertions(+), 13 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts index d10cf2de00894..92d11a3bd8758 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts @@ -1828,7 +1828,11 @@ export class ModifiedElement extends AbstractElementRenderer { export class CollapsedCellOverlayWidget extends Disposable implements IDiffCellMarginOverlay { private readonly _nodes = DOM.h('div.diff-hidden-cells', [ DOM.h('div.center@content', { style: { display: 'flex' } }, [ - DOM.$('a', { title: localize('showUnchangedCells', 'Show Unchanged Cells'), role: 'button' }, + DOM.$('a', { + title: localize('showUnchangedCells', 'Show Unchanged Cells'), + role: 'button', + onclick: () => { this._action.fire(); } + }, ...renderLabelWithIcons('$(unfold)'))] ), ]); @@ -1836,13 +1840,12 @@ export class CollapsedCellOverlayWidget extends Disposable implements IDiffCellM private readonly _action = this._register(new Emitter()); public readonly onAction = this._action.event; constructor( - container: HTMLElement + private readonly container: HTMLElement ) { super(); this._nodes.root.style.display = 'none'; container.appendChild(this._nodes.root); - this._register(DOM.addDisposableListener(this._nodes.content.children[0], 'click', () => this._action.fire())); } public show() { this._nodes.root.style.display = 'block'; @@ -1852,6 +1855,8 @@ export class CollapsedCellOverlayWidget extends Disposable implements IDiffCellM } public override dispose() { this.hide(); + this.container.removeChild(this._nodes.root); + DOM.reset(this._nodes.root); super.dispose(); } } @@ -1859,21 +1864,26 @@ export class CollapsedCellOverlayWidget extends Disposable implements IDiffCellM export class UnchangedCellOverlayWidget extends Disposable implements IDiffCellMarginOverlay { private readonly _nodes = DOM.h('div.diff-hidden-cells', [ DOM.h('div.center@content', { style: { display: 'flex' } }, [ - DOM.$('a', { title: localize('hideUnchangedCells', 'Hide Unchanged Cells'), role: 'button' }, - ...renderLabelWithIcons('$(fold)'))] + DOM.$('a', { + title: localize('hideUnchangedCells', 'Hide Unchanged Cells'), + role: 'button', + onclick: () => { this._action.fire(); } + }, + ...renderLabelWithIcons('$(fold)') + ), + ] ), ]); private readonly _action = this._register(new Emitter()); public readonly onAction = this._action.event; constructor( - container: HTMLElement + private readonly container: HTMLElement ) { super(); this._nodes.root.style.display = 'none'; container.appendChild(this._nodes.root); - this._register(DOM.addDisposableListener(this._nodes.content.children[0], 'click', () => this._action.fire())); } public show() { this._nodes.root.style.display = 'block'; @@ -1883,6 +1893,8 @@ export class UnchangedCellOverlayWidget extends Disposable implements IDiffCellM } public override dispose() { this.hide(); + this.container.removeChild(this._nodes.root); + DOM.reset(this._nodes.root); super.dispose(); } } diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts index 200ccdaccb86e..e7da6e38a31e2 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts @@ -76,7 +76,7 @@ export class DiffElementPlaceholderViewModel extends DiffElementViewModelBase { } get totalHeight() { - return 24; + return 50; } getHeight(_: number): number { return this.totalHeight; diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index 4288e2f1faf54..d2c97172ec0c0 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -609,7 +609,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD this._diffElementViewModels = viewModels; this.viewModelStore.clear(); - const newViewModel: IDiffElementViewModelBase[] = []; + const newViewModels: IDiffElementViewModelBase[] = []; let placeholder: DiffElementPlaceholderViewModel | undefined = undefined; viewModels.forEach((vm, index) => { if (vm.type === 'unchanged') { @@ -627,16 +627,16 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD placeholder.hiddenCells.push(vm); } else { if (placeholder) { - newViewModel.push(placeholder); + newViewModels.push(placeholder); placeholder.hiddenCells[0].unchangedCells = placeholder.hiddenCells.length; } placeholder = undefined; - newViewModel.push(vm); + newViewModels.push(vm); } }); - this._list.splice(0, this._list.length, newViewModel); + this._list.splice(0, this._list.length, newViewModels); if (this.isOverviewRulerEnabled()) { - this._overviewRuler.updateViewModels(newViewModel, this._eventDispatcher); + this._overviewRuler.updateViewModels(newViewModels, this._eventDispatcher); } } From e6e053406e58527d4250a140cd4c60f81c4114d9 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 8 Aug 2024 11:03:08 +1000 Subject: [PATCH 05/11] Notebook Diff ViewModel --- .../notebook/browser/diff/diffComponents.ts | 2 +- .../browser/diff/diffElementViewModel.ts | 2 +- .../browser/diff/notebookDiffEditor.ts | 74 ++++++------------ .../browser/diff/notebookDiffEditorBrowser.ts | 13 ++++ .../browser/diff/notebookDiffOverviewRuler.ts | 4 +- .../browser/diff/notebookDiffViewModel.ts | 76 +++++++++++++++++++ 6 files changed, 117 insertions(+), 54 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts index 92d11a3bd8758..6122b0fefb8ad 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts @@ -1390,7 +1390,7 @@ export class ModifiedElement extends AbstractElementRenderer { override buildBody(): void { super.buildBody(); - if (this.cell.unchangedCells) { + if (this.cell.displayIconToHideUnmodifiedCells) { this._register(this.templateData.marginOverlay.onAction(() => this.cell.hideUnchangedCells())); this.templateData.marginOverlay.show(); } else { diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts index e7da6e38a31e2..d45d3d0347b74 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts @@ -97,7 +97,7 @@ export abstract class DiffElementCellViewModelBase extends DiffElementViewModelB onDidStateChange = this._stateChangeEmitter.event; protected _layoutInfo!: IDiffElementLayoutInfo; - public unchangedCells?: number; + public displayIconToHideUnmodifiedCells?: boolean; private _hideUnchangedCells = this._register(new Emitter()); public onHideUnchangedCells = this._hideUnchangedCells.event; diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index d2c97172ec0c0..3a8bd2311bb6f 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -14,7 +14,7 @@ import { getDefaultNotebookCreationOptions } from 'vs/workbench/contrib/notebook import { IEditorGroup } from 'vs/workbench/services/editor/common/editorGroupsService'; import { NotebookDiffEditorInput } from '../../common/notebookDiffEditorInput'; import { CancellationToken, CancellationTokenSource } from 'vs/base/common/cancellation'; -import { DiffElementCellViewModelBase, DiffElementPlaceholderViewModel, IDiffElementViewModelBase, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { DiffElementCellViewModelBase, IDiffElementViewModelBase, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation'; import { CellDiffPlaceholderRenderer, CellDiffSideBySideRenderer, CellDiffSingleSideRenderer, NotebookCellTextDiffListDelegate, NotebookTextDiffList } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffList'; import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; @@ -25,7 +25,7 @@ import { IEditorOptions as ICodeEditorOptions } from 'vs/editor/common/config/ed import { BareFontInfo, FontInfo } from 'vs/editor/common/config/fontInfo'; import { PixelRatio } from 'vs/base/browser/pixelRatio'; import { CellEditState, ICellOutputViewModel, IDisplayOutputLayoutUpdateRequest, IGenericCellViewModel, IInsetRenderOutput, INotebookEditorCreationOptions, INotebookEditorOptions } from 'vs/workbench/contrib/notebook/browser/notebookBrowser'; -import { DiffSide, DIFF_CELL_MARGIN, IDiffCellInfo, INotebookTextDiffEditor } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; +import { DiffSide, DIFF_CELL_MARGIN, IDiffCellInfo, INotebookTextDiffEditor, INotebookDiffViewModel } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; import { Emitter, Event } from 'vs/base/common/event'; import { DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle'; import { EditorPane } from 'vs/workbench/browser/parts/editor/editorPane'; @@ -46,6 +46,7 @@ import { IEditorOptions } from 'vs/platform/editor/common/editor'; import { cellIndexesToRanges, cellRangesToIndexes } from 'vs/workbench/contrib/notebook/common/notebookRange'; import { NotebookDiffOverviewRuler } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffOverviewRuler'; import { registerZIndex, ZIndex } from 'vs/platform/layout/browser/zIndexRegistry'; +import { NotebookDiffViewModel } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel'; const $ = DOM.$; @@ -94,7 +95,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD private _overviewRulerContainer!: HTMLElement; private _overviewRuler!: NotebookDiffOverviewRuler; private _dimension: DOM.Dimension | null = null; - private _diffElementViewModels: IDiffElementViewModelBase[] = []; + private _diffElementViewModels: INotebookDiffViewModel = this._register(new NotebookDiffViewModel()); private _list!: NotebookTextDiffList; private _modifiedWebview: BackLayerWebView | null = null; private _originalWebview: BackLayerWebView | null = null; @@ -332,6 +333,12 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD ); this._register(this._list); + this._register(this._diffElementViewModels.onDidChangeItems(e => { + this._list.splice(e.start, e.deleteCount, e.elements); + if (this.isOverviewRulerEnabled()) { + this._overviewRuler.updateViewModels(this._diffElementViewModels.items, this._eventDispatcher); + } + })); this._register(this._list.onMouseUp(e => { if (e.element) { @@ -565,7 +572,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD if (!isSame) { this._originalWebview?.removeInsets([...this._originalWebview?.insetMapping.keys()]); this._modifiedWebview?.removeInsets([...this._modifiedWebview?.insetMapping.keys()]); - this._setViewModel(viewModels); + this._diffElementViewModels.setViewModel(viewModels); } if (this._revealFirst && firstChangeIndex !== -1 && firstChangeIndex < this._list.length) { @@ -581,9 +588,10 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD private _isViewModelTheSame(viewModels: IDiffElementViewModelBase[]) { let isSame = true; - if (this._diffElementViewModels.length === viewModels.length) { + const currentViewModels = this._diffElementViewModels.items; + if (currentViewModels.length === viewModels.length) { for (let i = 0; i < viewModels.length; i++) { - const a = this._diffElementViewModels[i]; + const a = currentViewModels[i]; const b = viewModels[i]; if (a.type === 'placeholder' || b.type === 'placeholder') { if (a.type !== b.type) { @@ -604,41 +612,6 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD return isSame; } - private readonly viewModelStore = this._register(new DisposableStore()); - private _setViewModel(viewModels: DiffElementCellViewModelBase[]) { - this._diffElementViewModels = viewModels; - this.viewModelStore.clear(); - - const newViewModels: IDiffElementViewModelBase[] = []; - let placeholder: DiffElementPlaceholderViewModel | undefined = undefined; - viewModels.forEach((vm, index) => { - if (vm.type === 'unchanged') { - if (!placeholder) { - placeholder = new DiffElementPlaceholderViewModel(vm.mainDocumentTextModel, vm.editorEventDispatcher, vm.initData); - const placeholderItem = placeholder; - this.viewModelStore.add(placeholderItem.onUnfoldHiddenCells(() => { - this._list.splice(index, 1, placeholderItem.hiddenCells); - })); - vm.unchangedCells = 1; - this._register(vm.onHideUnchangedCells(() => { - this._list.splice(index, placeholderItem.hiddenCells.length, [placeholderItem]); - })); - } - placeholder.hiddenCells.push(vm); - } else { - if (placeholder) { - newViewModels.push(placeholder); - placeholder.hiddenCells[0].unchangedCells = placeholder.hiddenCells.length; - } - placeholder = undefined; - newViewModels.push(vm); - } - }); - this._list.splice(0, this._list.length, newViewModels); - if (this.isOverviewRulerEnabled()) { - this._overviewRuler.updateViewModels(newViewModels, this._eventDispatcher); - } - } /** * making sure that swapping cells are always translated to `insert+delete`. @@ -874,8 +847,9 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD // find the index of previous change let prevChangeIndex = currFocus - 1; + const currentViewModels = this._diffElementViewModels.items; while (prevChangeIndex >= 0) { - const vm = this._diffElementViewModels[prevChangeIndex]; + const vm = currentViewModels[prevChangeIndex]; if (vm.type !== 'unchanged') { break; } @@ -888,7 +862,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD this._list.reveal(prevChangeIndex); } else { // go to the last one - const index = findLastIdx(this._diffElementViewModels, vm => vm.type !== 'unchanged'); + const index = findLastIdx(currentViewModels, vm => vm.type !== 'unchanged'); if (index >= 0) { this._list.setFocus([index]); this._list.reveal(index); @@ -905,8 +879,9 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD // find the index of next change let nextChangeIndex = currFocus + 1; - while (nextChangeIndex < this._diffElementViewModels.length) { - const vm = this._diffElementViewModels[nextChangeIndex]; + const currentViewModels = this._diffElementViewModels.items; + while (nextChangeIndex < currentViewModels.length) { + const vm = currentViewModels[nextChangeIndex]; if (vm.type !== 'unchanged') { break; } @@ -914,12 +889,12 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD nextChangeIndex++; } - if (nextChangeIndex < this._diffElementViewModels.length) { + if (nextChangeIndex < currentViewModels.length) { this._list.setFocus([nextChangeIndex]); this._list.reveal(nextChangeIndex); } else { // go to the first one - const index = this._diffElementViewModels.findIndex(vm => vm.type !== 'unchanged'); + const index = currentViewModels.findIndex(vm => vm.type !== 'unchanged'); if (index >= 0) { this._list.setFocus([index]); this._list.reveal(index); @@ -1032,8 +1007,7 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD this._modifiedResourceDisposableStore.clear(); this._list?.splice(0, this._list?.length || 0); this._model = null; - this._diffElementViewModels.forEach(vm => vm.dispose()); - this._diffElementViewModels = []; + this._diffElementViewModels.clear(); } deltaCellOutputContainerClassNames(diffSide: DiffSide, cellId: string, added: string[], removed: string[]) { @@ -1118,5 +1092,5 @@ registerThemingParticipant((theme, collector) => { collector.addRule(`.notebook-text-diff-editor .cell-body { margin: ${DIFF_CELL_MARGIN}px; }`); // We do not want a left margin, as we add an overlay for expanind the collapsed/hidden cells. - collector.addRule(`.notebook-text-diff-editor .cell-placeholder-body { margin: ${DIFF_CELL_MARGIN}px; margin-left:0px }`); + collector.addRule(`.notebook-text-diff-editor .cell-placeholder-body { margin: ${DIFF_CELL_MARGIN}px 0; }`); }); diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts index 4fda303c2f158..9e7f84bce99ce 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts @@ -142,3 +142,16 @@ export const DIFF_CELL_MARGIN = 16; export const NOTEBOOK_DIFF_CELL_INPUT = new RawContextKey('notebookDiffCellInputChanged', false); export const NOTEBOOK_DIFF_CELL_PROPERTY = new RawContextKey('notebookDiffCellPropertyChanged', false); export const NOTEBOOK_DIFF_CELL_PROPERTY_EXPANDED = new RawContextKey('notebookDiffCellPropertyExpanded', false); + +export interface INotebookDiffViewModelUpdateEvent { + readonly start: number; + readonly deleteCount: number; + readonly elements: readonly IDiffElementViewModelBase[]; +} + +export interface INotebookDiffViewModel { + readonly items: readonly IDiffElementViewModelBase[]; + onDidChangeItems: Event; + setViewModel(cellViewModels: DiffElementCellViewModelBase[]): void; + clear(): void; +} diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffOverviewRuler.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffOverviewRuler.ts index 90390e19fa537..a63de3c7cbdea 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffOverviewRuler.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffOverviewRuler.ts @@ -20,7 +20,7 @@ export class NotebookDiffOverviewRuler extends Themable { private readonly _domNode: FastDomNode; private readonly _overviewViewportDomElement: FastDomNode; - private _diffElementViewModels: IDiffElementViewModelBase[] = []; + private _diffElementViewModels: readonly IDiffElementViewModelBase[] = []; private _lanes = 2; private _insertColor: Color | null; @@ -94,7 +94,7 @@ export class NotebookDiffOverviewRuler extends Themable { this._layoutNow(); } - updateViewModels(elements: IDiffElementViewModelBase[], eventDispatcher: NotebookDiffEditorEventDispatcher | undefined) { + updateViewModels(elements: readonly IDiffElementViewModelBase[], eventDispatcher: NotebookDiffEditorEventDispatcher | undefined) { this._disposables.clear(); this._diffElementViewModels = elements; diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts new file mode 100644 index 0000000000000..ea49b3577dda4 --- /dev/null +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts @@ -0,0 +1,76 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import { Emitter } from 'vs/base/common/event'; +import { Disposable, DisposableStore, dispose } from 'vs/base/common/lifecycle'; +import { DiffElementCellViewModelBase, DiffElementPlaceholderViewModel, IDiffElementViewModelBase } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { INotebookDiffViewModel, INotebookDiffViewModelUpdateEvent } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; + +export class NotebookDiffViewModel extends Disposable implements INotebookDiffViewModel { + private readonly placeholderAndRelatedCells = new Map(); + private readonly _items: IDiffElementViewModelBase[] = []; + get items(): readonly IDiffElementViewModelBase[] { + return this._items; + } + private readonly _onDidChangeItems = this._register(new Emitter()); + public readonly onDidChangeItems = this._onDidChangeItems.event; + private readonly disposables = this._register(new DisposableStore()); + + override dispose() { + this.clear(); + super.dispose(); + } + clear() { + dispose(Array.from(this.placeholderAndRelatedCells.values()).flat()); + dispose(Array.from(this.placeholderAndRelatedCells.keys())); + this.placeholderAndRelatedCells.clear(); + dispose(this._items); + this._items.splice(0, this._items.length); + } + + setViewModel(cellViewModels: DiffElementCellViewModelBase[]) { + this.disposables.clear(); + const newViewModels: IDiffElementViewModelBase[] = []; + let placeholder: DiffElementPlaceholderViewModel | undefined = undefined; + cellViewModels.forEach((vm, index) => { + if (vm.type === 'unchanged') { + if (!placeholder) { + vm.displayIconToHideUnmodifiedCells = true; + placeholder = new DiffElementPlaceholderViewModel(vm.mainDocumentTextModel, vm.editorEventDispatcher, vm.initData); + newViewModels.push(placeholder); + const placeholderItem = placeholder; + + this.disposables.add(placeholderItem.onUnfoldHiddenCells(() => { + const hiddenCellViewModels = this.placeholderAndRelatedCells.get(placeholderItem); + if (!Array.isArray(hiddenCellViewModels)) { + return; + } + newViewModels.splice(index, 1, ...hiddenCellViewModels); + this._onDidChangeItems.fire({ start: index, deleteCount: 1, elements: hiddenCellViewModels }); + })); + this.disposables.add(vm.onHideUnchangedCells(() => { + const hiddenCellViewModels = this.placeholderAndRelatedCells.get(placeholderItem); + if (!Array.isArray(hiddenCellViewModels)) { + return; + } + newViewModels.splice(index, hiddenCellViewModels.length, placeholderItem); + this._onDidChangeItems.fire({ start: index, deleteCount: hiddenCellViewModels.length, elements: [placeholderItem] }); + })); + } + const hiddenCellViewModels = this.placeholderAndRelatedCells.get(placeholder) || []; + hiddenCellViewModels.push(vm); + this.placeholderAndRelatedCells.set(placeholder, hiddenCellViewModels); + placeholder.hiddenCells.push(vm); + } else { + placeholder = undefined; + newViewModels.push(vm); + } + }); + + const oldLength = this._items.length; + this._items.splice(0, oldLength, ...newViewModels); + this._onDidChangeItems.fire({ start: 0, deleteCount: oldLength, elements: newViewModels }); + } +} From b8ce3187292dae90a731632dabe67e1e00987da9 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Thu, 8 Aug 2024 12:44:39 +1000 Subject: [PATCH 06/11] Add tests --- .../browser/diff/notebookDiffViewModel.ts | 16 +- .../browser/notebookDiffViewModel.test.ts | 236 ++++++++++++++++++ 2 files changed, 244 insertions(+), 8 deletions(-) create mode 100644 src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts index ea49b3577dda4..ad7e3153804dd 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts @@ -32,14 +32,16 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi setViewModel(cellViewModels: DiffElementCellViewModelBase[]) { this.disposables.clear(); - const newViewModels: IDiffElementViewModelBase[] = []; + const oldLength = this._items.length; + this._items.splice(0, oldLength); + let placeholder: DiffElementPlaceholderViewModel | undefined = undefined; cellViewModels.forEach((vm, index) => { if (vm.type === 'unchanged') { if (!placeholder) { vm.displayIconToHideUnmodifiedCells = true; placeholder = new DiffElementPlaceholderViewModel(vm.mainDocumentTextModel, vm.editorEventDispatcher, vm.initData); - newViewModels.push(placeholder); + this._items.push(placeholder); const placeholderItem = placeholder; this.disposables.add(placeholderItem.onUnfoldHiddenCells(() => { @@ -47,7 +49,7 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi if (!Array.isArray(hiddenCellViewModels)) { return; } - newViewModels.splice(index, 1, ...hiddenCellViewModels); + this._items.splice(index, 1, ...hiddenCellViewModels); this._onDidChangeItems.fire({ start: index, deleteCount: 1, elements: hiddenCellViewModels }); })); this.disposables.add(vm.onHideUnchangedCells(() => { @@ -55,7 +57,7 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi if (!Array.isArray(hiddenCellViewModels)) { return; } - newViewModels.splice(index, hiddenCellViewModels.length, placeholderItem); + this._items.splice(index, hiddenCellViewModels.length, placeholderItem); this._onDidChangeItems.fire({ start: index, deleteCount: hiddenCellViewModels.length, elements: [placeholderItem] }); })); } @@ -65,12 +67,10 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi placeholder.hiddenCells.push(vm); } else { placeholder = undefined; - newViewModels.push(vm); + this._items.push(vm); } }); - const oldLength = this._items.length; - this._items.splice(0, oldLength, ...newViewModels); - this._onDidChangeItems.fire({ start: 0, deleteCount: oldLength, elements: newViewModels }); + this._onDidChangeItems.fire({ start: 0, deleteCount: oldLength, elements: this._items }); } } diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts new file mode 100644 index 0000000000000..8f46d980c3ffe --- /dev/null +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts @@ -0,0 +1,236 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. See License.txt in the project root for license information. + *--------------------------------------------------------------------------------------------*/ + +import assert from 'assert'; +import * as sinon from 'sinon'; +import { CellKind } from 'vs/workbench/contrib/notebook/common/notebookCommon'; +import { setupInstantiationService, withTestNotebook } from 'vs/workbench/contrib/notebook/test/browser/testNotebookEditor'; +import { DisposableStore } from 'vs/base/common/lifecycle'; +import { TestInstantiationService } from 'vs/platform/instantiation/test/common/instantiationServiceMock'; +import { ensureNoDisposablesAreLeakedInTestSuite } from 'vs/base/test/common/utils'; +import { DiffElementPlaceholderViewModel, SideBySideDiffElementViewModel, SingleSideDiffElementViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffElementViewModel'; +import { DiffNestedCellViewModel } from 'vs/workbench/contrib/notebook/browser/diff/diffNestedCellViewModel'; +import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel'; +import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService'; +import { mock } from 'vs/base/test/common/mock'; +import { Event } from 'vs/base/common/event'; +import { NotebookDiffEditorEventDispatcher } from 'vs/workbench/contrib/notebook/browser/diff/eventDispatcher'; +import { NotebookDiffViewModel } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel'; +import { INotebookDiffViewModelUpdateEvent } from 'vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser'; + +suite('Notebook Diff ViewModel', () => { + let disposables: DisposableStore; + let instantiationService: TestInstantiationService; + const initData = { fontInfo: undefined, metadataStatusHeight: 0, outputStatusHeight: 0 }; + teardown(() => disposables.dispose()); + + ensureNoDisposablesAreLeakedInTestSuite(); + + setup(() => { + disposables = new DisposableStore(); + disposables.add({ dispose: () => sinon.restore() }); + instantiationService = setupInstantiationService(disposables); + instantiationService.stub(INotebookService, new class extends mock() { + override onDidAddNotebookDocument = Event.None; + override onWillRemoveNotebookDocument = Event.None; + override getNotebookTextModels() { return []; } + override getOutputMimeTypeInfo() { return []; } + }); + + }); + + + test('Cells are returned as they are', async function () { + await withTestNotebook( + [ + ['# header 1', 'markdown', CellKind.Markup, [], {}], + ], + (editor, viewModel, ds) => { + const cell1 = ds.add(new SingleSideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + undefined, + 'delete', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const cell2 = ds.add(new SingleSideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + undefined, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'insert', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const diffViewModel = ds.add(new NotebookDiffViewModel()); + diffViewModel.setViewModel([cell1, cell2]); + + assert.deepStrictEqual(diffViewModel.items, [cell1, cell2]); + } + ); + }); + + test('Trigger change event when viewmodel is updated', async function () { + await withTestNotebook( + [ + ['# header 1', 'markdown', CellKind.Markup, [], {}], + ], + (editor, viewModel, ds) => { + const cell1 = ds.add(new SingleSideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + undefined, + 'delete', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const cell2 = ds.add(new SingleSideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + undefined, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'insert', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const diffViewModel = ds.add(new NotebookDiffViewModel()); + diffViewModel.setViewModel([cell1]); + + assert.deepStrictEqual(diffViewModel.items, [cell1]); + + let eventArgs: INotebookDiffViewModelUpdateEvent | undefined = undefined; + Event.once(diffViewModel.onDidChangeItems)(e => eventArgs = e); + + diffViewModel.setViewModel([cell2]); + + assert.deepStrictEqual(diffViewModel.items, [cell2]); + assert.deepStrictEqual(eventArgs, { start: 0, deleteCount: 1, elements: [cell2] }); + + eventArgs = undefined; + Event.once(diffViewModel.onDidChangeItems)(e => eventArgs = e); + + diffViewModel.setViewModel([]); + + assert.deepStrictEqual(diffViewModel.items, []); + assert.deepStrictEqual(eventArgs, { start: 0, deleteCount: 1, elements: [] }); + } + ); + }); + + test('Unmodified cells should be replace with placeholders', async function () { + await withTestNotebook( + [ + ['# header 1', 'markdown', CellKind.Markup, [], {}], + ], + (editor, viewModel, ds) => { + const deleted = ds.add(new SingleSideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + undefined, + 'delete', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const inserted = ds.add(new SingleSideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + undefined, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'insert', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified1 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified2 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified3 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified4 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const diffViewModel = ds.add(new NotebookDiffViewModel()); + diffViewModel.setViewModel([deleted, unmodified1, inserted, unmodified2, unmodified3, unmodified4]); + + // Default state + assert.strictEqual(diffViewModel.items.length, 4); + assert.deepStrictEqual(diffViewModel.items[0], deleted); + assert.deepStrictEqual(diffViewModel.items[2], inserted); + assert.ok(diffViewModel.items[1] instanceof DiffElementPlaceholderViewModel); + assert.ok(diffViewModel.items[3] instanceof DiffElementPlaceholderViewModel); + + // Expand first collapsed section. + let eventArgs: INotebookDiffViewModelUpdateEvent | undefined = undefined; + Event.once(diffViewModel.onDidChangeItems)(e => eventArgs = e); + diffViewModel.items[1].showHiddenCells(); + + assert.deepStrictEqual(diffViewModel.items[0], deleted); + assert.deepStrictEqual(diffViewModel.items[1], unmodified1); + assert.deepStrictEqual(diffViewModel.items[2], inserted); + assert.ok(diffViewModel.items[3] instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(eventArgs, { start: 1, deleteCount: 1, elements: [unmodified1] }); + + // Expand last collapsed section. + Event.once(diffViewModel.onDidChangeItems)(e => eventArgs = e); + diffViewModel.items[3].showHiddenCells(); + + assert.deepStrictEqual(diffViewModel.items, [deleted, unmodified1, inserted, unmodified2, unmodified3, unmodified4]); + + // Collapse the second cell + (diffViewModel.items[1] as SideBySideDiffElementViewModel).hideUnchangedCells(); + + // Verify we collpased second cell + assert.deepStrictEqual(diffViewModel.items[0], deleted); + assert.deepStrictEqual(diffViewModel.items[2], inserted); + assert.ok((diffViewModel.items[1] as any) instanceof DiffElementPlaceholderViewModel); + + // Collapse the 4th cell + (diffViewModel.items[3] as SideBySideDiffElementViewModel).hideUnchangedCells(); + + // Verify we collpased second cell + assert.strictEqual(diffViewModel.items.length, 4); + assert.deepStrictEqual(diffViewModel.items[0], deleted); + assert.deepStrictEqual(diffViewModel.items[2], inserted); + assert.ok((diffViewModel.items[1] as any) instanceof DiffElementPlaceholderViewModel); + assert.ok((diffViewModel.items[3] as any) instanceof DiffElementPlaceholderViewModel); + } + ); + }); + + function createDiffNestedViewModel(cellTextModel: NotebookCellTextModel) { + return new DiffNestedCellViewModel(cellTextModel, instantiationService.get(INotebookService)); + } +}); From 8b1b0fcdfb10400d287cbb6e1f8af375bf64fc59 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 9 Aug 2024 06:57:08 +1000 Subject: [PATCH 07/11] Fix issue with collapse/expand --- .../browser/diff/notebookDiffViewModel.ts | 10 +- .../browser/notebookDiffViewModel.test.ts | 168 ++++++++++++++++++ 2 files changed, 174 insertions(+), 4 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts index ad7e3153804dd..96229ca21f281 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts @@ -49,16 +49,18 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi if (!Array.isArray(hiddenCellViewModels)) { return; } - this._items.splice(index, 1, ...hiddenCellViewModels); - this._onDidChangeItems.fire({ start: index, deleteCount: 1, elements: hiddenCellViewModels }); + const start = this._items.indexOf(placeholderItem); + this._items.splice(start, 1, ...hiddenCellViewModels); + this._onDidChangeItems.fire({ start, deleteCount: 1, elements: hiddenCellViewModels }); })); this.disposables.add(vm.onHideUnchangedCells(() => { const hiddenCellViewModels = this.placeholderAndRelatedCells.get(placeholderItem); if (!Array.isArray(hiddenCellViewModels)) { return; } - this._items.splice(index, hiddenCellViewModels.length, placeholderItem); - this._onDidChangeItems.fire({ start: index, deleteCount: hiddenCellViewModels.length, elements: [placeholderItem] }); + const start = this._items.indexOf(vm); + this._items.splice(start, hiddenCellViewModels.length, placeholderItem); + this._onDidChangeItems.fire({ start, deleteCount: hiddenCellViewModels.length, elements: [placeholderItem] }); })); } const hiddenCellViewModels = this.placeholderAndRelatedCells.get(placeholder) || []; diff --git a/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts b/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts index 8f46d980c3ffe..378bd40cb79b4 100644 --- a/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts +++ b/src/vs/workbench/contrib/notebook/test/browser/notebookDiffViewModel.test.ts @@ -230,6 +230,174 @@ suite('Notebook Diff ViewModel', () => { ); }); + + test('Ensure placeholder positions are as expected', async function () { + await withTestNotebook( + [ + ['# header 1', 'markdown', CellKind.Markup, [], {}], + ['# header 2', 'markdown', CellKind.Markup, [], {}], + ], + (editor, viewModel, ds) => { + const unmodified1 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified2 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const modified1 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(1)!.model)), + 'modified', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const modified2 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(1)!.model)), + 'modified', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified3 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified4 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified5 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified6 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const modified3 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(1)!.model)), + 'modified', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const unmodified7 = ds.add(new SideBySideDiffElementViewModel( + viewModel.notebookDocument, + viewModel.notebookDocument, + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + ds.add(createDiffNestedViewModel(viewModel.cellAt(0)!.model)), + 'unchanged', + ds.add(new NotebookDiffEditorEventDispatcher()), + initData)); + + const diffViewModel = ds.add(new NotebookDiffViewModel()); + diffViewModel.setViewModel([unmodified1, unmodified2, modified1, modified2, unmodified3, unmodified4, unmodified5, unmodified6, modified3, unmodified7]); + + // Default state + assert.strictEqual(diffViewModel.items.length, 6); + assert.ok(diffViewModel.items[0] instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[1], modified1); + assert.deepStrictEqual(diffViewModel.items[2], modified2); + assert.ok(diffViewModel.items[3] instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[4], modified3); + assert.ok(diffViewModel.items[5] instanceof DiffElementPlaceholderViewModel); + + // Expand first collapsed section. + let eventArgs: INotebookDiffViewModelUpdateEvent | undefined = undefined; + ds.add(diffViewModel.onDidChangeItems(e => eventArgs = e)); + diffViewModel.items[0].showHiddenCells(); + + assert.strictEqual(diffViewModel.items.length, 7); + assert.deepStrictEqual(diffViewModel.items[0], unmodified1); + assert.deepStrictEqual(diffViewModel.items[1], unmodified2); + assert.deepStrictEqual(diffViewModel.items[2], modified1); + assert.deepStrictEqual(diffViewModel.items[3], modified2); + assert.ok(diffViewModel.items[4] instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[5], modified3); + assert.ok(diffViewModel.items[6] instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(eventArgs, { start: 0, deleteCount: 1, elements: [unmodified1, unmodified2] }); + + // Collapse the 1st two cells + (diffViewModel.items[0] as SideBySideDiffElementViewModel).hideUnchangedCells(); + + assert.strictEqual(diffViewModel.items.length, 6); + assert.ok((diffViewModel.items[0] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[1], modified1); + assert.deepStrictEqual(diffViewModel.items[2], modified2); + assert.ok((diffViewModel.items[3] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[4], modified3); + assert.ok((diffViewModel.items[5] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(eventArgs, { start: 0, deleteCount: 2, elements: [diffViewModel.items[0]] }); + + + // Expand second collapsed section. + (diffViewModel.items[3] as any).showHiddenCells(); + + assert.strictEqual(diffViewModel.items.length, 9); + assert.ok((diffViewModel.items[0] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[1], modified1); + assert.deepStrictEqual(diffViewModel.items[2], modified2); + assert.deepStrictEqual(diffViewModel.items[3], unmodified3); + assert.deepStrictEqual(diffViewModel.items[4], unmodified4); + assert.deepStrictEqual(diffViewModel.items[5], unmodified5); + assert.deepStrictEqual(diffViewModel.items[6], unmodified6); + assert.deepStrictEqual(diffViewModel.items[7], modified3); + assert.ok((diffViewModel.items[8] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(eventArgs, { start: 3, deleteCount: 1, elements: [unmodified3, unmodified4, unmodified5, unmodified6] }); + + // Collapse the 2nd section + (diffViewModel.items[3] as SideBySideDiffElementViewModel).hideUnchangedCells(); + + assert.strictEqual(diffViewModel.items.length, 6); + assert.ok((diffViewModel.items[0] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[1], modified1); + assert.deepStrictEqual(diffViewModel.items[2], modified2); + assert.ok((diffViewModel.items[3] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(diffViewModel.items[4], modified3); + assert.ok((diffViewModel.items[5] as any) instanceof DiffElementPlaceholderViewModel); + assert.deepStrictEqual(eventArgs, { start: 3, deleteCount: 4, elements: [diffViewModel.items[3]] }); + } + ); + }); + function createDiffNestedViewModel(cellTextModel: NotebookCellTextModel) { return new DiffNestedCellViewModel(cellTextModel, instantiationService.get(INotebookService)); } From 7e87eda1b4e70a8916451d1e1a3205cf65a9729f Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 9 Aug 2024 08:10:19 +1000 Subject: [PATCH 08/11] Fix margin --- .../contrib/notebook/browser/diff/diffElementViewModel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts index d45d3d0347b74..1e7d1fe38a7be 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffElementViewModel.ts @@ -76,7 +76,7 @@ export class DiffElementPlaceholderViewModel extends DiffElementViewModelBase { } get totalHeight() { - return 50; + return 24 + (2 * DIFF_CELL_MARGIN); } getHeight(_: number): number { return this.totalHeight; From 9f18d303992842af699321e11b3416d2c2cb35a7 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 9 Aug 2024 08:12:11 +1000 Subject: [PATCH 09/11] Fix issue with sizing of metadata editor --- .../contrib/notebook/browser/diff/diffComponents.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts index 6122b0fefb8ad..16a6c5f4af9f5 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts @@ -1794,7 +1794,10 @@ export class ModifiedElement extends AbstractElementRenderer { if (state.metadataHeight || state.outerWidth) { if (this._metadataEditorContainer) { this._metadataEditorContainer.style.height = `${this.cell.layoutInfo.metadataHeight}px`; - this._metadataEditor?.layout(); + this._metadataEditor?.layout({ + width: this._editor!.getViewWidth(), + height: this.cell.layoutInfo.metadataHeight + }); } } From d0a92afb2a3636b06be076833ea8c5ad4ef2874e Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 9 Aug 2024 09:56:27 +1000 Subject: [PATCH 10/11] Fixes --- .../workbench/contrib/notebook/browser/diff/diffComponents.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts index 16a6c5f4af9f5..b09a2d8926f8e 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/diffComponents.ts @@ -1795,7 +1795,7 @@ export class ModifiedElement extends AbstractElementRenderer { if (this._metadataEditorContainer) { this._metadataEditorContainer.style.height = `${this.cell.layoutInfo.metadataHeight}px`; this._metadataEditor?.layout({ - width: this._editor!.getViewWidth(), + width: this._editor?.getViewWidth() || this.cell.getComputedCellContainerWidth(this.notebookEditor.getLayoutInfo(), false, true), height: this.cell.layoutInfo.metadataHeight }); } From 81e2e6bf5b06f225cf7af291c23ea6a6407f4812 Mon Sep 17 00:00:00 2001 From: Don Jayamanne Date: Fri, 9 Aug 2024 10:12:01 +1000 Subject: [PATCH 11/11] Fix flickering of diff editor --- .../browser/diff/notebookDiffEditor.ts | 30 +------------------ .../browser/diff/notebookDiffEditorBrowser.ts | 1 + .../browser/diff/notebookDiffViewModel.ts | 20 ++++++++++++- 3 files changed, 21 insertions(+), 30 deletions(-) diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts index 3a8bd2311bb6f..3590f16e938a3 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditor.ts @@ -567,9 +567,8 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD NotebookTextDiffEditor.prettyChanges(this._model, diffResult.cellsDiff); const { viewModels, firstChangeIndex } = NotebookTextDiffEditor.computeDiff(this.instantiationService, this.configurationService, this._model, this._eventDispatcher!, diffResult, this.fontInfo); - const isSame = this._isViewModelTheSame(viewModels); - if (!isSame) { + if (!this._diffElementViewModels.isEqual(viewModels)) { this._originalWebview?.removeInsets([...this._originalWebview?.insetMapping.keys()]); this._modifiedWebview?.removeInsets([...this._modifiedWebview?.insetMapping.keys()]); this._diffElementViewModels.setViewModel(viewModels); @@ -586,33 +585,6 @@ export class NotebookTextDiffEditor extends EditorPane implements INotebookTextD } } - private _isViewModelTheSame(viewModels: IDiffElementViewModelBase[]) { - let isSame = true; - const currentViewModels = this._diffElementViewModels.items; - if (currentViewModels.length === viewModels.length) { - for (let i = 0; i < viewModels.length; i++) { - const a = currentViewModels[i]; - const b = viewModels[i]; - if (a.type === 'placeholder' || b.type === 'placeholder') { - if (a.type !== b.type) { - isSame = false; - break; - } - } else if (a.original?.textModel.getHashValue() !== b.original?.textModel.getHashValue() - || a.modified?.textModel.getHashValue() !== b.modified?.textModel.getHashValue()) { - isSame = false; - break; - - } - } - } else { - isSame = false; - } - - return isSame; - } - - /** * making sure that swapping cells are always translated to `insert+delete`. */ diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts index 9e7f84bce99ce..105ce9b0677ff 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffEditorBrowser.ts @@ -152,6 +152,7 @@ export interface INotebookDiffViewModelUpdateEvent { export interface INotebookDiffViewModel { readonly items: readonly IDiffElementViewModelBase[]; onDidChangeItems: Event; + isEqual(viewModels: DiffElementCellViewModelBase[]): boolean; setViewModel(cellViewModels: DiffElementCellViewModelBase[]): void; clear(): void; } diff --git a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts index 96229ca21f281..f2ecb1b07c480 100644 --- a/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts +++ b/src/vs/workbench/contrib/notebook/browser/diff/notebookDiffViewModel.ts @@ -18,14 +18,16 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi public readonly onDidChangeItems = this._onDidChangeItems.event; private readonly disposables = this._register(new DisposableStore()); + private originalCellViewModels: DiffElementCellViewModelBase[] = []; override dispose() { this.clear(); super.dispose(); } clear() { - dispose(Array.from(this.placeholderAndRelatedCells.values()).flat()); dispose(Array.from(this.placeholderAndRelatedCells.keys())); this.placeholderAndRelatedCells.clear(); + dispose(this.originalCellViewModels); + this.originalCellViewModels = []; dispose(this._items); this._items.splice(0, this._items.length); } @@ -36,6 +38,7 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi this._items.splice(0, oldLength); let placeholder: DiffElementPlaceholderViewModel | undefined = undefined; + this.originalCellViewModels = cellViewModels; cellViewModels.forEach((vm, index) => { if (vm.type === 'unchanged') { if (!placeholder) { @@ -75,4 +78,19 @@ export class NotebookDiffViewModel extends Disposable implements INotebookDiffVi this._onDidChangeItems.fire({ start: 0, deleteCount: oldLength, elements: this._items }); } + public isEqual(viewModels: DiffElementCellViewModelBase[]) { + if (this.originalCellViewModels.length !== viewModels.length) { + return false; + } + for (let i = 0; i < viewModels.length; i++) { + const a = this.originalCellViewModels[i]; + const b = viewModels[i]; + if (a.original?.textModel.getHashValue() !== b.original?.textModel.getHashValue() + || a.modified?.textModel.getHashValue() !== b.modified?.textModel.getHashValue()) { + return false; + } + } + + return true; + } }