Merge pull request microsoft#154457 from microsoft/ben/flying-fowl

Author: jrieken

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

@@ -573,24 +573,31 @@ abstract class AbstractCloseAllAction extends Action {
 	override async run(): Promise<void> {
 
 		// Depending on the editor and auto save configuration,
-		// split dirty editors into buckets
+		// split editors into buckets for handling confirmation
 
 		const dirtyEditorsWithDefaultConfirm = new Set<IEditorIdentifier>();
 		const dirtyAutoSaveOnFocusChangeEditors = new Set<IEditorIdentifier>();
 		const dirtyAutoSaveOnWindowChangeEditors = new Set<IEditorIdentifier>();
-		const dirtyEditorsWithCustomConfirm = new Map<string /* typeId */, Set<IEditorIdentifier>>();
+		const editorsWithCustomConfirm = new Map<string /* typeId */, Set<IEditorIdentifier>>();
 
 		for (const { editor, groupId } of this.editorService.getEditors(EditorsOrder.SEQUENTIAL, { excludeSticky: this.excludeSticky })) {
-			if (!editor.isDirty() || editor.isSaving()) {
-				continue; // only interested in dirty editors that are not in the process of saving
+			let confirmClose = false;
+			if (editor.closeHandler) {
+				confirmClose = editor.closeHandler.showConfirm(); // custom handling of confirmation on close
+			} else {
+				confirmClose = editor.isDirty() && !editor.isSaving(); // default confirm only when dirty and not saving
+			}
+
+			if (!confirmClose) {
+				continue;
 			}
 
 			// Editor has custom confirm implementation
-			if (typeof editor.confirm === 'function') {
-				let customEditorsToConfirm = dirtyEditorsWithCustomConfirm.get(editor.typeId);
+			if (typeof editor.closeHandler?.confirm === 'function') {
+				let customEditorsToConfirm = editorsWithCustomConfirm.get(editor.typeId);
 				if (!customEditorsToConfirm) {
 					customEditorsToConfirm = new Set();
-					dirtyEditorsWithCustomConfirm.set(editor.typeId, customEditorsToConfirm);
+					editorsWithCustomConfirm.set(editor.typeId, customEditorsToConfirm);
 				}
 
 				customEditorsToConfirm.add({ editor, groupId });
@@ -619,7 +626,7 @@ abstract class AbstractCloseAllAction extends Action {
 		if (dirtyEditorsWithDefaultConfirm.size > 0) {
 			const editors = Array.from(dirtyEditorsWithDefaultConfirm.values());
 
-			await this.revealDirtyEditors(editors); // help user make a decision by revealing editors
+			await this.revealEditorsToConfirm(editors); // help user make a decision by revealing editors
 
 			const confirmation = await this.fileDialogService.showSaveConfirm(editors.map(({ editor }) => {
 				if (editor instanceof SideBySideEditorInput) {
@@ -642,12 +649,12 @@ abstract class AbstractCloseAllAction extends Action {
 		}
 
 		// 2.) Show custom confirm based dialog
-		for (const [, editorIdentifiers] of dirtyEditorsWithCustomConfirm) {
+		for (const [, editorIdentifiers] of editorsWithCustomConfirm) {
 			const editors = Array.from(editorIdentifiers.values());
 
-			await this.revealDirtyEditors(editors); // help user make a decision by revealing editors
+			await this.revealEditorsToConfirm(editors); // help user make a decision by revealing editors
 
-			const confirmation = await firstOrDefault(editors)?.editor.confirm?.(editors);
+			const confirmation = await firstOrDefault(editors)?.editor.closeHandler?.confirm?.(editors);
 			if (typeof confirmation === 'number') {
 				switch (confirmation) {
 					case ConfirmResult.CANCEL:
@@ -683,7 +690,7 @@ abstract class AbstractCloseAllAction extends Action {
 		return this.doCloseAll();
 	}
 
-	private async revealDirtyEditors(editors: ReadonlyArray<IEditorIdentifier>): Promise<void> {
+	private async revealEditorsToConfirm(editors: ReadonlyArray<IEditorIdentifier>): Promise<void> {
 		try {
 			const handledGroups = new Set<GroupIdentifier>();
 			for (const { editor, groupId } of editors) {

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

@@ -1322,16 +1322,16 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 	//#region closeEditor()
 
 	async closeEditor(editor: EditorInput | undefined = this.activeEditor || undefined, options?: ICloseEditorOptions): Promise<boolean> {
-		return this.doCloseEditorWithDirtyHandling(editor, options);
+		return this.doCloseEditorWithConfirmationHandling(editor, options);
 	}
 
-	private async doCloseEditorWithDirtyHandling(editor: EditorInput | undefined = this.activeEditor || undefined, options?: ICloseEditorOptions, internalOptions?: IInternalEditorCloseOptions): Promise<boolean> {
+	private async doCloseEditorWithConfirmationHandling(editor: EditorInput | undefined = this.activeEditor || undefined, options?: ICloseEditorOptions, internalOptions?: IInternalEditorCloseOptions): Promise<boolean> {
 		if (!editor) {
 			return false;
 		}
 
-		// Check for dirty and veto
-		const veto = await this.handleDirtyClosing([editor]);
+		// Check for confirmation and veto
+		const veto = await this.handleCloseConfirmation([editor]);
 		if (veto) {
 			return false;
 		}
@@ -1461,7 +1461,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		return this.model.closeEditor(editor, internalOptions?.context)?.editorIndex;
 	}
 
-	private async handleDirtyClosing(editors: EditorInput[]): Promise<boolean /* veto */> {
+	private async handleCloseConfirmation(editors: EditorInput[]): Promise<boolean /* veto */> {
 		if (!editors.length) {
 			return false; // no veto
 		}
@@ -1470,15 +1470,15 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 
 		// To prevent multiple confirmation dialogs from showing up one after the other
 		// we check if a pending confirmation is currently showing and if so, join that
-		let handleDirtyClosingPromise = this.mapEditorToPendingConfirmation.get(editor);
-		if (!handleDirtyClosingPromise) {
-			handleDirtyClosingPromise = this.doHandleDirtyClosing(editor);
-			this.mapEditorToPendingConfirmation.set(editor, handleDirtyClosingPromise);
+		let handleCloseConfirmationPromise = this.mapEditorToPendingConfirmation.get(editor);
+		if (!handleCloseConfirmationPromise) {
+			handleCloseConfirmationPromise = this.doHandleCloseConfirmation(editor);
+			this.mapEditorToPendingConfirmation.set(editor, handleCloseConfirmationPromise);
 		}
 
 		let veto: boolean;
 		try {
-			veto = await handleDirtyClosingPromise;
+			veto = await handleCloseConfirmationPromise;
 		} finally {
 			this.mapEditorToPendingConfirmation.delete(editor);
 		}
@@ -1489,12 +1489,12 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		}
 
 		// Otherwise continue with the remainders
-		return this.handleDirtyClosing(editors);
+		return this.handleCloseConfirmation(editors);
 	}
 
-	private async doHandleDirtyClosing(editor: EditorInput, options?: { skipAutoSave: boolean }): Promise<boolean /* veto */> {
-		if (!editor.isDirty() || editor.isSaving()) {
-			return false; // editor must be dirty and not saving
+	private async doHandleCloseConfirmation(editor: EditorInput, options?: { skipAutoSave: boolean }): Promise<boolean /* veto */> {
+		if (!this.shouldConfirmClose(editor)) {
+			return false; // no veto
 		}
 
 		if (editor instanceof SideBySideEditorInput && this.model.contains(editor.primary)) {
@@ -1531,10 +1531,11 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		// on auto-save configuration.
 		// However, make sure to respect `skipAutoSave` option in case the automated
 		// save fails which would result in the editor never closing.
+		// Also, we only do this if no custom confirmation handling is implemented.
 		let confirmation = ConfirmResult.CANCEL;
 		let saveReason = SaveReason.EXPLICIT;
 		let autoSave = false;
-		if (!editor.hasCapability(EditorInputCapabilities.Untitled) && !options?.skipAutoSave) {
+		if (!editor.hasCapability(EditorInputCapabilities.Untitled) && !options?.skipAutoSave && !editor.closeHandler) {
 
 			// Auto-save on focus change: save, because a dialog would steal focus
 			// (see https://github.com/microsoft/vscode/issues/108752)
@@ -1554,15 +1555,15 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 			}
 		}
 
-		// No auto-save on focus change: ask user
+		// No auto-save on focus change or custom confirmation handler: ask user
 		if (!autoSave) {
 
-			// Switch to editor that we want to handle and confirm to save/revert
+			// Switch to editor that we want to handle for confirmation
 			await this.doOpenEditor(editor);
 
 			// Let editor handle confirmation if implemented
-			if (typeof editor.confirm === 'function') {
-				confirmation = await editor.confirm();
+			if (typeof editor.closeHandler?.confirm === 'function') {
+				confirmation = await editor.closeHandler.confirm();
 			}
 
 			// Show a file specific confirmation
@@ -1578,11 +1579,12 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 			}
 		}
 
-		// It could be that the editor saved meanwhile or is saving, so we check
+		// It could be that the editor's choice of confirmation has changed
+		// given the check for confirmation is long running, so we check
 		// again to see if anything needs to happen before closing for good.
-		// This can happen for example if autoSave: onFocusChange is configured
+		// This can happen for example if `autoSave: onFocusChange` is configured
 		// so that the save happens when the dialog opens.
-		if (!editor.isDirty() || editor.isSaving()) {
+		if (!this.shouldConfirmClose(editor)) {
 			return confirmation === ConfirmResult.CANCEL ? true : false;
 		}
 
@@ -1595,7 +1597,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 					// we handle the dirty editor again but this time ensuring to
 					// show the confirm dialog
 					// (see https://github.com/microsoft/vscode/issues/108752)
-					return this.doHandleDirtyClosing(editor, { skipAutoSave: true });
+					return this.doHandleCloseConfirmation(editor, { skipAutoSave: true });
 				}
 
 				return editor.isDirty(); // veto if still dirty
@@ -1621,6 +1623,14 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 		}
 	}
 
+	private shouldConfirmClose(editor: EditorInput): boolean {
+		if (editor.closeHandler) {
+			return editor.closeHandler.showConfirm(); // custom handling of confirmation on close
+		}
+
+		return editor.isDirty() && !editor.isSaving(); // editor must be dirty and not saving
+	}
+
 	//#endregion
 
 	//#region closeEditors()
@@ -1632,8 +1642,8 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 
 		const editors = this.doGetEditorsToClose(args);
 
-		// Check for dirty and veto
-		const veto = await this.handleDirtyClosing(editors.slice(0));
+		// Check for confirmation and veto
+		const veto = await this.handleCloseConfirmation(editors.slice(0));
 		if (veto) {
 			return false;
 		}
@@ -1714,8 +1724,8 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 			return true;
 		}
 
-		// Check for dirty and veto
-		const veto = await this.handleDirtyClosing(this.model.getEditors(EditorsOrder.MOST_RECENTLY_ACTIVE, options));
+		// Check for confirmation and veto
+		const veto = await this.handleCloseConfirmation(this.model.getEditors(EditorsOrder.MOST_RECENTLY_ACTIVE, options));
 		if (veto) {
 			return false;
 		}
@@ -1795,7 +1805,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 					this.doCloseEditor(editor, false, { context: EditorCloseContext.REPLACE });
 					closed = true;
 				} else {
-					closed = await this.doCloseEditorWithDirtyHandling(editor, { preserveFocus: true }, { context: EditorCloseContext.REPLACE });
+					closed = await this.doCloseEditorWithConfirmationHandling(editor, { preserveFocus: true }, { context: EditorCloseContext.REPLACE });
 				}
 
 				if (!closed) {
@@ -1815,7 +1825,7 @@ export class EditorGroupView extends Themable implements IEditorGroupView {
 				if (activeReplacement.forceReplaceDirty) {
 					this.doCloseEditor(activeReplacement.editor, false, { context: EditorCloseContext.REPLACE });
 				} else {
-					await this.doCloseEditorWithDirtyHandling(activeReplacement.editor, { preserveFocus: true }, { context: EditorCloseContext.REPLACE });
+					await this.doCloseEditorWithConfirmationHandling(activeReplacement.editor, { preserveFocus: true }, { context: EditorCloseContext.REPLACE });
 				}
 			}
 

src/vs/workbench/common/editor/editorInput.ts

@@ -11,6 +11,31 @@ import { EditorInputCapabilities, Verbosity, GroupIdentifier, ISaveOptions, IRev
 import { isEqual } from 'vs/base/common/resources';
 import { ConfirmResult } from 'vs/platform/dialogs/common/dialogs';
 
+export interface IEditorCloseHandler {
+
+	/**
+	 * If `true`, will call into the `confirm` method to ask for confirmation
+	 * before closing the editor.
+	 */
+	showConfirm(): boolean;
+
+	/**
+	 * Allows an editor to control what should happen when the editor
+	 * (or a list of editor of the same kind) is being closed.
+	 *
+	 * By default a file specific dialog will open if the editor is
+	 * dirty and not in the process of saving.
+	 *
+	 * If the editor is not dealing with files or another condition
+	 * should be used besides dirty state, this method should be
+	 * implemented to show a different dialog.
+	 *
+	 * @param editors if more than one editor is closed, will pass in
+	 * each editor of the same kind to be able to show a combined dialog.
+	 */
+	confirm(editors?: ReadonlyArray<IEditorIdentifier>): Promise<ConfirmResult>;
+}
+
 /**
  * Editor inputs are lightweight objects that can be passed to the workbench API to open inside the editor part.
  * Each editor input is mapped to an editor that is capable of opening it through the Platform facade.
@@ -45,6 +70,12 @@ export abstract class EditorInput extends AbstractEditorInput {
 
 	private disposed: boolean = false;
 
+	/**
+	 * Optional: subclasses can override to implement
+	 * custom confirmation on close behavior.
+	 */
+	readonly closeHandler?: IEditorCloseHandler;
+
 	/**
 	 * Unique type identifier for this input. Every editor input of the
 	 * same class should share the same type identifier. The type identifier
@@ -168,20 +199,6 @@ export abstract class EditorInput extends AbstractEditorInput {
 		return null;
 	}
 
-	/**
-	 * Optional: if this method is implemented, allows an editor to
-	 * control what should happen when the editor (or a list of editors
-	 * of the same kind) is dirty and there is an intent to close it.
-	 *
-	 * By default a file specific dialog will open. If the editor is
-	 * not dealing with files, this method should be implemented to
-	 * show a different dialog.
-	 *
-	 * @param editors if more than one editor is closed, will pass in
-	 * each editor of the same kind to be able to show a combined dialog.
-	 */
-	confirm?(editors?: ReadonlyArray<IEditorIdentifier>): Promise<ConfirmResult>;
-
 	/**
 	 * Saves the editor. The provided groupId helps implementors
 	 * to e.g. preserve view state of the editor and re-open it

src/vs/workbench/contrib/mergeEditor/browser/mergeEditorInput.ts

@@ -14,15 +14,14 @@ import { IFileService } from 'vs/platform/files/common/files';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ILabelService } from 'vs/platform/label/common/label';
 import { IEditorIdentifier, IUntypedEditorInput } from 'vs/workbench/common/editor';
-import { EditorInput } from 'vs/workbench/common/editor/editorInput';
+import { EditorInput, IEditorCloseHandler } from 'vs/workbench/common/editor/editorInput';
 import { AbstractTextResourceEditorInput } from 'vs/workbench/common/editor/textResourceEditorInput';
 import { EditorWorkerServiceDiffComputer } from 'vs/workbench/contrib/mergeEditor/browser/model/diffComputer';
 import { MergeEditorModel } from 'vs/workbench/contrib/mergeEditor/browser/model/mergeEditorModel';
 import { IEditorService } from 'vs/workbench/services/editor/common/editorService';
 import { AutoSaveMode, IFilesConfigurationService } from 'vs/workbench/services/filesConfiguration/common/filesConfigurationService';
 import { ILanguageSupport, ITextFileEditorModel, ITextFileService } from 'vs/workbench/services/textfile/common/textfiles';
 import { assertType } from 'vs/base/common/types';
-import { Event } from 'vs/base/common/event';
 
 export class MergeEditorInputData {
 	constructor(
@@ -33,7 +32,7 @@ export class MergeEditorInputData {
 	) { }
 }
 
-export class MergeEditorInput extends AbstractTextResourceEditorInput implements ILanguageSupport {
+export class MergeEditorInput extends AbstractTextResourceEditorInput implements ILanguageSupport, IEditorCloseHandler {
 
 	static readonly ID = 'mergeEditor.Input';
 
@@ -125,8 +124,6 @@ export class MergeEditorInput extends AbstractTextResourceEditorInput implements
 			this._store.add(input1);
 			this._store.add(input2);
 			this._store.add(result);
-
-			this._store.add(Event.fromObservable(this._model.hasUnhandledConflicts)(() => this._onDidChangeDirty.fire(undefined)));
 		}
 
 		this._ignoreUnhandledConflictsForDirtyState = undefined;
@@ -146,21 +143,25 @@ export class MergeEditorInput extends AbstractTextResourceEditorInput implements
 	// ---- FileEditorInput
 
 	override isDirty(): boolean {
-		const textModelDirty = Boolean(this._outTextModel?.isDirty());
-		if (textModelDirty) {
+		return Boolean(this._outTextModel?.isDirty());
+	}
+
+	override readonly closeHandler = this;
+
+	showConfirm(): boolean {
+		if (this.isDirty()) {
 			// text model dirty -> 3wm is dirty
 			return true;
 		}
 		if (!this._ignoreUnhandledConflictsForDirtyState) {
-			// unhandled conflicts -> 3wm is dirty UNLESS we explicitly set this input
-			// to ignore unhandled conflicts for the dirty-state. This happens only
-			// after confirming to ignore unhandled changes
+			// unhandled conflicts -> 3wm asks to confirm UNLESS we explicitly set this input
+			// to ignore unhandled conflicts. This happens only after confirming to ignore unhandled changes
 			return Boolean(this._model && this._model.hasUnhandledConflicts.get());
 		}
 		return false;
 	}
 
-	override async confirm(editors?: ReadonlyArray<IEditorIdentifier>): Promise<ConfirmResult> {
+	async confirm(editors?: ReadonlyArray<IEditorIdentifier>): Promise<ConfirmResult> {
 
 		const inputs: MergeEditorInput[] = [this];
 		if (editors) {