Skip to content

Commit c04734f

Browse files
hediethediet
committed
Fixes #218001 
1 parent 9b7b416 commit c04734f

File tree

7 files changed

+219
-117
lines changed

7 files changed

+219
-117
lines changed

‎src/vs/editor/browser/widget/diffEditor/diffEditorWidget.ts

+26-26
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { findLast } from 'vs/base/common/arraysFind';
88
import { BugIndicatingError, onUnexpectedError } from 'vs/base/common/errors';
99
import { Event } from 'vs/base/common/event';
1010
import { toDisposable } from 'vs/base/common/lifecycle';
11-
import { IObservable, ITransaction, autorun, autorunWithStore, derived, observableFromEvent, observableValue, recomputeInitiallyAndOnChange, subtransaction, transaction } from 'vs/base/common/observable';
11+
import { IObservable, ITransaction, autorun, autorunWithStore, derived, disposableObservableValue, observableFromEvent, observableValue, recomputeInitiallyAndOnChange, subtransaction, transaction } from 'vs/base/common/observable';
1212
import { derivedDisposable } from 'vs/base/common/observableInternal/derived';
1313
import 'vs/css!./style';
1414
import { IEditorConstructionOptions } from 'vs/editor/browser/config/editorConfiguration';
@@ -26,7 +26,7 @@ import { HideUnchangedRegionsFeature } from 'vs/editor/browser/widget/diffEditor
2626
import { MovedBlocksLinesFeature } from 'vs/editor/browser/widget/diffEditor/features/movedBlocksLinesFeature';
2727
import { OverviewRulerFeature } from 'vs/editor/browser/widget/diffEditor/features/overviewRulerFeature';
2828
import { RevertButtonsFeature } from 'vs/editor/browser/widget/diffEditor/features/revertButtonsFeature';
29-
import { CSSStyle, ObservableElementSizeObserver, applyStyle, applyViewZones, translatePosition } from 'vs/editor/browser/widget/diffEditor/utils';
29+
import { CSSStyle, ObservableElementSizeObserver, RefCounted, applyStyle, applyViewZones, translatePosition } from 'vs/editor/browser/widget/diffEditor/utils';
3030
import { readHotReloadableExport } from 'vs/base/common/hotReloadHelpers';
3131
import { bindContextKey } from 'vs/platform/observable/common/platformObservableUtils';
3232
import { IDiffEditorOptions } from 'vs/editor/common/config/editorOptions';
@@ -62,8 +62,8 @@ export class DiffEditorWidget extends DelegatingEditor implements IDiffEditor {
6262
h('div.editor.modified@modified', { style: { position: 'absolute', height: '100%', } }),
6363
h('div.accessibleDiffViewer@accessibleDiffViewer', { style: { position: 'absolute', height: '100%' } }),
6464
]);
65-
private readonly _diffModel = observableValue<DiffEditorViewModel | undefined>(this, undefined);
66-
private _shouldDisposeDiffModel = false;
65+
private readonly _diffModelSrc = this._register(disposableObservableValue<RefCounted<DiffEditorViewModel> | undefined>(this, undefined));
66+
private readonly _diffModel = derived<DiffEditorViewModel | undefined>(this, reader => this._diffModelSrc.read(reader)?.object);
6767
public readonly onDidChangeModel = Event.fromObservableLight(this._diffModel);
6868

6969
public get onDidContentSizeChange() { return this._editors.onDidContentSizeChange; }
@@ -318,12 +318,6 @@ export class DiffEditorWidget extends DelegatingEditor implements IDiffEditor {
318318
}
319319
}));
320320

321-
this._register(toDisposable(() => {
322-
if (this._shouldDisposeDiffModel) {
323-
this._diffModel.get()?.dispose();
324-
}
325-
}));
326-
327321
this._register(autorunWithStore((reader, store) => {
328322
store.add(new (readHotReloadableExport(RevertButtonsFeature, reader))(this._editors, this._diffModel, this._options, this));
329323
}));
@@ -478,30 +472,36 @@ export class DiffEditorWidget extends DelegatingEditor implements IDiffEditor {
478472

479473
override getModel(): IDiffEditorModel | null { return this._diffModel.get()?.model ?? null; }
480474

481-
override setModel(model: IDiffEditorModel | null | IDiffEditorViewModel, tx?: ITransaction): void {
482-
if (!model && this._diffModel.get()) {
475+
override setModel(model: IDiffEditorModel | null | IDiffEditorViewModel): void {
476+
const vm = !model ? null
477+
: ('model' in model) ? RefCounted.create(model).createNewRef(this)
478+
: RefCounted.create(this.createViewModel(model), this);
479+
this.setDiffModel(vm);
480+
}
481+
482+
setDiffModel(viewModel: RefCounted<IDiffEditorViewModel> | null, tx?: ITransaction): void {
483+
const currentModel = this._diffModel.get();
484+
485+
if (!viewModel && currentModel) {
483486
// Transitioning from a model to no-model
484487
this._accessibleDiffViewer.get().close();
485488
}
486489

487-
const vm = model ? ('model' in model) ? { model, shouldDispose: false } : { model: this.createViewModel(model), shouldDispose: true } : undefined;
488-
489-
if (this._diffModel.get() !== vm?.model) {
490+
if (this._diffModel.get() !== viewModel?.object) {
490491
subtransaction(tx, tx => {
492+
const vm = viewModel?.object;
491493
/** @description DiffEditorWidget.setModel */
492494
observableFromEvent.batchEventsGlobally(tx, () => {
493-
this._editors.original.setModel(vm ? vm.model.model.original : null);
494-
this._editors.modified.setModel(vm ? vm.model.model.modified : null);
495+
this._editors.original.setModel(vm ? vm.model.original : null);
496+
this._editors.modified.setModel(vm ? vm.model.modified : null);
495497
});
496-
const prevValue = this._diffModel.get();
497-
const shouldDispose = this._shouldDisposeDiffModel;
498-
499-
this._shouldDisposeDiffModel = vm?.shouldDispose ?? false;
500-
this._diffModel.set(vm?.model as (DiffEditorViewModel | undefined), tx);
501-
502-
if (shouldDispose) {
503-
prevValue?.dispose();
504-
}
498+
const prevValueRef = this._diffModelSrc.get()?.createNewRef(this);
499+
this._diffModelSrc.set(viewModel?.createNewRef(this) as RefCounted<DiffEditorViewModel> | undefined, tx);
500+
setTimeout(() => {
501+
// async, so that this runs after the transaction finished.
502+
// TODO: use the transaction to schedule disposal
503+
prevValueRef?.dispose();
504+
}, 0);
505505
});
506506
}
507507
}

‎src/vs/editor/browser/widget/diffEditor/utils.ts

+102-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
import { IDimension } from 'vs/base/browser/dom';
77
import { findLast } from 'vs/base/common/arraysFind';
88
import { CancellationTokenSource } from 'vs/base/common/cancellation';
9-
import { Disposable, DisposableStore, IDisposable, toDisposable } from 'vs/base/common/lifecycle';
9+
import { Disposable, DisposableStore, IDisposable, IReference, toDisposable } from 'vs/base/common/lifecycle';
1010
import { IObservable, ISettableObservable, autorun, autorunHandleChanges, autorunOpts, autorunWithStore, observableValue, transaction } from 'vs/base/common/observable';
1111
import { ElementSizeObserver } from 'vs/editor/browser/config/elementSizeObserver';
1212
import { ICodeEditor, IOverlayWidget, IViewZone } from 'vs/editor/browser/editorBrowser';
@@ -415,3 +415,104 @@ export function filterWithPrevious<T>(arr: T[], filter: (cur: T, prev: T | undef
415415
return result;
416416
});
417417
}
418+
419+
export interface IRefCounted extends IDisposable {
420+
createNewRef(): this;
421+
}
422+
423+
export abstract class RefCounted<T> implements IDisposable, IReference<T> {
424+
public static create<T extends IDisposable>(value: T, debugOwner: object | undefined = undefined): RefCounted<T> {
425+
return new BaseRefCounted(value, value, debugOwner);
426+
}
427+
428+
public static createWithDisposable<T extends IDisposable>(value: T, disposable: IDisposable, debugOwner: object | undefined = undefined): RefCounted<T> {
429+
const store = new DisposableStore();
430+
store.add(disposable);
431+
store.add(value);
432+
return new BaseRefCounted(value, store, debugOwner);
433+
}
434+
435+
public static createOfNonDisposable<T>(value: T, disposable: IDisposable, debugOwner: object | undefined = undefined): RefCounted<T> {
436+
return new BaseRefCounted(value, disposable, debugOwner);
437+
}
438+
439+
public abstract createNewRef(debugOwner?: object | undefined): RefCounted<T>;
440+
441+
public abstract dispose(): void;
442+
443+
public abstract get object(): T;
444+
}
445+
446+
class BaseRefCounted<T> extends RefCounted<T> {
447+
private _refCount = 1;
448+
private _isDisposed = false;
449+
private readonly _owners: object[] = [];
450+
451+
constructor(
452+
public override readonly object: T,
453+
private readonly _disposable: IDisposable,
454+
private readonly _debugOwner: object | undefined,
455+
) {
456+
super();
457+
458+
if (_debugOwner) {
459+
this._addOwner(_debugOwner);
460+
}
461+
}
462+
463+
private _addOwner(debugOwner: object | undefined) {
464+
if (debugOwner) {
465+
this._owners.push(debugOwner);
466+
}
467+
}
468+
469+
public createNewRef(debugOwner?: object | undefined): RefCounted<T> {
470+
this._refCount++;
471+
if (debugOwner) {
472+
this._addOwner(debugOwner);
473+
}
474+
return new ClonedRefCounted(this, debugOwner);
475+
}
476+
477+
public dispose(): void {
478+
if (this._isDisposed) { return; }
479+
this._isDisposed = true;
480+
this._decreaseRefCount(this._debugOwner);
481+
}
482+
483+
public _decreaseRefCount(debugOwner?: object | undefined): void {
484+
this._refCount--;
485+
if (this._refCount === 0) {
486+
this._disposable.dispose();
487+
}
488+
489+
if (debugOwner) {
490+
const idx = this._owners.indexOf(debugOwner);
491+
if (idx !== -1) {
492+
this._owners.splice(idx, 1);
493+
}
494+
}
495+
}
496+
}
497+
498+
class ClonedRefCounted<T> extends RefCounted<T> {
499+
private _isDisposed = false;
500+
constructor(
501+
private readonly _base: BaseRefCounted<T>,
502+
private readonly _debugOwner: object | undefined,
503+
) {
504+
super();
505+
}
506+
507+
public get object(): T { return this._base.object; }
508+
509+
public createNewRef(debugOwner?: object | undefined): RefCounted<T> {
510+
return this._base.createNewRef(debugOwner);
511+
}
512+
513+
public dispose(): void {
514+
if (this._isDisposed) { return; }
515+
this._isDisposed = true;
516+
this._base._decreaseRefCount(this._debugOwner);
517+
}
518+
}

‎src/vs/editor/browser/widget/multiDiffEditor/diffEditorItemTemplate.ts

+28-26
Original file line numberDiff line numberDiff line change
@@ -6,20 +6,20 @@ import { h } from 'vs/base/browser/dom';
66
import { Button } from 'vs/base/browser/ui/button/button';
77
import { Codicon } from 'vs/base/common/codicons';
88
import { Disposable, DisposableStore } from 'vs/base/common/lifecycle';
9-
import { autorun, derived, observableFromEvent } from 'vs/base/common/observable';
10-
import { IObservable, globalTransaction, observableValue } from 'vs/base/common/observableInternal/base';
11-
import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
9+
import { autorun, derived } from 'vs/base/common/observable';
10+
import { globalTransaction, observableValue } from 'vs/base/common/observableInternal/base';
11+
import { observableCodeEditor } from 'vs/editor/browser/observableCodeEditor';
1212
import { DiffEditorWidget } from 'vs/editor/browser/widget/diffEditor/diffEditorWidget';
1313
import { DocumentDiffItemViewModel } from 'vs/editor/browser/widget/multiDiffEditor/multiDiffEditorViewModel';
1414
import { IWorkbenchUIElementFactory } from 'vs/editor/browser/widget/multiDiffEditor/workbenchUIElementFactory';
1515
import { IDiffEditorOptions } from 'vs/editor/common/config/editorOptions';
1616
import { OffsetRange } from 'vs/editor/common/core/offsetRange';
17+
import { createActionViewItem } from 'vs/platform/actions/browser/menuEntryActionViewItem';
1718
import { MenuWorkbenchToolBar } from 'vs/platform/actions/browser/toolbar';
1819
import { MenuId } from 'vs/platform/actions/common/actions';
1920
import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
2021
import { IObjectData, IPooledObject } from './objectPool';
2122
import { ActionRunnerWithContext } from './utils';
22-
import { createActionViewItem } from 'vs/platform/actions/browser/menuEntryActionViewItem';
2323

2424
export class TemplateData implements IObjectData {
2525
constructor(
@@ -81,8 +81,8 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject<
8181
overflowWidgetsDomNode: this._overflowWidgetsDomNode,
8282
}, {}));
8383

84-
private readonly isModifedFocused = isFocused(this.editor.getModifiedEditor());
85-
private readonly isOriginalFocused = isFocused(this.editor.getOriginalEditor());
84+
private readonly isModifedFocused = observableCodeEditor(this.editor.getModifiedEditor()).isFocused;
85+
private readonly isOriginalFocused = observableCodeEditor(this.editor.getOriginalEditor()).isFocused;
8686
public readonly isFocused = derived(this, reader => this.isModifedFocused.read(reader) || this.isOriginalFocused.read(reader));
8787

8888
private readonly _resourceLabel = this._workbenchUIElementFactory.createResourceLabel
@@ -177,7 +177,7 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject<
177177

178178
private _data: TemplateData | undefined;
179179

180-
public setData(data: TemplateData): void {
180+
public setData(data: TemplateData | undefined): void {
181181
this._data = data;
182182
function updateOptions(options: IDiffEditorOptions): IDiffEditorOptions {
183183
return {
@@ -198,13 +198,17 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject<
198198
};
199199
}
200200

201-
const value = data.viewModel.entry.value!; // TODO
202-
203-
if (value.onOptionsDidChange) {
204-
this._dataStore.add(value.onOptionsDidChange(() => {
205-
this.editor.updateOptions(updateOptions(value.options ?? {}));
206-
}));
201+
if (!data) {
202+
globalTransaction(tx => {
203+
this._viewModel.set(undefined, tx);
204+
this.editor.setDiffModel(null, tx);
205+
this._dataStore.clear();
206+
});
207+
return;
207208
}
209+
210+
const value = data.viewModel.documentDiffItem;
211+
208212
globalTransaction(tx => {
209213
this._resourceLabel?.setUri(data.viewModel.modifiedUri ?? data.viewModel.originalUri!, { strikethrough: data.viewModel.modifiedUri === undefined });
210214

@@ -231,9 +235,19 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject<
231235

232236
this._dataStore.clear();
233237
this._viewModel.set(data.viewModel, tx);
234-
this.editor.setModel(data.viewModel.diffEditorViewModel, tx);
238+
this.editor.setDiffModel(data.viewModel.diffEditorViewModelRef, tx);
235239
this.editor.updateOptions(updateOptions(value.options ?? {}));
236240
});
241+
if (value.onOptionsDidChange) {
242+
this._dataStore.add(value.onOptionsDidChange(() => {
243+
this.editor.updateOptions(updateOptions(value.options ?? {}));
244+
}));
245+
}
246+
data.viewModel.isAlive.recomputeInitiallyAndOnChange(this._dataStore, value => {
247+
if (!value) {
248+
this.setData(undefined);
249+
}
250+
});
237251
}
238252

239253
private readonly _headerHeight = /*this._elements.header.clientHeight*/ 40;
@@ -276,15 +290,3 @@ export class DiffEditorItemTemplate extends Disposable implements IPooledObject<
276290
this._elements.root.style.visibility = 'hidden'; // Some editor parts are still visible
277291
}
278292
}
279-
280-
function isFocused(editor: ICodeEditor): IObservable<boolean> {
281-
return observableFromEvent(
282-
h => {
283-
const store = new DisposableStore();
284-
store.add(editor.onDidFocusEditorWidget(() => h(true)));
285-
store.add(editor.onDidBlurEditorWidget(() => h(false)));
286-
return store;
287-
},
288-
() => editor.hasTextFocus()
289-
);
290-
}

‎src/vs/editor/browser/widget/multiDiffEditor/model.ts

+2-23
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,16 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { Event, IValueWithChangeEvent } from 'vs/base/common/event';
7+
import { RefCounted } from 'vs/editor/browser/widget/diffEditor/utils';
78
import { IDiffEditorOptions } from 'vs/editor/common/config/editorOptions';
89
import { ITextModel } from 'vs/editor/common/model';
910
import { ContextKeyValue } from 'vs/platform/contextkey/common/contextkey';
1011

1112
export interface IMultiDiffEditorModel {
12-
readonly documents: IValueWithChangeEvent<readonly LazyPromise<IDocumentDiffItem>[]>;
13+
readonly documents: IValueWithChangeEvent<readonly RefCounted<IDocumentDiffItem>[]>;
1314
readonly contextKeys?: Record<string, ContextKeyValue>;
1415
}
1516

16-
export interface LazyPromise<T> {
17-
request(): Promise<T>;
18-
readonly value: T | undefined;
19-
readonly onHasValueDidChange: Event<void>;
20-
}
21-
22-
export class ConstLazyPromise<T> implements LazyPromise<T> {
23-
public readonly onHasValueDidChange = Event.None;
24-
25-
constructor(
26-
private readonly _value: T
27-
) { }
28-
29-
public request(): Promise<T> {
30-
return Promise.resolve(this._value);
31-
}
32-
33-
public get value(): T {
34-
return this._value;
35-
}
36-
}
37-
3817
export interface IDocumentDiffItem {
3918
/**
4019
* undefined if the file was created.

0 commit comments

Comments
 (0)