Fixes microsoft#187164 (microsoft#187192)

Author: hediet

src/vs/editor/browser/widget/diffEditorWidget2/diffEditorEditors.ts

@@ -15,6 +15,7 @@ import { IContentSizeChangedEvent } from 'vs/editor/common/editorCommon';
 import { localize } from 'vs/nls';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { DiffEditorOptions } from './diffEditorOptions';
+import { IObservable, IReader } from 'vs/base/common/observable';
 
 export class DiffEditorEditors extends Disposable {
 	public readonly modified: CodeEditorWidget;
@@ -29,6 +30,7 @@ export class DiffEditorEditors extends Disposable {
 		private readonly _options: DiffEditorOptions,
 		codeEditorWidgetOptions: IDiffCodeEditorWidgetOptions,
 		private readonly _createInnerEditor: (instantiationService: IInstantiationService, container: HTMLElement, options: Readonly<IEditorOptions>, editorWidgetOptions: ICodeEditorWidgetOptions) => CodeEditorWidget,
+		private readonly _modifiedReadOnlyOverride: IObservable<boolean>,
 		@IInstantiationService private readonly _instantiationService: IInstantiationService
 	) {
 		super();
@@ -47,19 +49,21 @@ export class DiffEditorEditors extends Disposable {
 		}, (reader, changeSummary) => {
 			_options.editorOptions.read(reader);
 
-			this.modified.updateOptions(this._adjustOptionsForRightHandSide(changeSummary));
-			this.original.updateOptions(this._adjustOptionsForLeftHandSide(changeSummary));
+			this.modified.updateOptions(this._adjustOptionsForRightHandSide(reader, changeSummary));
+			this.original.updateOptions(this._adjustOptionsForLeftHandSide(reader, changeSummary));
 		}));
 	}
 
 	private _createLeftHandSideEditor(options: Readonly<IDiffEditorConstructionOptions>, codeEditorWidgetOptions: ICodeEditorWidgetOptions): CodeEditorWidget {
-		const editor = this._constructInnerEditor(this._instantiationService, this.originalEditorElement, this._adjustOptionsForLeftHandSide(options), codeEditorWidgetOptions);
+		const leftHandSideOptions = this._adjustOptionsForLeftHandSide(undefined, options);
+		const editor = this._constructInnerEditor(this._instantiationService, this.originalEditorElement, leftHandSideOptions, codeEditorWidgetOptions);
 		editor.setContextValue('isInDiffLeftEditor', true);
 		return editor;
 	}
 
 	private _createRightHandSideEditor(options: Readonly<IDiffEditorConstructionOptions>, codeEditorWidgetOptions: ICodeEditorWidgetOptions): CodeEditorWidget {
-		const editor = this._constructInnerEditor(this._instantiationService, this.modifiedEditorElement, this._adjustOptionsForRightHandSide(options), codeEditorWidgetOptions);
+		const rightHandSideOptions = this._adjustOptionsForRightHandSide(undefined, options);
+		const editor = this._constructInnerEditor(this._instantiationService, this.modifiedEditorElement, rightHandSideOptions, codeEditorWidgetOptions);
 		editor.setContextValue('isInDiffRightEditor', true);
 		return editor;
 	}
@@ -81,7 +85,7 @@ export class DiffEditorEditors extends Disposable {
 		return editor;
 	}
 
-	private _adjustOptionsForLeftHandSide(changedOptions: Readonly<IDiffEditorConstructionOptions>): IEditorConstructionOptions {
+	private _adjustOptionsForLeftHandSide(_reader: IReader | undefined, changedOptions: Readonly<IDiffEditorConstructionOptions>): IEditorConstructionOptions {
 		const result = this._adjustOptionsForSubEditor(changedOptions);
 		if (!this._options.renderSideBySide.get()) {
 			// never wrap hidden editor
@@ -101,7 +105,7 @@ export class DiffEditorEditors extends Disposable {
 		return result;
 	}
 
-	private _adjustOptionsForRightHandSide(changedOptions: Readonly<IDiffEditorConstructionOptions>): IEditorConstructionOptions {
+	private _adjustOptionsForRightHandSide(reader: IReader | undefined, changedOptions: Readonly<IDiffEditorConstructionOptions>): IEditorConstructionOptions {
 		const result = this._adjustOptionsForSubEditor(changedOptions);
 		if (changedOptions.modifiedAriaLabel) {
 			result.ariaLabel = changedOptions.modifiedAriaLabel;
@@ -111,6 +115,7 @@ export class DiffEditorEditors extends Disposable {
 		result.revealHorizontalRightPadding = EditorOptions.revealHorizontalRightPadding.defaultValue + OverviewRulerPart.ENTIRE_DIFF_OVERVIEW_WIDTH;
 		result.scrollbar!.verticalHasArrows = false;
 		result.extraEditorClassName = 'modified-in-monaco-diff-editor';
+		result.readOnly = this._modifiedReadOnlyOverride.read(reader) || this._options.editorOptions.get().readOnly;
 		return result;
 	}
 

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

@@ -96,13 +96,15 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
 		this._rootSizeObserver = this._register(new ObservableElementSizeObserver(this.elements.root, options.dimension));
 		this._rootSizeObserver.setAutomaticLayout(options.automaticLayout ?? false);
 
+		const reviewPaneObservable = observableValue<undefined | DiffReview2>('reviewPane', undefined);
 		this._editors = this._register(this._instantiationService.createInstance(
 			DiffEditorEditors,
 			this.elements.original,
 			this.elements.modified,
 			this._options,
 			codeEditorWidgetOptions,
 			(i, c, o, o2) => this._createInnerEditor(i, c, o, o2),
+			reviewPaneObservable.map((r, reader) => r?.isVisible.read(reader) ?? false),
 		));
 
 		this._sash = derivedWithStore('sash', (reader, store) => {
@@ -159,6 +161,7 @@ export class DiffEditorWidget2 extends DelegatingEditor implements IDiffEditor {
 		this.elements.root.appendChild(this._reviewPane.domNode.domNode);
 		this.elements.root.appendChild(this._reviewPane.shadow.domNode);
 		this.elements.root.appendChild(this._reviewPane.actionBarContainer.domNode);
+		reviewPaneObservable.set(this._reviewPane, undefined);
 
 		this._createDiffEditorContributions();
 

src/vs/editor/browser/widget/diffEditorWidget2/diffReview.ts

@@ -11,6 +11,7 @@ import { Action } from 'vs/base/common/actions';
 import { Codicon } from 'vs/base/common/codicons';
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
 import { Disposable } from 'vs/base/common/lifecycle';
+import { IObservable, observableValue } from 'vs/base/common/observable';
 import { ThemeIcon } from 'vs/base/common/themables';
 import { Constants } from 'vs/base/common/uint';
 import { applyFontInfo } from 'vs/editor/browser/config/domFontInfo';
@@ -85,7 +86,7 @@ export class DiffReview2 extends Disposable {
 	private static _ttPolicy = DiffReview._ttPolicy; // TODO inline once DiffReview is deprecated.
 
 	private readonly _diffEditor: DiffEditorWidget2;
-	private _isVisible: boolean;
+	private get _isVisible() { return this._isVisibleObs.get(); }
 	public readonly shadow: FastDomNode<HTMLElement>;
 	private readonly _actionBar: ActionBar;
 	public readonly actionBarContainer: FastDomNode<HTMLElement>;
@@ -95,6 +96,10 @@ export class DiffReview2 extends Disposable {
 	private _diffs: Diff[];
 	private _currentDiff: Diff | null;
 
+	private readonly _isVisibleObs = observableValue('isVisible', false);
+
+	public readonly isVisible: IObservable<boolean> = this._isVisibleObs;
+
 	constructor(
 		diffEditor: DiffEditorWidget2,
 		@ILanguageService private readonly _languageService: ILanguageService,
@@ -103,7 +108,6 @@ export class DiffReview2 extends Disposable {
 	) {
 		super();
 		this._diffEditor = diffEditor;
-		this._isVisible = false;
 
 		this.shadow = createFastDomNode(document.createElement('div'));
 		this.shadow.setClassName('diff-review-shadow');
@@ -215,7 +219,7 @@ export class DiffReview2 extends Disposable {
 		const entries = this._diffs[index].entries;
 		this._diffEditor.setPosition(new Position(entries[0].modifiedLineStart, 1));
 		this._diffEditor.setSelection({ startColumn: 1, startLineNumber: entries[0].modifiedLineStart, endColumn: Constants.MAX_SAFE_SMALL_INTEGER, endLineNumber: entries[entries.length - 1].modifiedLineEnd });
-		this._isVisible = true;
+		this._isVisibleObs.set(true, undefined);
 		this.layout();
 		this._render();
 		this._goToRow(this._getPrevRow(), 'previous');
@@ -250,7 +254,7 @@ export class DiffReview2 extends Disposable {
 		const entries = this._diffs[index].entries;
 		this._diffEditor.setPosition(new Position(entries[0].modifiedLineStart, 1));
 		this._diffEditor.setSelection({ startColumn: 1, startLineNumber: entries[0].modifiedLineStart, endColumn: Constants.MAX_SAFE_SMALL_INTEGER, endLineNumber: entries[entries.length - 1].modifiedLineEnd });
-		this._isVisible = true;
+		this._isVisibleObs.set(true, undefined);
 		this.layout();
 		this._render();
 		this._goToRow(this._getNextRow(), 'next');
@@ -274,8 +278,7 @@ export class DiffReview2 extends Disposable {
 	}
 
 	private hide(): void {
-		this._isVisible = false;
-		this._diffEditor.updateOptions({ readOnly: false });
+		this._isVisibleObs.set(false, undefined);
 		this._diffEditor.focus();
 		this.layout();
 		this._render();
@@ -331,10 +334,6 @@ export class DiffReview2 extends Disposable {
 		this.scrollbar.scanDomNode();
 	}
 
-	public isVisible(): boolean {
-		return this._isVisible;
-	}
-
 	private _width: number = 0;
 	private _top: number = 0;
 	private _height: number = 0;
@@ -564,7 +563,6 @@ export class DiffReview2 extends Disposable {
 			return;
 		}
 
-		this._diffEditor.updateOptions({ readOnly: true });
 		const diffIndex = this._findDiffIndex(this._diffEditor.getPosition()!);
 
 		if (this._diffs[diffIndex] === this._currentDiff) {