Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Small diff editor refactoring #192107

Merged
merged 1 commit into from
Sep 4, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*--------------------------------------------------------------------------------------------*/
import { Emitter } from 'vs/base/common/event';
import { Disposable } from 'vs/base/common/lifecycle';
import { IReader, autorunHandleChanges } from 'vs/base/common/observable';
import { IObservable, IReader, autorunHandleChanges, observableFromEvent } from 'vs/base/common/observable';
import { IEditorConstructionOptions } from 'vs/editor/browser/config/editorConfiguration';
import { IDiffEditorConstructionOptions } from 'vs/editor/browser/editorBrowser';
import { CodeEditorWidget, ICodeEditorWidgetOptions } from 'vs/editor/browser/widget/codeEditorWidget';
Expand All @@ -16,6 +16,7 @@ import { localize } from 'vs/nls';
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
import { DiffEditorOptions } from './diffEditorOptions';
import { ITextModel } from 'vs/editor/common/model';

export class DiffEditorEditors extends Disposable {
public readonly modified: CodeEditorWidget;
Expand All @@ -24,6 +25,8 @@ export class DiffEditorEditors extends Disposable {
private readonly _onDidContentSizeChange = this._register(new Emitter<IContentSizeChangedEvent>());
public get onDidContentSizeChange() { return this._onDidContentSizeChange.event; }

public readonly modifiedModel: IObservable<ITextModel | null>;

constructor(
private readonly originalEditorElement: HTMLElement,
private readonly modifiedEditorElement: HTMLElement,
Expand All @@ -38,6 +41,8 @@ export class DiffEditorEditors extends Disposable {
this.original = this._register(this._createLeftHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.originalEditor || {}));
this.modified = this._register(this._createRightHandSideEditor(_options.editorOptions.get(), codeEditorWidgetOptions.modifiedEditor || {}));

this.modifiedModel = observableFromEvent(this.modified.onDidChangeModel, () => this.modified.getModel());

this._register(autorunHandleChanges({
createEmptyChangeSummary: () => ({} as IDiffEditorConstructionOptions),
handleChange: (ctx, changeSummary) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ export class DiffEditorOptions {
public readonly hideUnchangedRegions = derived(reader => /** @description hideUnchangedRegions */ this._options.read(reader).hideUnchangedRegions.enabled!);
public readonly hideUnchangedRegionsRevealLineCount = derived(reader => /** @description hideUnchangedRegions */ this._options.read(reader).hideUnchangedRegions.revealLineCount!);
public readonly hideUnchangedRegionsContextLineCount = derived(reader => /** @description hideUnchangedRegions */ this._options.read(reader).hideUnchangedRegions.contextLineCount!);
public readonly hideUnchangedRegionsminimumLineCount = derived(reader => /** @description hideUnchangedRegions */ this._options.read(reader).hideUnchangedRegions.minimumLineCount!);
public readonly hideUnchangedRegionsMinimumLineCount = derived(reader => /** @description hideUnchangedRegions */ this._options.read(reader).hideUnchangedRegions.minimumLineCount!);

public updateOptions(changedOptions: IDiffEditorOptions): void {
const newDiffEditorOptions = validateDiffEditorOptions(changedOptions, this._options.get());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export class DiffEditorViewModel extends Disposable implements IDiffEditorViewMo
result.changes,
model.original.getLineCount(),
model.modified.getLineCount(),
this._options.hideUnchangedRegionsminimumLineCount.read(reader),
this._options.hideUnchangedRegionsMinimumLineCount.read(reader),
this._options.hideUnchangedRegionsContextLineCount.read(reader),
);

Expand All @@ -99,18 +99,18 @@ export class DiffEditorViewModel extends Disposable implements IDiffEditorViewMo

const originalDecorationIds = model.original.deltaDecorations(
lastUnchangedRegions.originalDecorationIds,
newUnchangedRegions.map(r => ({ range: r.originalRange.toInclusiveRange()!, options: { description: 'unchanged' } }))
newUnchangedRegions.map(r => ({ range: r.originalUnchangedRange.toInclusiveRange()!, options: { description: 'unchanged' } }))
);
const modifiedDecorationIds = model.modified.deltaDecorations(
lastUnchangedRegions.modifiedDecorationIds,
newUnchangedRegions.map(r => ({ range: r.modifiedRange.toInclusiveRange()!, options: { description: 'unchanged' } }))
newUnchangedRegions.map(r => ({ range: r.modifiedUnchangedRange.toInclusiveRange()!, options: { description: 'unchanged' } }))
);


for (const r of newUnchangedRegions) {
for (let i = 0; i < lastUnchangedRegions.regions.length; i++) {
if (r.originalRange.intersectsStrict(lastUnchangedRegionsOrigRanges[i])
&& r.modifiedRange.intersectsStrict(lastUnchangedRegionsModRanges[i])) {
if (r.originalUnchangedRange.intersectsStrict(lastUnchangedRegionsOrigRanges[i])
&& r.modifiedUnchangedRange.intersectsStrict(lastUnchangedRegionsModRanges[i])) {
r.setHiddenModifiedRange(lastUnchangedRegions.regions[i].getHiddenModifiedRange(undefined), tx);
break;
}
Expand Down Expand Up @@ -169,7 +169,7 @@ export class DiffEditorViewModel extends Disposable implements IDiffEditorViewMo
/** @description compute diff */

// So that they get recomputed when these settings change
this._options.hideUnchangedRegionsminimumLineCount.read(reader);
this._options.hideUnchangedRegionsMinimumLineCount.read(reader);
this._options.hideUnchangedRegionsContextLineCount.read(reader);

debouncer.cancel();
Expand Down Expand Up @@ -260,7 +260,7 @@ export class DiffEditorViewModel extends Disposable implements IDiffEditorViewMo
transaction(tx => {
for (const r of regions.regions) {
for (const range of ranges) {
if (r.modifiedRange.intersect(range)) {
if (r.modifiedUnchangedRange.intersect(range)) {
r.setHiddenModifiedRange(range, tx);
break;
}
Expand Down Expand Up @@ -357,11 +357,11 @@ export class UnchangedRegion {
return result;
}

public get originalRange(): LineRange {
public get originalUnchangedRange(): LineRange {
return LineRange.ofLength(this.originalLineNumber, this.lineCount);
}

public get modifiedRange(): LineRange {
public get modifiedUnchangedRange(): LineRange {
return LineRange.ofLength(this.modifiedLineNumber, this.lineCount);
}

Expand All @@ -371,7 +371,8 @@ export class UnchangedRegion {
private readonly _visibleLineCountBottom = observableValue<number>('visibleLineCountBottom', 0);
public readonly visibleLineCountBottom: ISettableObservable<number> = this._visibleLineCountBottom;

private readonly _shouldHideControls = derived(reader => /** @description isVisible */ this.visibleLineCountTop.read(reader) + this.visibleLineCountBottom.read(reader) === this.lineCount && !this.isDragged.read(reader));
private readonly _shouldHideControls = derived(reader => /** @description isVisible */
this.visibleLineCountTop.read(reader) + this.visibleLineCountBottom.read(reader) === this.lineCount && !this.isDragged.read(reader));

public readonly isDragged = observableValue<boolean>('isDragged', false);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { DiffEditorSash } from 'vs/editor/browser/widget/diffEditorWidget2/diffE
import { ViewZoneManager } from 'vs/editor/browser/widget/diffEditorWidget2/lineAlignment';
import { MovedBlocksLinesPart } from 'vs/editor/browser/widget/diffEditorWidget2/movedBlocksLines';
import { OverviewRulerPart } from 'vs/editor/browser/widget/diffEditorWidget2/overviewRulerPart';
import { UnchangedRangesFeature } from 'vs/editor/browser/widget/diffEditorWidget2/unchangedRanges';
import { HideUnchangedRegionsFeature } from 'vs/editor/browser/widget/diffEditorWidget2/hideUnchangedRegionsFeature';
import { CSSStyle, ObservableElementSizeObserver, applyStyle, readHotReloadableExport } from 'vs/editor/browser/widget/diffEditorWidget2/utils';
import { WorkerBasedDocumentDiffProvider } from 'vs/editor/browser/widget/workerBasedDocumentDiffProvider';
import { IDiffEditorOptions } from 'vs/editor/common/config/editorOptions';
Expand Down Expand Up @@ -68,7 +68,7 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
private readonly _sash: IObservable<DiffEditorSash | undefined>;
private readonly _boundarySashes = observableValue<IBoundarySashes | undefined>('boundarySashes', undefined);

private unchangedRangesFeature!: UnchangedRangesFeature;
private unchangedRangesFeature!: HideUnchangedRegionsFeature;

private _accessibleDiffViewerShouldBeVisible = observableValue('accessibleDiffViewerShouldBeVisible', false);
private _accessibleDiffViewerVisible = derived(reader =>
Expand Down Expand Up @@ -162,7 +162,7 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
this._register(autorunWithStore((reader, store) => {
/** @description UnchangedRangesFeature */
this.unchangedRangesFeature = store.add(
this._instantiationService.createInstance(readHotReloadableExport(UnchangedRangesFeature, reader), this._editors, this._diffModel, this._options)
this._instantiationService.createInstance(readHotReloadableExport(HideUnchangedRegionsFeature, reader), this._editors, this._diffModel, this._options)
);
}));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
import { $, addDisposableListener, h, reset } from 'vs/base/browser/dom';
import { renderIcon, renderLabelWithIcons } from 'vs/base/browser/ui/iconLabel/iconLabels';
import { compareBy, numberComparator, reverseOrder } from 'vs/base/common/arrays';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { Codicon } from 'vs/base/common/codicons';
import { Event } from 'vs/base/common/event';
import { MarkdownString } from 'vs/base/common/htmlContent';
Expand All @@ -19,7 +18,7 @@ import { DiffEditorEditors } from 'vs/editor/browser/widget/diffEditorWidget2/di
import { DiffEditorOptions } from 'vs/editor/browser/widget/diffEditorWidget2/diffEditorOptions';
import { DiffEditorViewModel, UnchangedRegion } from 'vs/editor/browser/widget/diffEditorWidget2/diffEditorViewModel';
import { OutlineModel } from 'vs/editor/browser/widget/diffEditorWidget2/outlineModel';
import { PlaceholderViewZone, ViewZoneOverlayWidget, applyObservableDecorations, applyStyle, applyViewZones } from 'vs/editor/browser/widget/diffEditorWidget2/utils';
import { DisposableCancellationTokenSource, PlaceholderViewZone, ViewZoneOverlayWidget, applyObservableDecorations, applyStyle, applyViewZones } from 'vs/editor/browser/widget/diffEditorWidget2/utils';
import { EditorOption } from 'vs/editor/common/config/editorOptions';
import { LineRange } from 'vs/editor/common/core/lineRange';
import { Position } from 'vs/editor/common/core/position';
Expand All @@ -30,14 +29,12 @@ import { IModelDecorationOptions, IModelDeltaDecoration, ITextModel } from 'vs/e
import { ILanguageFeaturesService } from 'vs/editor/common/services/languageFeatures';
import { localize } from 'vs/nls';

export class UnchangedRangesFeature extends Disposable {
export class HideUnchangedRegionsFeature extends Disposable {
private _isUpdatingViewZones = false;
public get isUpdatingViewZones(): boolean { return this._isUpdatingViewZones; }

private readonly _modifiedModel = observableFromEvent(this._editors.modified.onDidChangeModel, () => this._editors.modified.getModel());

private readonly _modifiedOutlineSource = derivedWithStore('modified outline source', (reader, store) => {
const m = this._modifiedModel.read(reader);
const m = this._editors.modifiedModel.read(reader);
if (!m) { return undefined; }
return store.add(new OutlineSource(this._languageFeaturesService, m));
});
Expand Down Expand Up @@ -77,13 +74,13 @@ export class UnchangedRangesFeature extends Disposable {
const unchangedRegions = this._diffModel.map((m, reader) => m?.diff.read(reader)?.mappings.length === 0 ? [] : m?.unchangedRegions.read(reader) ?? []);

const viewZones = derivedWithStore('view zones', (reader, store) => {
const modifiedOutlineSource = this._modifiedOutlineSource.read(reader);
if (!modifiedOutlineSource) { return { origViewZones: [], modViewZones: [] }; }

const origViewZones: IViewZone[] = [];
const modViewZones: IViewZone[] = [];
const sideBySide = this._options.renderSideBySide.read(reader);

const modifiedOutlineSource = this._modifiedOutlineSource.read(reader);
if (!modifiedOutlineSource) { return { origViewZones, modViewZones }; }

const curUnchangedRegions = unchangedRegions.read(reader);
for (const r of curUnchangedRegions) {
if (r.shouldHideControls(reader)) {
Expand All @@ -94,13 +91,30 @@ export class UnchangedRangesFeature extends Disposable {
const d = derived(reader => /** @description hiddenOriginalRangeStart */ r.getHiddenOriginalRange(reader).startLineNumber - 1);
const origVz = new PlaceholderViewZone(d, 24);
origViewZones.push(origVz);
store.add(new CollapsedCodeOverlayWidget(this._editors.original, origVz, r, r.originalRange, !sideBySide, modifiedOutlineSource, l => this._diffModel.get()!.ensureModifiedLineIsVisible(l, undefined), this._options));
store.add(new CollapsedCodeOverlayWidget(
this._editors.original,
origVz,
r,
r.originalUnchangedRange,
!sideBySide,
modifiedOutlineSource,
l => this._diffModel.get()!.ensureModifiedLineIsVisible(l, undefined),
this._options
));
}
{
const d = derived(reader => /** @description hiddenModifiedRangeStart */ r.getHiddenModifiedRange(reader).startLineNumber - 1);
const modViewZone = new PlaceholderViewZone(d, 24);
modViewZones.push(modViewZone);
store.add(new CollapsedCodeOverlayWidget(this._editors.modified, modViewZone, r, r.modifiedRange, false, modifiedOutlineSource, l => this._diffModel.get()!.ensureModifiedLineIsVisible(l, undefined), this._options));
store.add(new CollapsedCodeOverlayWidget(
this._editors.modified,
modViewZone,
r,
r.modifiedUnchangedRange,
false,
modifiedOutlineSource,
l => this._diffModel.get()!.ensureModifiedLineIsVisible(l, undefined), this._options
));
}
}

Expand All @@ -125,14 +139,14 @@ export class UnchangedRangesFeature extends Disposable {
/** @description decorations */
const curUnchangedRegions = unchangedRegions.read(reader);
const result = curUnchangedRegions.map<IModelDeltaDecoration>(r => ({
range: r.originalRange.toInclusiveRange()!,
range: r.originalUnchangedRange.toInclusiveRange()!,
options: unchangedLinesDecoration,
}));
for (const r of curUnchangedRegions) {
if (r.shouldHideControls(reader)) {
result.push({
range: Range.fromPositions(new Position(r.originalLineNumber, 1)),
options: unchangedLinesDecorationShow
options: unchangedLinesDecorationShow,
});
}
}
Expand All @@ -143,14 +157,14 @@ export class UnchangedRangesFeature extends Disposable {
/** @description decorations */
const curUnchangedRegions = unchangedRegions.read(reader);
const result = curUnchangedRegions.map<IModelDeltaDecoration>(r => ({
range: r.modifiedRange.toInclusiveRange()!,
range: r.modifiedUnchangedRange.toInclusiveRange()!,
options: unchangedLinesDecoration,
}));
for (const r of curUnchangedRegions) {
if (r.shouldHideControls(reader)) {
result.push({
range: LineRange.ofLength(r.modifiedLineNumber, 1).toInclusiveRange()!,
options: unchangedLinesDecorationShow
options: unchangedLinesDecorationShow,
});
}
}
Expand All @@ -172,7 +186,7 @@ export class UnchangedRangesFeature extends Disposable {
const lineNumber = event.target.position.lineNumber;
const model = this._diffModel.get();
if (!model) { return; }
const region = model.unchangedRegions.get().find(r => r.modifiedRange.includes(lineNumber));
const region = model.unchangedRegions.get().find(r => r.modifiedUnchangedRange.includes(lineNumber));
if (!region) { return; }
region.collapseAll(undefined);
event.event.stopPropagation();
Expand All @@ -185,7 +199,7 @@ export class UnchangedRangesFeature extends Disposable {
const lineNumber = event.target.position.lineNumber;
const model = this._diffModel.get();
if (!model) { return; }
const region = model.unchangedRegions.get().find(r => r.originalRange.includes(lineNumber));
const region = model.unchangedRegions.get().find(r => r.originalUnchangedRange.includes(lineNumber));
if (!region) { return; }
region.collapseAll(undefined);
event.event.stopPropagation();
Expand All @@ -195,12 +209,6 @@ export class UnchangedRangesFeature extends Disposable {
}
}

class DisposableCancellationTokenSource extends CancellationTokenSource {
public override dispose() {
super.dispose(true);
}
}

class OutlineSource extends Disposable {
private readonly _currentModel = observableValue<OutlineModel | undefined>('current model', undefined);

Expand Down Expand Up @@ -251,7 +259,8 @@ class CollapsedCodeOverlayWidget extends ViewZoneOverlayWidget {
h('div.top@top', { title: localize('diff.hiddenLines.top', 'Click or drag to show more above') }),
h('div.center@content', { style: { display: 'flex' } }, [
h('div@first', { style: { display: 'flex', justifyContent: 'center', alignItems: 'center', flexShrink: '0' } },
[$('a', { title: localize('showAll', 'Show all'), role: 'button', onclick: () => { this.showAll(); } }, ...renderLabelWithIcons('$(unfold)'))]
[$('a', { title: localize('showAll', 'Show all'), role: 'button', onclick: () => { this._unchangedRegion.showAll(undefined); } },
...renderLabelWithIcons('$(unfold)'))]
),
h('div@others', { style: { display: 'flex', justifyContent: 'center', alignItems: 'center' } }),
]),
Expand Down Expand Up @@ -370,7 +379,7 @@ class CollapsedCodeOverlayWidget extends ViewZoneOverlayWidget {
span.addEventListener('dblclick', e => {
if (e.button !== 0) { return; }
e.preventDefault();
this.showAll();
this._unchangedRegion.showAll(undefined);
});
children.push(span);

Expand Down Expand Up @@ -405,6 +414,4 @@ class CollapsedCodeOverlayWidget extends ViewZoneOverlayWidget {
reset(this._nodes.others, ...children);
}));
}

private showAll() { this._unchangedRegion.showAll(undefined); }
}
7 changes: 7 additions & 0 deletions src/vs/editor/browser/widget/diffEditorWidget2/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*--------------------------------------------------------------------------------------------*/

import { IDimension } from 'vs/base/browser/dom';
import { CancellationTokenSource } from 'vs/base/common/cancellation';
import { isHotReloadEnabled, registerHotReloadHandler } from 'vs/base/common/hotReload';
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
import { IObservable, IReader, ISettableObservable, autorun, autorunHandleChanges, autorunOpts, observableFromEvent, observableSignalFromEvent, observableValue, transaction } from 'vs/base/common/observable';
Expand Down Expand Up @@ -369,3 +370,9 @@ export function applyViewZones(editor: ICodeEditor, viewZones: IObservable<IObse

return store;
}

export class DisposableCancellationTokenSource extends CancellationTokenSource {
public override dispose() {
super.dispose(true);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { DetailedLineRangeMapping } from 'vs/editor/common/diff/rangeMapping';
suite('DiffEditorWidget2', () => {
suite('UnchangedRegion', () => {
function serialize(regions: UnchangedRegion[]): unknown {
return regions.map(r => `${r.originalRange} - ${r.modifiedRange}`);
return regions.map(r => `${r.originalUnchangedRange} - ${r.modifiedUnchangedRange}`);
}

test('Everything changed', () => {
Expand Down