From 636dd226c634538a7e56b522b8ef405a00e58968 Mon Sep 17 00:00:00 2001 From: Cristopher Claeys Date: Fri, 3 Feb 2023 16:45:51 +0100 Subject: [PATCH] Address PR comments --- src/vs/editor/common/languages.ts | 8 +- .../editor/contrib/format/browser/format.ts | 150 ++++++------------ src/vs/monaco.d.ts | 8 +- .../api/browser/mainThreadLanguageFeatures.ts | 2 +- .../workbench/api/common/extHost.api.impl.ts | 2 +- .../workbench/api/common/extHost.protocol.ts | 2 +- .../api/common/extHostLanguageFeatures.ts | 39 ++--- src/vscode-dts/vscode.d.ts | 15 +- 8 files changed, 95 insertions(+), 131 deletions(-) diff --git a/src/vs/editor/common/languages.ts b/src/vs/editor/common/languages.ts index 25e78075d3415..724c153827632 100644 --- a/src/vs/editor/common/languages.ts +++ b/src/vs/editor/common/languages.ts @@ -1182,6 +1182,10 @@ export interface FormattingOptions { * Prefer spaces over tabs. */ insertSpaces: boolean; + /** + * The list of multiple ranges to format at once, if the provider supports it. + */ + ranges?: Range[]; } /** * The document formatting provider interface defines the contract between extensions and @@ -1213,7 +1217,7 @@ export interface DocumentRangeFormattingEditProvider { readonly displayName?: string; - readonly multiRange: boolean; + readonly canFormatMultipleRanges: boolean; /** * Provide formatting edits for a range in a document. @@ -1222,7 +1226,7 @@ export interface DocumentRangeFormattingEditProvider { * or larger range. Often this is done by adjusting the start and end * of the range to full syntax nodes. */ - provideDocumentRangeFormattingEdits(model: model.ITextModel, range: Range | Range[], options: FormattingOptions, token: CancellationToken): ProviderResult; + provideDocumentRangeFormattingEdits(model: model.ITextModel, range: Range, options: FormattingOptions, token: CancellationToken): ProviderResult; } /** * The document formatting provider interface defines the contract between extensions and diff --git a/src/vs/editor/contrib/format/browser/format.ts b/src/vs/editor/contrib/format/browser/format.ts index 97bee12cd9a13..e25cf6e1a61b2 100644 --- a/src/vs/editor/contrib/format/browser/format.ts +++ b/src/vs/editor/contrib/format/browser/format.ts @@ -107,11 +107,6 @@ export interface IFormattingEditProviderSelector { (formatter: T[], document: ITextModel, mode: FormattingMode): Promise; } -interface IEditResult { - cancelled: boolean; - allEdits: TextEdit[]; -} - export abstract class FormattingConflicts { private static readonly _selectors = new LinkedList(); @@ -172,95 +167,7 @@ export async function formatDocumentRangesWithProvider( model = editorOrModel; cts = new TextModelCancellationTokenSource(editorOrModel, token); } - let result: IEditResult; - if (provider.multiRange) { - result = await formatDocumentRangesWithMultiRangeProvider(provider, editorOrModel, rangeOrRanges, cts, model, workerService); - } else { - result = await formatDocumentRangesWithSingleRangeProvider(provider, editorOrModel, rangeOrRanges, cts, model, workerService); - } - if (result.cancelled) { return true; } - - const { allEdits } = result; - - if (allEdits.length === 0) { - return false; - } - - if (isCodeEditor(editorOrModel)) { - // use editor to apply edits - FormattingEdit.execute(editorOrModel, allEdits, true); - alertFormattingEdits(allEdits); - editorOrModel.revealPositionInCenterIfOutsideViewport(editorOrModel.getPosition(), ScrollType.Immediate); - - } else { - // use model to apply edits - const [{ range }] = allEdits; - const initialSelection = new Selection(range.startLineNumber, range.startColumn, range.endLineNumber, range.endColumn); - model.pushEditOperations([initialSelection], allEdits.map(edit => { - return { - text: edit.text, - range: Range.lift(edit.range), - forceMoveMarkers: true - }; - }), undoEdits => { - for (const { range } of undoEdits) { - if (Range.areIntersectingOrTouching(range, initialSelection)) { - return [new Selection(range.startLineNumber, range.startColumn, range.endLineNumber, range.endColumn)]; - } - } - return null; - }); - } - - return true; -} - -async function formatDocumentRangesWithMultiRangeProvider( - provider: DocumentRangeFormattingEditProvider, - editorOrModel: ITextModel | IActiveCodeEditor, - rangeOrRanges: Range | Range[], - cts: CancellationTokenSource, - model: ITextModel, - workerService: IEditorWorkerService -): Promise { - const ranges = asArray(rangeOrRanges); - - const allEdits: TextEdit[] = []; - try { - if (cts.token.isCancellationRequested) { - return { cancelled: true, allEdits }; - } - - const rawEdits = (await provider.provideDocumentRangeFormattingEdits( - model, - ranges, - model.getFormattingOptions(), - cts.token - )) || []; - if (cts.token.isCancellationRequested) { - return { cancelled: true, allEdits }; - } - - const minimalEdits = await workerService.computeMoreMinimalEdits(model.uri, rawEdits); - if (minimalEdits) { - allEdits.push(...minimalEdits); - } - } finally { - cts.dispose(); - } - - return { cancelled: false, allEdits }; -} - -async function formatDocumentRangesWithSingleRangeProvider( - provider: DocumentRangeFormattingEditProvider, - editorOrModel: ITextModel | IActiveCodeEditor, - rangeOrRanges: Range | Range[], - cts: CancellationTokenSource, - model: ITextModel, - workerService: IEditorWorkerService -): Promise { // make sure that ranges don't overlap nor touch each other const ranges: Range[] = []; let len = 0; @@ -310,17 +217,29 @@ async function formatDocumentRangesWithSingleRangeProvider( const allEdits: TextEdit[] = []; const rawEditsList: TextEdit[][] = []; try { - for (const range of ranges) { - if (cts.token.isCancellationRequested) { - return { cancelled: true, allEdits }; + if (provider.canFormatMultipleRanges) { + logService.trace(`[format][provideDocumentRangeFormattingEdits] (request)`, provider.extensionId?.value, ranges); + const result = (await provider.provideDocumentRangeFormattingEdits( + model, + ranges[0], + { ...model.getFormattingOptions(), ranges }, + cts.token + )) || []; + logService.trace(`[format][provideDocumentRangeFormattingEdits] (response)`, provider.extensionId?.value, result); + rawEditsList.push(result); + } else { + for (const range of ranges) { + if (cts.token.isCancellationRequested) { + return true; + } + rawEditsList.push(await computeEdits(range)); } - rawEditsList.push(await computeEdits(range)); } for (let i = 0; i < ranges.length; ++i) { for (let j = i + 1; j < ranges.length; ++j) { if (cts.token.isCancellationRequested) { - return { cancelled: true, allEdits }; + return true; } if (hasIntersectingEdit(rawEditsList[i], rawEditsList[j])) { // Merge ranges i and j into a single range, recompute the associated edits @@ -341,7 +260,7 @@ async function formatDocumentRangesWithSingleRangeProvider( for (const rawEdits of rawEditsList) { if (cts.token.isCancellationRequested) { - return { cancelled: true, allEdits }; + return true; } const minimalEdits = await workerService.computeMoreMinimalEdits(model.uri, rawEdits); if (minimalEdits) { @@ -351,7 +270,38 @@ async function formatDocumentRangesWithSingleRangeProvider( } finally { cts.dispose(); } - return { cancelled: false, allEdits }; + + if (allEdits.length === 0) { + return false; + } + + if (isCodeEditor(editorOrModel)) { + // use editor to apply edits + FormattingEdit.execute(editorOrModel, allEdits, true); + alertFormattingEdits(allEdits); + editorOrModel.revealPositionInCenterIfOutsideViewport(editorOrModel.getPosition(), ScrollType.Immediate); + + } else { + // use model to apply edits + const [{ range }] = allEdits; + const initialSelection = new Selection(range.startLineNumber, range.startColumn, range.endLineNumber, range.endColumn); + model.pushEditOperations([initialSelection], allEdits.map(edit => { + return { + text: edit.text, + range: Range.lift(edit.range), + forceMoveMarkers: true + }; + }), undoEdits => { + for (const { range } of undoEdits) { + if (Range.areIntersectingOrTouching(range, initialSelection)) { + return [new Selection(range.startLineNumber, range.startColumn, range.endLineNumber, range.endColumn)]; + } + } + return null; + }); + } + + return true; } export async function formatDocumentWithSelectedProvider( diff --git a/src/vs/monaco.d.ts b/src/vs/monaco.d.ts index 9544913da5e1c..3a63a10e7c136 100644 --- a/src/vs/monaco.d.ts +++ b/src/vs/monaco.d.ts @@ -7121,6 +7121,10 @@ declare namespace monaco.languages { * Prefer spaces over tabs. */ insertSpaces: boolean; + /** + * The list of multiple ranges to format at once, if the provider supports it. + */ + ranges?: Range[]; } /** @@ -7141,7 +7145,7 @@ declare namespace monaco.languages { */ export interface DocumentRangeFormattingEditProvider { readonly displayName?: string; - readonly multiRange: boolean; + readonly canFormatMultipleRanges: boolean; /** * Provide formatting edits for a range in a document. * @@ -7149,7 +7153,7 @@ declare namespace monaco.languages { * or larger range. Often this is done by adjusting the start and end * of the range to full syntax nodes. */ - provideDocumentRangeFormattingEdits(model: editor.ITextModel, range: Range | Range[], options: FormattingOptions, token: CancellationToken): ProviderResult; + provideDocumentRangeFormattingEdits(model: editor.ITextModel, range: Range, options: FormattingOptions, token: CancellationToken): ProviderResult; } /** diff --git a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts index 1ffd26003544f..3743c52522947 100644 --- a/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts +++ b/src/vs/workbench/api/browser/mainThreadLanguageFeatures.ts @@ -391,7 +391,7 @@ export class MainThreadLanguageFeatures extends Disposable implements MainThread this._registrations.set(handle, this._languageFeaturesService.documentRangeFormattingEditProvider.register(selector, { extensionId, displayName, - multiRange: metadata.multiRange, + canFormatMultipleRanges: metadata.canFormatMultipleRanges, provideDocumentRangeFormattingEdits: (model: ITextModel, range: EditorRange | EditorRange[], options: languages.FormattingOptions, token: CancellationToken): Promise => { return this._proxy.$provideDocumentRangeFormattingEdits(handle, model.uri, range, options, token); } diff --git a/src/vs/workbench/api/common/extHost.api.impl.ts b/src/vs/workbench/api/common/extHost.api.impl.ts index 65363f34c8eda..8785ecc0989d2 100644 --- a/src/vs/workbench/api/common/extHost.api.impl.ts +++ b/src/vs/workbench/api/common/extHost.api.impl.ts @@ -526,7 +526,7 @@ export function createApiFactoryAndRegisterActors(accessor: ServicesAccessor): I registerDocumentFormattingEditProvider(selector: vscode.DocumentSelector, provider: vscode.DocumentFormattingEditProvider): vscode.Disposable { return extHostLanguageFeatures.registerDocumentFormattingEditProvider(extension, checkSelector(selector), provider); }, - registerDocumentRangeFormattingEditProvider(selector: vscode.DocumentSelector, provider: vscode.DocumentRangeFormattingEditProvider, metadata?: vscode.DocumentRangeFormattingEditProviderMetadata): vscode.Disposable { + registerDocumentRangeFormattingEditProvider(selector: vscode.DocumentSelector, provider: vscode.DocumentRangeFormattingEditProvider, metadata?: vscode.DocumentRangeFormattingEditProviderMetadata): vscode.Disposable { return extHostLanguageFeatures.registerDocumentRangeFormattingEditProvider(extension, checkSelector(selector), provider, metadata); }, registerOnTypeFormattingEditProvider(selector: vscode.DocumentSelector, provider: vscode.OnTypeFormattingEditProvider, firstTriggerCharacter: string, ...moreTriggerCharacters: string[]): vscode.Disposable { diff --git a/src/vs/workbench/api/common/extHost.protocol.ts b/src/vs/workbench/api/common/extHost.protocol.ts index f8a2db1692c75..3195c3df01b9e 100644 --- a/src/vs/workbench/api/common/extHost.protocol.ts +++ b/src/vs/workbench/api/common/extHost.protocol.ts @@ -353,7 +353,7 @@ export interface IDocumentFilterDto { } export interface IRangeFormattingProviderMetadataDto { - readonly multiRange?: boolean; + readonly canFormatMultipleRanges?: boolean; } export interface ISignatureHelpProviderMetadataDto { diff --git a/src/vs/workbench/api/common/extHostLanguageFeatures.ts b/src/vs/workbench/api/common/extHostLanguageFeatures.ts index 201a29ec75de1..d2aff45e99369 100644 --- a/src/vs/workbench/api/common/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/common/extHostLanguageFeatures.ts @@ -547,27 +547,28 @@ class DocumentFormattingAdapter { } } -class RangeFormattingAdapter { +class RangeFormattingAdapter { constructor( private readonly _documents: ExtHostDocuments, - private readonly _provider: vscode.DocumentRangeFormattingEditProvider, - private readonly _multiRange: boolean + private readonly _provider: vscode.DocumentRangeFormattingEditProvider, + private readonly _canFormatMultipleRanges: boolean ) { } - async provideDocumentRangeFormattingEdits(resource: URI, range: IRange | IRange[], options: languages.FormattingOptions, token: CancellationToken): Promise { + async provideDocumentRangeFormattingEdits(resource: URI, range: IRange, options: languages.FormattingOptions, token: CancellationToken): Promise { - const document = this._documents.getDocument(resource); - let ran: Range | Range[]; - if (this._multiRange) { - if (!Array.isArray(range)) { range = [range]; } - ran = range.map(typeConvert.Range.to); + if (this._canFormatMultipleRanges) { + if (!Array.isArray(options.ranges)) { throw new Error('Provided no list of ranges to multiple range provider'); } + + options.ranges = options.ranges.map(typeConvert.Range.to) as any; } else { - if (Array.isArray(range)) { throw new Error('Provided list of ranges to non-multiRange provider'); } - ran = typeConvert.Range.to(range); + if (Array.isArray(range)) { throw new Error('Provided list of ranges to single range provider'); } } - const value = await this._provider.provideDocumentRangeFormattingEdits(document, ran, options, token); + const document = this._documents.getDocument(resource); + const ran = typeConvert.Range.to(range); + + const value = await this._provider.provideDocumentRangeFormattingEdits(document, ran, options, token); if (Array.isArray(value)) { return value.map(typeConvert.TextEdit.from); } @@ -1730,9 +1731,9 @@ class DocumentOnDropEditAdapter { } } -type Adapter = DocumentSymbolAdapter | CodeLensAdapter | DefinitionAdapter | HoverAdapter +type Adapter = DocumentSymbolAdapter | CodeLensAdapter | DefinitionAdapter | HoverAdapter | DocumentHighlightAdapter | ReferenceAdapter | CodeActionAdapter | DocumentPasteEditProvider | DocumentFormattingAdapter - | RangeFormattingAdapter | OnTypeFormattingAdapter | NavigateTypeAdapter | RenameAdapter + | RangeFormattingAdapter | OnTypeFormattingAdapter | NavigateTypeAdapter | RenameAdapter | CompletionsAdapter | SignatureHelpAdapter | LinkProviderAdapter | ImplementationAdapter | TypeDefinitionAdapter | ColorProviderAdapter | FoldingProviderAdapter | DeclarationAdapter | SelectionRangeAdapter | CallHierarchyAdapter | TypeHierarchyAdapter @@ -1823,7 +1824,7 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF return result; } - private _addNewAdapter(adapter: Adapter, extension: IExtensionDescription): number { + private _addNewAdapter(adapter: Adapter, extension: IExtensionDescription): number { const handle = this._nextHandle(); this._adapter.set(handle, new AdapterData(adapter, extension)); return handle; @@ -2049,13 +2050,13 @@ export class ExtHostLanguageFeatures implements extHostProtocol.ExtHostLanguageF return this._withAdapter(handle, DocumentFormattingAdapter, adapter => adapter.provideDocumentFormattingEdits(URI.revive(resource), options, token), undefined, token); } - registerDocumentRangeFormattingEditProvider(extension: IExtensionDescription, selector: vscode.DocumentSelector, provider: vscode.DocumentRangeFormattingEditProvider, metadata?: vscode.DocumentRangeFormattingEditProviderMetadata): vscode.Disposable { - const handle = this._addNewAdapter(new RangeFormattingAdapter(this._documents, provider, metadata?.multiRange ?? false), extension); - this._proxy.$registerRangeFormattingSupport(handle, this._transformDocumentSelector(selector), extension.identifier, extension.displayName || extension.name, metadata ?? { multiRange: false }); + registerDocumentRangeFormattingEditProvider(extension: IExtensionDescription, selector: vscode.DocumentSelector, provider: vscode.DocumentRangeFormattingEditProvider, metadata?: vscode.DocumentRangeFormattingEditProviderMetadata): vscode.Disposable { + const handle = this._addNewAdapter(new RangeFormattingAdapter(this._documents, provider, metadata?.canFormatMultipleRanges ?? false), extension); + this._proxy.$registerRangeFormattingSupport(handle, this._transformDocumentSelector(selector), extension.identifier, extension.displayName || extension.name, metadata ?? { canFormatMultipleRanges: false }); return this._createDisposable(handle); } - $provideDocumentRangeFormattingEdits(handle: number, resource: UriComponents, range: IRange | IRange[], options: languages.FormattingOptions, token: CancellationToken): Promise { + $provideDocumentRangeFormattingEdits(handle: number, resource: UriComponents, range: IRange, options: languages.FormattingOptions, token: CancellationToken): Promise { return this._withAdapter(handle, RangeFormattingAdapter, adapter => adapter.provideDocumentRangeFormattingEdits(URI.revive(resource), range, options, token), undefined, token); } diff --git a/src/vscode-dts/vscode.d.ts b/src/vscode-dts/vscode.d.ts index 0a100f664dd76..33deba6c8a583 100644 --- a/src/vscode-dts/vscode.d.ts +++ b/src/vscode-dts/vscode.d.ts @@ -4089,10 +4089,15 @@ declare module 'vscode' { */ insertSpaces: boolean; + /** + * The list of multiple ranges to format at once, if the provider supports it. + */ + ranges?: Range[]; + /** * Signature for further properties. */ - [key: string]: boolean | number | string; + [key: string]: boolean | number | string | Range[] | undefined; } /** @@ -4117,7 +4122,7 @@ declare module 'vscode' { * The document formatting provider interface defines the contract between extensions and * the formatting-feature. */ - export interface DocumentRangeFormattingEditProvider { + export interface DocumentRangeFormattingEditProvider { /** * Provide formatting edits for a range in a document. @@ -4127,13 +4132,13 @@ declare module 'vscode' { * of the range to full syntax nodes. * * @param document The document in which the command was invoked. - * @param range The range or ranges which should be formatted. + * @param range The range which should be formatted. * @param options Options controlling formatting. * @param token A cancellation token. * @return A set of text edits or a thenable that resolves to such. The lack of a result can be * signaled by returning `undefined`, `null`, or an empty array. */ - provideDocumentRangeFormattingEdits(document: TextDocument, range: T, options: FormattingOptions, token: CancellationToken): ProviderResult; + provideDocumentRangeFormattingEdits(document: TextDocument, range: Range, options: FormattingOptions, token: CancellationToken): ProviderResult; } /** @@ -4143,7 +4148,7 @@ declare module 'vscode' { /** * `true` if the range formatting provider supports formatting multiple ranges at once. */ - readonly multiRange?: boolean; + readonly canFormatMultipleRanges?: boolean; } /**