format on save, format on paste don't format when multiple formatters are available, #41882

src/vs/editor/common/services/editorWorkerService.ts

@@ -26,7 +26,7 @@ export interface IEditorWorkerService {
 	canComputeDirtyDiff(original: URI, modified: URI): boolean;
 	computeDirtyDiff(original: URI, modified: URI, ignoreTrimWhitespace: boolean): Promise<IChange[] | null>;
 
-	computeMoreMinimalEdits(resource: URI, edits: TextEdit[] | null | undefined): Promise<TextEdit[] | null | undefined>;
+	computeMoreMinimalEdits(resource: URI, edits: TextEdit[] | null | undefined): Promise<TextEdit[] | undefined>;
 
 	canComputeWordRanges(resource: URI): boolean;
 	computeWordRanges(resource: URI, range: IRange): Promise<{ [word: string]: IRange[] } | null>;

src/vs/editor/common/services/editorWorkerServiceImpl.ts

@@ -20,6 +20,7 @@ import { IDiffComputationResult, IEditorWorkerService } from 'vs/editor/common/s
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { ITextResourceConfigurationService } from 'vs/editor/common/services/resourceConfiguration';
 import { regExpFlags } from 'vs/base/common/strings';
+import { isNonEmptyArray } from 'vs/base/common/arrays';
 
 /**
  * Stop syncing a model to the worker if it was not needed for 1 min.
@@ -88,14 +89,15 @@ export class EditorWorkerServiceImpl extends Disposable implements IEditorWorker
 		return this._workerManager.withWorker().then(client => client.computeDirtyDiff(original, modified, ignoreTrimWhitespace));
 	}
 
-	public computeMoreMinimalEdits(resource: URI, edits: modes.TextEdit[] | null | undefined): Promise<modes.TextEdit[] | null | undefined> {
-		if (!Array.isArray(edits) || edits.length === 0) {
-			return Promise.resolve(edits);
-		} else {
+	public computeMoreMinimalEdits(resource: URI, edits: modes.TextEdit[] | null | undefined): Promise<modes.TextEdit[] | undefined> {
+		if (isNonEmptyArray(edits)) {
 			if (!canSyncModel(this._modelService, resource)) {
 				return Promise.resolve(edits); // File too large
 			}
 			return this._workerManager.withWorker().then(client => client.computeMoreMinimalEdits(resource, edits));
+
+		} else {
+			return Promise.resolve(undefined);
 		}
 	}
 

src/vs/editor/contrib/format/format.ts

@@ -5,7 +5,6 @@
 
 import { alert } from 'vs/base/browser/ui/aria/aria';
 import { isNonEmptyArray } from 'vs/base/common/arrays';
-import { first } from 'vs/base/common/async';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { illegalArgument, onUnexpectedExternalError } from 'vs/base/common/errors';
 import { URI } from 'vs/base/common/uri';
@@ -22,7 +21,6 @@ import { IEditorWorkerService } from 'vs/editor/common/services/editorWorkerServ
 import { IModelService } from 'vs/editor/common/services/modelService';
 import { FormattingEdit } from 'vs/editor/contrib/format/formattingEdit';
 import * as nls from 'vs/nls';
-import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
 import { ExtensionIdentifier } from 'vs/platform/extensions/common/extensions';
 
 export function alertFormattingEdits(edits: ISingleEditOperation[]): void {
@@ -52,7 +50,7 @@ export function alertFormattingEdits(edits: ISingleEditOperation[]): void {
 	}
 }
 
-export function getAllDocumentFormattersOrdered(model: ITextModel): DocumentFormattingEditProvider[] {
+export function getRealAndSyntheticDocumentFormattersOrdered(model: ITextModel): DocumentFormattingEditProvider[] {
 	const result: DocumentFormattingEditProvider[] = [];
 	const seen = new Set<string>();
 
@@ -85,29 +83,6 @@ export function getAllDocumentFormattersOrdered(model: ITextModel): DocumentForm
 	return result;
 }
 
-export async function formatDocumentRangeUntilResult(
-	accessor: ServicesAccessor,
-	editorOrModel: ITextModel | IActiveCodeEditor,
-	range: Range,
-	token: CancellationToken
-): Promise<boolean> {
-
-	const insta = accessor.get(IInstantiationService);
-	const model = isCodeEditor(editorOrModel) ? editorOrModel.getModel() : editorOrModel;
-	const providers = DocumentRangeFormattingEditProviderRegistry.ordered(model);
-
-	for (const provider of providers) {
-		if (token.isCancellationRequested) {
-			return false;
-		}
-		const didFormat = await insta.invokeFunction(formatDocumentRangeWithProvider, provider, editorOrModel, range, token);
-		if (didFormat) {
-			return true;
-		}
-	}
-	return false;
-}
-
 export async function formatDocumentRangeWithProvider(
 	accessor: ServicesAccessor,
 	provider: DocumentRangeFormattingEditProvider,
@@ -177,28 +152,6 @@ export async function formatDocumentRangeWithProvider(
 	return true;
 }
 
-export async function formatDocumentUntilResult(
-	accessor: ServicesAccessor,
-	editorOrModel: ITextModel | IActiveCodeEditor,
-	token: CancellationToken
-): Promise<boolean> {
-
-	const insta = accessor.get(IInstantiationService);
-	const model = isCodeEditor(editorOrModel) ? editorOrModel.getModel() : editorOrModel;
-	const providers = DocumentFormattingEditProviderRegistry.ordered(model);
-
-	for (const provider of providers) {
-		if (token.isCancellationRequested) {
-			return false;
-		}
-		const didFormat = await insta.invokeFunction(formatDocumentWithProvider, provider, editorOrModel, token);
-		if (didFormat) {
-			return true;
-		}
-	}
-	return false;
-}
-
 export async function formatDocumentWithProvider(
 	accessor: ServicesAccessor,
 	provider: DocumentFormattingEditProvider,
@@ -272,15 +225,16 @@ export async function getDocumentRangeFormattingEditsUntilResult(
 	range: Range,
 	options: FormattingOptions,
 	token: CancellationToken
-): Promise<TextEdit[] | undefined | null> {
+): Promise<TextEdit[] | undefined> {
 
 	const providers = DocumentRangeFormattingEditProviderRegistry.ordered(model);
-	return first(providers.map(provider => () => {
-		return Promise.resolve(provider.provideDocumentRangeFormattingEdits(model, range, options, token)).catch(onUnexpectedExternalError);
-	}), isNonEmptyArray).then(edits => {
-		// break edits into smaller edits
-		return workerService.computeMoreMinimalEdits(model.uri, edits);
-	});
+	for (const provider of providers) {
+		let rawEdits = await Promise.resolve(provider.provideDocumentRangeFormattingEdits(model, range, options, token)).catch(onUnexpectedExternalError);
+		if (isNonEmptyArray(rawEdits)) {
+			return await workerService.computeMoreMinimalEdits(model.uri, rawEdits);
+		}
+	}
+	return undefined;
 }
 
 export async function getDocumentFormattingEditsUntilResult(
@@ -288,25 +242,16 @@ export async function getDocumentFormattingEditsUntilResult(
 	model: ITextModel,
 	options: FormattingOptions,
 	token: CancellationToken
-): Promise<TextEdit[] | null | undefined> {
+): Promise<TextEdit[] | undefined> {
 
-	// (1) try document formatter - if available, if successfull
-	const providers = DocumentFormattingEditProviderRegistry.ordered(model);
+	const providers = getRealAndSyntheticDocumentFormattersOrdered(model);
 	for (const provider of providers) {
 		let rawEdits = await Promise.resolve(provider.provideDocumentFormattingEdits(model, options, token)).catch(onUnexpectedExternalError);
-		if (rawEdits) {
+		if (isNonEmptyArray(rawEdits)) {
 			return await workerService.computeMoreMinimalEdits(model.uri, rawEdits);
 		}
 	}
-
-	// (2) try range formatters when no document formatter is registered
-	return getDocumentRangeFormattingEditsUntilResult(
-		workerService,
-		model,
-		model.getFullModelRange(),
-		options,
-		token
-	);
+	return undefined;
 }
 
 export function getOnTypeFormattingEdits(

src/vs/editor/contrib/format/formatActions.ts

@@ -16,7 +16,7 @@ import * as editorCommon from 'vs/editor/common/editorCommon';
 import { EditorContextKeys } from 'vs/editor/common/editorContextKeys';
 import { DocumentRangeFormattingEditProviderRegistry, OnTypeFormattingEditProviderRegistry } from 'vs/editor/common/modes';
 import { IEditorWorkerService } from 'vs/editor/common/services/editorWorkerService';
-import { getOnTypeFormattingEdits, formatDocumentRangeUntilResult, formatDocumentWithProvider, formatDocumentRangeWithProvider, alertFormattingEdits, getAllDocumentFormattersOrdered } from 'vs/editor/contrib/format/format';
+import { getOnTypeFormattingEdits, formatDocumentWithProvider, formatDocumentRangeWithProvider, alertFormattingEdits, getRealAndSyntheticDocumentFormattersOrdered } from 'vs/editor/contrib/format/format';
 import { FormattingEdit } from 'vs/editor/contrib/format/formattingEdit';
 import * as nls from 'vs/nls';
 import { CommandsRegistry, ICommandService } from 'vs/platform/commands/common/commands';
@@ -196,16 +196,12 @@ class FormatOnPaste implements editorCommon.IEditorContribution {
 			return;
 		}
 
-		let model = this.editor.getModel();
-
-		// no support
-		if (!DocumentRangeFormattingEditProviderRegistry.has(model)) {
+		// no formatter
+		if (!DocumentRangeFormattingEditProviderRegistry.has(this.editor.getModel())) {
 			return;
 		}
 
-		this._callOnModel.push(this.editor.onDidPaste((range: Range) => {
-			this._trigger(range);
-		}));
+		this._callOnModel.push(this.editor.onDidPaste(range => this._trigger(range)));
 	}
 
 	private _trigger(range: Range): void {
@@ -215,7 +211,12 @@ class FormatOnPaste implements editorCommon.IEditorContribution {
 		if (this.editor.getSelections().length > 1) {
 			return;
 		}
-		this._instantiationService.invokeFunction(formatDocumentRangeUntilResult, this.editor, range, CancellationToken.None).catch(onUnexpectedError);
+		const provider = DocumentRangeFormattingEditProviderRegistry.ordered(this.editor.getModel());
+		if (provider.length !== 1) {
+			// print status in n>1 case?
+			return;
+		}
+		this._instantiationService.invokeFunction(formatDocumentRangeWithProvider, provider[0], this.editor, range, CancellationToken.None).catch(onUnexpectedError);
 	}
 }
 
@@ -247,7 +248,7 @@ class FormatDocumentAction extends EditorAction {
 		}
 		const instaService = accessor.get(IInstantiationService);
 		const model = editor.getModel();
-		const [provider] = getAllDocumentFormattersOrdered(model);
+		const [provider] = getRealAndSyntheticDocumentFormattersOrdered(model);
 		if (provider) {
 			await instaService.invokeFunction(formatDocumentWithProvider, provider, editor, CancellationToken.None);
 		}

src/vs/workbench/api/browser/mainThreadSaveParticipant.ts

@@ -21,7 +21,7 @@ import { shouldSynchronizeModel } from 'vs/editor/common/services/modelService';
 import { getCodeActions } from 'vs/editor/contrib/codeAction/codeAction';
 import { applyCodeAction } from 'vs/editor/contrib/codeAction/codeActionCommands';
 import { CodeActionKind } from 'vs/editor/contrib/codeAction/codeActionTrigger';
-import { formatDocumentUntilResult } from 'vs/editor/contrib/format/format';
+import { getRealAndSyntheticDocumentFormattersOrdered, formatDocumentWithProvider } from 'vs/editor/contrib/format/format';
 import { SnippetController2 } from 'vs/editor/contrib/snippet/snippetController2';
 import { localize } from 'vs/nls';
 import { ICommandService } from 'vs/platform/commands/common/commands';
@@ -219,23 +219,32 @@ class FormatOnSaveParticipant implements ISaveParticipantParticipant {
 	async participate(editorModel: IResolvedTextFileEditorModel, env: { reason: SaveReason }): Promise<void> {
 
 		const model = editorModel.textEditorModel;
-		if (env.reason === SaveReason.AUTO
-			|| !this._configurationService.getValue('editor.formatOnSave', { overrideIdentifier: model.getLanguageIdentifier().language, resource: editorModel.getResource() })) {
+		const overrides = { overrideIdentifier: model.getLanguageIdentifier().language, resource: model.uri };
+
+		if (env.reason === SaveReason.AUTO || !this._configurationService.getValue('editor.formatOnSave', overrides)) {
 			return undefined;
 		}
 
-		const timeout = this._configurationService.getValue<number>('editor.formatOnSaveTimeout', { overrideIdentifier: model.getLanguageIdentifier().language, resource: editorModel.getResource() });
-
 		return new Promise<any>((resolve, reject) => {
 			const source = new CancellationTokenSource();
-			const request = this._instantiationService.invokeFunction(formatDocumentUntilResult, model, source.token);
 
-			setTimeout(() => {
-				reject(localize('timeout.formatOnSave', "Aborted format on save after {0}ms", timeout));
-				source.cancel();
-			}, timeout);
+			const provider = getRealAndSyntheticDocumentFormattersOrdered(model);
+			if (provider.length !== 1) {
+				// print message for >1 case?
+				resolve();
+
+			} else {
+				// having 1 formatter -> go for it
+				const timeout = this._configurationService.getValue<number>('editor.formatOnSaveTimeout', overrides);
+				const request = this._instantiationService.invokeFunction(formatDocumentWithProvider, provider[0], model, source.token);
 
-			request.then(resolve, reject);
+				setTimeout(() => {
+					reject(localize('timeout.formatOnSave', "Aborted format on save after {0}ms", timeout));
+					source.cancel();
+				}, timeout);
+
+				request.then(resolve, reject);
+			}
 		});
 	}
 }

src/vs/workbench/contrib/format/browser/formatActionsMultiple.ts

@@ -14,7 +14,7 @@ import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegis
 import { IQuickInputService, IQuickPickItem, IQuickInputButton } from 'vs/platform/quickinput/common/quickInput';
 import { CancellationToken } from 'vs/base/common/cancellation';
 import { IInstantiationService } from 'vs/platform/instantiation/common/instantiation';
-import { formatDocumentRangeWithProvider, formatDocumentWithProvider, getAllDocumentFormattersOrdered } from 'vs/editor/contrib/format/format';
+import { formatDocumentRangeWithProvider, formatDocumentWithProvider, getRealAndSyntheticDocumentFormattersOrdered } from 'vs/editor/contrib/format/format';
 import { Range } from 'vs/editor/common/core/range';
 import { showExtensionQuery } from 'vs/workbench/contrib/format/browser/showExtensionQuery';
 import { IViewletService } from 'vs/workbench/services/viewlet/browser/viewlet';
@@ -58,7 +58,7 @@ registerEditorAction(class FormatDocumentMultipleAction extends EditorAction {
 		const viewletService = accessor.get(IViewletService);
 		const model = editor.getModel();
 
-		const provider = getAllDocumentFormattersOrdered(model);
+		const provider = getRealAndSyntheticDocumentFormattersOrdered(model);
 		const picks = provider.map((provider, index) => {
 			return <IIndexedPick>{
 				index,

src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts

@@ -905,8 +905,8 @@ suite('ExtHostLanguageFeatures', function () {
 	// --- format
 
 	const NullWorkerService = new class extends mock<IEditorWorkerService>() {
-		computeMoreMinimalEdits(resource: URI, edits: modes.TextEdit[] | null | undefined): Promise<modes.TextEdit[] | null | undefined> {
-			return Promise.resolve(edits);
+		computeMoreMinimalEdits(resource: URI, edits: modes.TextEdit[] | null | undefined): Promise<modes.TextEdit[] | undefined> {
+			return Promise.resolve(edits || undefined);
 		}
 	};