Merge pull request #183522 from microsoft/hediet/changes

Bugfixes

src/vs/base/common/observableImpl/autorun.ts

@@ -23,6 +23,41 @@ export function autorunHandleChanges<TChangeSummary>(
 	return new AutorunObserver(debugName, fn, options.createEmptyChangeSummary, options.handleChange);
 }
 
+// TODO@hediet rename to autorunWithStore
+export function autorunWithStore2(
+	debugName: string,
+	fn: (reader: IReader, store: DisposableStore) => void,
+): IDisposable {
+	return autorunWithStore(fn, debugName);
+}
+
+export function autorunWithStoreHandleChanges<TChangeSummary>(
+	debugName: string,
+	options: {
+		createEmptyChangeSummary?: () => TChangeSummary;
+		handleChange: (context: IChangeContext, changeSummary: TChangeSummary) => boolean;
+	},
+	fn: (reader: IReader, changeSummary: TChangeSummary, store: DisposableStore) => void
+): IDisposable {
+	const store = new DisposableStore();
+	const disposable = autorunHandleChanges(
+		debugName,
+		{
+			createEmptyChangeSummary: options.createEmptyChangeSummary,
+			handleChange: options.handleChange,
+		},
+		(reader, changeSummary) => {
+			store.clear();
+			fn(reader, changeSummary, store);
+		}
+	);
+	return toDisposable(() => {
+		disposable.dispose();
+		store.dispose();
+	});
+}
+
+// TODO@hediet deprecate, rename to autorunWithStoreEx
 export function autorunWithStore(
 	fn: (reader: IReader, store: DisposableStore) => void,
 	debugName: string
@@ -144,13 +179,13 @@ export class AutorunObserver<TChangeSummary = any> implements IObserver, IReader
 	}
 
 	public handlePossibleChange(observable: IObservable<any>): void {
-		if (this.state === AutorunState.upToDate && this.dependencies.has(observable)) {
+		if (this.state === AutorunState.upToDate && this.dependencies.has(observable) && !this.dependenciesToBeRemoved.has(observable)) {
 			this.state = AutorunState.dependenciesMightHaveChanged;
 		}
 	}
 
 	public handleChange<T, TChange>(observable: IObservable<T, TChange>, change: TChange): void {
-		if (this.dependencies.has(observable)) {
+		if (this.dependencies.has(observable) && !this.dependenciesToBeRemoved.has(observable)) {
 			const shouldReact = this._handleChange ? this._handleChange({
 				changedObservable: observable,
 				change,

src/vs/base/common/observableImpl/base.ts

@@ -7,6 +7,11 @@ import { IDisposable } from 'vs/base/common/lifecycle';
 import type { derived } from 'vs/base/common/observableImpl/derived';
 import { getLogger } from 'vs/base/common/observableImpl/logging';
 
+/**
+ * Represents an observable value.
+ * @template T The type of the value.
+ * @template TChange The type of delta information (usually `void` and only used in advanced scenarios).
+ */
 export interface IObservable<T, TChange = unknown> {
 	/**
 	 * Returns the current value.
@@ -248,6 +253,10 @@ export function getFunctionName(fn: Function): string | undefined {
 export interface ISettableObservable<T, TChange = void> extends IObservable<T, TChange>, ISettable<T, TChange> {
 }
 
+/**
+ * Creates an observable value.
+ * Observers get informed when the value changes.
+ */
 export function observableValue<T, TChange = void>(name: string, initialValue: T): ISettableObservable<T, TChange> {
 	return new ObservableValue(name, initialValue);
 }

src/vs/base/common/observableImpl/derived.ts

@@ -196,7 +196,7 @@ export class Derived<T, TChangeSummary = any> extends BaseObservable<T, void> im
 
 	public handlePossibleChange<T>(observable: IObservable<T, unknown>): void {
 		// In all other states, observers already know that we might have changed.
-		if (this.state === DerivedState.upToDate && this.dependencies.has(observable)) {
+		if (this.state === DerivedState.upToDate && this.dependencies.has(observable) && !this.dependenciesToBeRemoved.has(observable)) {
 			this.state = DerivedState.dependenciesMightHaveChanged;
 			for (const r of this.observers) {
 				r.handlePossibleChange(this);
@@ -205,22 +205,19 @@ export class Derived<T, TChangeSummary = any> extends BaseObservable<T, void> im
 	}
 
 	public handleChange<T, TChange>(observable: IObservable<T, TChange>, change: TChange): void {
-		const isUpToDate = this.state === DerivedState.upToDate;
-		let shouldReact = true;
-
-		if (this._handleChange && this.dependencies.has(observable)) {
-			shouldReact = this._handleChange({
+		if (this.dependencies.has(observable) && !this.dependenciesToBeRemoved.has(observable)) {
+			const shouldReact = this._handleChange ? this._handleChange({
 				changedObservable: observable,
 				change,
 				didChange: o => o === observable as any,
-			}, this.changeSummary!);
-		}
-
-		if (shouldReact && (this.state === DerivedState.dependenciesMightHaveChanged || isUpToDate) && this.dependencies.has(observable)) {
-			this.state = DerivedState.stale;
-			if (isUpToDate) {
-				for (const r of this.observers) {
-					r.handlePossibleChange(this);
+			}, this.changeSummary!) : true;
+			const wasUpToDate = this.state === DerivedState.upToDate;
+			if (shouldReact && (this.state === DerivedState.dependenciesMightHaveChanged || wasUpToDate)) {
+				this.state = DerivedState.stale;
+				if (wasUpToDate) {
+					for (const r of this.observers) {
+						r.handlePossibleChange(this);
+					}
 				}
 			}
 		}

src/vs/base/common/observableImpl/utils.ts

@@ -198,6 +198,9 @@ class FromEventObservableSignal extends BaseObservable<void> {
 	}
 }
 
+/**
+ * Creates a signal that can be triggered to invalidate observers.
+ */
 export function observableSignal<TDelta = void>(
 	debugName: string
 ): IObservableSignal<TDelta> {
@@ -287,6 +290,10 @@ export function wasEventTriggeredRecently(event: Event<any>, timeoutMs: number,
 export function keepAlive(observable: IObservable<any>, forceRecompute?: boolean): IDisposable {
 	const o = new KeepAliveObserver(forceRecompute ?? false);
 	observable.addObserver(o);
+	if (forceRecompute) {
+		observable.reportChanges();
+	}
+
 	return toDisposable(() => {
 		observable.removeObserver(o);
 	});

src/vs/editor/browser/editorBrowser.ts

@@ -21,6 +21,7 @@ import { IEditorWhitespace, IViewModel } from 'vs/editor/common/viewModel';
 import { InjectedText } from 'vs/editor/common/modelLineProjectionData';
 import { ILineChange, IDiffComputationResult } from 'vs/editor/common/diff/smartLinesDiffComputer';
 import { IDimension } from 'vs/editor/common/core/dimension';
+import { IBoundarySashes } from 'vs/base/browser/ui/sash/sash';
 
 /**
  * A view zone is a full horizontal rectangle that 'pushes' text down.
@@ -1210,6 +1211,11 @@ export interface IDiffEditor extends editorCommon.IEditor {
 	 * Update the editor's options after the editor has been created.
 	 */
 	updateOptions(newOptions: IDiffEditorOptions): void;
+
+	/**
+	 * @internal
+	 */
+	setBoundarySashes(sashes: IBoundarySashes): void;
 }
 
 /**

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

@@ -866,6 +866,8 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
 		return this._domElement;
 	}
 
+	// #region editorBrowser.IDiffEditor: Delegating to modified Editor
+
 	public getVisibleColumnFromPosition(position: IPosition): number {
 		return this._modifiedEditor.getVisibleColumnFromPosition(position);
 	}
@@ -978,6 +980,24 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
 		return this._modifiedEditor.getSupportedActions();
 	}
 
+	public focus(): void {
+		this._modifiedEditor.focus();
+	}
+
+	public trigger(source: string | null | undefined, handlerId: string, payload: any): void {
+		this._modifiedEditor.trigger(source, handlerId, payload);
+	}
+
+	public createDecorationsCollection(decorations?: IModelDeltaDecoration[]): editorCommon.IEditorDecorationsCollection {
+		return this._modifiedEditor.createDecorationsCollection(decorations);
+	}
+
+	public changeDecorations(callback: (changeAccessor: IModelDecorationsChangeAccessor) => any): any {
+		return this._modifiedEditor.changeDecorations(callback);
+	}
+
+	// #endregion
+
 	public saveViewState(): editorCommon.IDiffEditorViewState {
 		const originalViewState = this._originalEditor.saveViewState();
 		const modifiedViewState = this._modifiedEditor.saveViewState();
@@ -999,9 +1019,6 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
 		this._elementSizeObserver.observe(dimension);
 	}
 
-	public focus(): void {
-		this._modifiedEditor.focus();
-	}
 
 	public hasTextFocus(): boolean {
 		return this._originalEditor.hasTextFocus() || this._modifiedEditor.hasTextFocus();
@@ -1023,18 +1040,6 @@ export class DiffEditorWidget extends Disposable implements editorBrowser.IDiffE
 		this._cleanViewZonesAndDecorations();
 	}
 
-	public trigger(source: string | null | undefined, handlerId: string, payload: any): void {
-		this._modifiedEditor.trigger(source, handlerId, payload);
-	}
-
-	public createDecorationsCollection(decorations?: IModelDeltaDecoration[]): editorCommon.IEditorDecorationsCollection {
-		return this._modifiedEditor.createDecorationsCollection(decorations);
-	}
-
-	public changeDecorations(callback: (changeAccessor: IModelDecorationsChangeAccessor) => any): any {
-		return this._modifiedEditor.changeDecorations(callback);
-	}
-
 	//------------ end IDiffEditor methods
 
 

src/vs/editor/common/diff/documentDiffProvider.ts

@@ -28,7 +28,7 @@ export interface IDocumentDiffProvider {
  */
 export interface IDocumentDiffProviderOptions {
 	/**
-	 * When set to true, the diff should ignore whitespace changes.i
+	 * When set to true, the diff should ignore whitespace changes.
 	 */
 	ignoreTrimWhitespace: boolean;
 

src/vs/monaco.d.ts

@@ -2348,7 +2348,7 @@ declare namespace monaco.editor {
 	 */
 	export interface IDocumentDiffProviderOptions {
 		/**
-		 * When set to true, the diff should ignore whitespace changes.i
+		 * When set to true, the diff should ignore whitespace changes.
 		 */
 		ignoreTrimWhitespace: boolean;
 		/**

src/vs/workbench/browser/parts/editor/textDiffEditor.ts

@@ -45,7 +45,7 @@ export class TextDiffEditor extends AbstractTextEditor<IDiffEditorViewState> imp
 
 	static readonly ID = TEXT_DIFF_EDITOR_ID;
 
-	private diffEditorControl: DiffEditorWidget | undefined = undefined;
+	private diffEditorControl: IDiffEditor | undefined = undefined;
 
 	private diffNavigator: DiffNavigator | undefined;
 	private readonly diffNavigatorDisposables = this._register(new DisposableStore());