diff --git a/extensions/typescript-language-features/src/features/refactorProvider.ts b/extensions/typescript-language-features/src/features/refactorProvider.ts index 40807e22bbc..066d74adfdc 100644 --- a/extensions/typescript-language-features/src/features/refactorProvider.ts +++ b/extensions/typescript-language-features/src/features/refactorProvider.ts @@ -102,7 +102,7 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv public async provideCodeActions( document: vscode.TextDocument, - _range: vscode.Range, + rangeOrSelection: vscode.Range | vscode.Selection, context: vscode.CodeActionContext, token: vscode.CancellationToken ): Promise { @@ -114,22 +114,16 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv return []; } - if (!vscode.window.activeTextEditor) { + if (!(rangeOrSelection instanceof vscode.Selection) || rangeOrSelection.isEmpty) { return []; } - const editor = vscode.window.activeTextEditor; const file = this.client.normalizePath(document.uri); - if (!file || editor.document.uri.fsPath !== document.uri.fsPath) { + if (!file) { return []; } - if (editor.selection.isEmpty) { - return []; - } - - const range = editor.selection; - const args: Proto.GetApplicableRefactorsRequestArgs = typeConverters.Range.toFileRangeRequestArgs(file, range); + const args: Proto.GetApplicableRefactorsRequestArgs = typeConverters.Range.toFileRangeRequestArgs(file, rangeOrSelection); try { const response = await this.client.execute('getApplicableRefactors', args, token); if (!response || !response.body) { @@ -143,7 +137,7 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv codeAction.command = { title: info.description, command: SelectRefactorCommand.ID, - arguments: [document, file, info, range] + arguments: [document, file, info, rangeOrSelection] }; actions.push(codeAction); } else { @@ -152,7 +146,7 @@ export default class TypeScriptRefactorProvider implements vscode.CodeActionProv codeAction.command = { title: action.description, command: ApplyRefactoringCommand.ID, - arguments: [document, file, info.name, action.name, range] + arguments: [document, file, info.name, action.name, rangeOrSelection] }; actions.push(codeAction); } diff --git a/src/vs/editor/common/modes.ts b/src/vs/editor/common/modes.ts index ee5ac00ee25..97a3a1ab345 100644 --- a/src/vs/editor/common/modes.ts +++ b/src/vs/editor/common/modes.ts @@ -18,6 +18,7 @@ import { Color } from 'vs/base/common/color'; import { IMarkerData } from 'vs/platform/markers/common/markers'; import * as model from 'vs/editor/common/model'; import { isObject } from 'vs/base/common/types'; +import { Selection } from 'vs/editor/common/core/selection'; /** * Open ended enum at runtime @@ -363,7 +364,7 @@ export interface CodeActionProvider { /** * Provide commands for the given document and range. */ - provideCodeActions(model: model.ITextModel, range: Range, context: CodeActionContext, token: CancellationToken): CodeAction[] | Thenable; + provideCodeActions(model: model.ITextModel, range: Range | Selection, context: CodeActionContext, token: CancellationToken): CodeAction[] | Thenable; /** * Optional list of of CodeActionKinds that this provider returns. diff --git a/src/vs/editor/contrib/codeAction/codeAction.ts b/src/vs/editor/contrib/codeAction/codeAction.ts index 2a7e9fb9d30..595d156bfb8 100644 --- a/src/vs/editor/contrib/codeAction/codeAction.ts +++ b/src/vs/editor/contrib/codeAction/codeAction.ts @@ -11,15 +11,18 @@ import { TPromise } from 'vs/base/common/winjs.base'; import { registerLanguageCommand } from 'vs/editor/browser/editorExtensions'; import { Range } from 'vs/editor/common/core/range'; import { ITextModel } from 'vs/editor/common/model'; -import { CodeAction, CodeActionProviderRegistry } from 'vs/editor/common/modes'; +import { CodeAction, CodeActionProviderRegistry, CodeActionContext } from 'vs/editor/common/modes'; import { IModelService } from 'vs/editor/common/services/modelService'; import { CodeActionFilter, CodeActionKind } from './codeActionTrigger'; +import { Selection } from 'vs/editor/common/core/selection'; -export function getCodeActions(model: ITextModel, range: Range, filter?: CodeActionFilter): TPromise { - const codeActionContext = { only: filter && filter.kind ? filter.kind.value : undefined }; +export function getCodeActions(model: ITextModel, rangeOrSelection: Range | Selection, filter?: CodeActionFilter): TPromise { + const codeActionContext: CodeActionContext = { + only: filter && filter.kind ? filter.kind.value : undefined, + }; const promises = CodeActionProviderRegistry.all(model).map(support => { - return asWinJsPromise(token => support.provideCodeActions(model, range, codeActionContext, token)).then(providedCodeActions => { + return asWinJsPromise(token => support.provideCodeActions(model, rangeOrSelection, codeActionContext, token)).then(providedCodeActions => { if (!Array.isArray(providedCodeActions)) { return []; } @@ -80,5 +83,5 @@ registerLanguageCommand('_executeCodeActionProvider', function (accessor, args) throw illegalArgument(); } - return getCodeActions(model, model.validateRange(range)); + return getCodeActions(model, model.validateRange(range), undefined); }); diff --git a/src/vs/editor/contrib/codeAction/codeActionModel.ts b/src/vs/editor/contrib/codeAction/codeActionModel.ts index a369f31e28e..5dbd28fb43b 100644 --- a/src/vs/editor/contrib/codeAction/codeActionModel.ts +++ b/src/vs/editor/contrib/codeAction/codeActionModel.ts @@ -40,11 +40,8 @@ export class CodeActionOracle { } trigger(trigger: CodeActionTrigger) { - let rangeOrSelection = this._getRangeOfMarker() || this._getRangeOfSelectionUnlessWhitespaceEnclosed(); - if (!rangeOrSelection && trigger.type === 'manual') { - rangeOrSelection = this._editor.getSelection(); - } - return this._createEventAndSignalChange(trigger, rangeOrSelection); + const selection = this._getRangeOfSelectionUnlessWhitespaceEnclosed(); + return this._createEventAndSignalChange(trigger, selection); } private _onMarkerChanges(resources: URI[]): void { @@ -61,8 +58,7 @@ export class CodeActionOracle { this.trigger({ type: 'auto' }); } - private _getRangeOfMarker(): Range { - const selection = this._editor.getSelection(); + private _getRangeOfMarker(selection: Selection): Range { const model = this._editor.getModel(); for (const marker of this._markerService.read({ resource: model.uri })) { if (Range.intersectRanges(marker, selection)) { @@ -72,7 +68,7 @@ export class CodeActionOracle { return undefined; } - private _getRangeOfSelectionUnlessWhitespaceEnclosed(): Selection { + private _getRangeOfSelectionUnlessWhitespaceEnclosed(): Selection | undefined { const model = this._editor.getModel(); const selection = this._editor.getSelection(); if (selection.isEmpty()) { @@ -101,26 +97,25 @@ export class CodeActionOracle { return selection; } - private _createEventAndSignalChange(trigger: CodeActionTrigger, rangeOrSelection: Range | Selection): TPromise { - if (!rangeOrSelection) { + private _createEventAndSignalChange(trigger: CodeActionTrigger, selection: Selection | undefined): TPromise { + if (!selection) { // cancel this._signalChange({ trigger, - range: undefined, + rangeOrSelection: undefined, position: undefined, actions: undefined, }); return TPromise.as(undefined); } else { - // actual const model = this._editor.getModel(); - const range = model.validateRange(rangeOrSelection); - const position = rangeOrSelection instanceof Selection ? rangeOrSelection.getPosition() : rangeOrSelection.getStartPosition(); - const actions = getCodeActions(model, range, trigger && trigger.filter); + const markerRange = this._getRangeOfMarker(selection); + const position = markerRange ? markerRange.getStartPosition() : selection.getStartPosition(); + const actions = getCodeActions(model, selection, trigger && trigger.filter); this._signalChange({ trigger, - range, + rangeOrSelection: selection, position, actions }); @@ -131,7 +126,7 @@ export class CodeActionOracle { export interface CodeActionsComputeEvent { trigger: CodeActionTrigger; - range: Range; + rangeOrSelection: Range | Selection; position: Position; actions: TPromise; } diff --git a/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts b/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts index 5ecc2949f62..97ec8e79436 100644 --- a/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts +++ b/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts @@ -6,15 +6,14 @@ import * as assert from 'assert'; import URI from 'vs/base/common/uri'; -import { TPromise } from 'vs/base/common/winjs.base'; import { TextModel } from 'vs/editor/common/model/textModel'; import { createTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor'; import { MarkerService } from 'vs/platform/markers/common/markerService'; import { CodeActionOracle } from 'vs/editor/contrib/codeAction/codeActionModel'; import { CodeActionProviderRegistry, LanguageIdentifier } from 'vs/editor/common/modes'; import { IDisposable } from 'vs/base/common/lifecycle'; -import { Range } from 'vs/editor/common/core/range'; import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; +import { Selection } from 'vs/editor/common/core/selection'; suite('CodeAction', () => { @@ -96,49 +95,6 @@ suite('CodeAction', () => { }); }); - test('Oracle -> marker wins over selection', () => { - - let range: Range; - let reg = CodeActionProviderRegistry.register(languageIdentifier.language, { - provideCodeActions(doc, _range) { - range = _range; - return []; - } - }); - - markerService.changeOne('fake', uri, [{ - startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6, - message: 'error', - severity: 1, - code: '', - source: '' - }]); - - let fixes: TPromise[] = []; - let oracle = new CodeActionOracle(editor, markerService, e => { - fixes.push(e.actions); - }, 10); - - editor.setSelection({ startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 13 }); - - return TPromise.join([TPromise.timeout(20)].concat(fixes)).then(_ => { - - // -> marker wins - assert.deepEqual(range, { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6 }); - - // 'auto' triggered, non-empty selection BUT within a marker - editor.setSelection({ startLineNumber: 1, startColumn: 2, endLineNumber: 1, endColumn: 4 }); - - return TPromise.join([TPromise.timeout(20)].concat(fixes)).then(_ => { - reg.dispose(); - oracle.dispose(); - - // assert marker - assert.deepEqual(range, { startLineNumber: 1, startColumn: 1, endLineNumber: 1, endColumn: 6 }); - }); - }); - }); - test('Lightbulb is in the wrong place, #29933', async function () { let reg = CodeActionProviderRegistry.register(languageIdentifier.language, { provideCodeActions(doc, _range) { @@ -161,7 +117,11 @@ suite('CodeAction', () => { let oracle = new CodeActionOracle(editor, markerService, e => { assert.equal(e.trigger.type, 'auto'); - assert.deepEqual(e.range, { startLineNumber: 3, startColumn: 1, endLineNumber: 3, endColumn: 4 }); + const selection = e.rangeOrSelection; + assert.deepEqual(selection.selectionStartLineNumber, 1); + assert.deepEqual(selection.selectionStartColumn, 1); + assert.deepEqual(selection.endLineNumber, 4); + assert.deepEqual(selection.endColumn, 1); assert.deepEqual(e.position, { lineNumber: 3, column: 1 }); oracle.dispose(); diff --git a/src/vs/vscode.d.ts b/src/vs/vscode.d.ts index afba554a423..3733eb30b88 100644 --- a/src/vs/vscode.d.ts +++ b/src/vs/vscode.d.ts @@ -2053,13 +2053,14 @@ declare module 'vscode' { * Provide commands for the given document and range. * * @param document The document in which the command was invoked. - * @param range The range for which the command was invoked. + * @param range The selector or range for which the command was invoked. This will always be a selection if + * there is a currently active editor. * @param context Context carrying additional information. * @param token A cancellation token. * @return An array of commands, quick fixes, or refactorings or a thenable of such. The lack of a result can be * signaled by returning `undefined`, `null`, or an empty array. */ - provideCodeActions(document: TextDocument, range: Range, context: CodeActionContext, token: CancellationToken): ProviderResult<(Command | CodeAction)[]>; + provideCodeActions(document: TextDocument, range: Range | Selection, context: CodeActionContext, token: CancellationToken): ProviderResult<(Command | CodeAction)[]>; } /** diff --git a/src/vs/workbench/api/electron-browser/mainThreadLanguageFeatures.ts b/src/vs/workbench/api/electron-browser/mainThreadLanguageFeatures.ts index caabce24407..c25c6477b50 100644 --- a/src/vs/workbench/api/electron-browser/mainThreadLanguageFeatures.ts +++ b/src/vs/workbench/api/electron-browser/mainThreadLanguageFeatures.ts @@ -22,6 +22,7 @@ import { IModeService } from 'vs/editor/common/services/modeService'; import { extHostNamedCustomer } from 'vs/workbench/api/electron-browser/extHostCustomers'; import * as typeConverters from 'vs/workbench/api/node/extHostTypeConverters'; import URI from 'vs/base/common/uri'; +import { Selection } from 'vs/editor/common/core/selection'; @extHostNamedCustomer(MainContext.MainThreadLanguageFeatures) export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesShape { @@ -193,8 +194,8 @@ export class MainThreadLanguageFeatures implements MainThreadLanguageFeaturesSha $registerQuickFixSupport(handle: number, selector: ISerializedDocumentFilter[], providedCodeActionKinds?: string[]): void { this._registrations[handle] = modes.CodeActionProviderRegistry.register(typeConverters.LanguageSelector.from(selector), { - provideCodeActions: (model: ITextModel, range: EditorRange, context: modes.CodeActionContext, token: CancellationToken): Thenable => { - return this._heapService.trackRecursive(wireCancellationToken(token, this._proxy.$provideCodeActions(handle, model.uri, range, context))).then(MainThreadLanguageFeatures._reviveCodeActionDto); + provideCodeActions: (model: ITextModel, rangeOrSelection: EditorRange | Selection, context: modes.CodeActionContext, token: CancellationToken): Thenable => { + return this._heapService.trackRecursive(wireCancellationToken(token, this._proxy.$provideCodeActions(handle, model.uri, rangeOrSelection, context))).then(MainThreadLanguageFeatures._reviveCodeActionDto); }, providedCodeActionKinds }); diff --git a/src/vs/workbench/api/node/extHost.protocol.ts b/src/vs/workbench/api/node/extHost.protocol.ts index 22d0278a203..3e070763dee 100644 --- a/src/vs/workbench/api/node/extHost.protocol.ts +++ b/src/vs/workbench/api/node/extHost.protocol.ts @@ -720,7 +720,7 @@ export interface ExtHostLanguageFeaturesShape { $provideHover(handle: number, resource: UriComponents, position: IPosition): TPromise; $provideDocumentHighlights(handle: number, resource: UriComponents, position: IPosition): TPromise; $provideReferences(handle: number, resource: UriComponents, position: IPosition, context: modes.ReferenceContext): TPromise; - $provideCodeActions(handle: number, resource: UriComponents, range: IRange, context: modes.CodeActionContext): TPromise; + $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext): TPromise; $provideDocumentFormattingEdits(handle: number, resource: UriComponents, options: modes.FormattingOptions): TPromise; $provideDocumentRangeFormattingEdits(handle: number, resource: UriComponents, range: IRange, options: modes.FormattingOptions): TPromise; $provideOnTypeFormattingEdits(handle: number, resource: UriComponents, position: IPosition, ch: string, options: modes.FormattingOptions): TPromise; diff --git a/src/vs/workbench/api/node/extHostLanguageFeatures.ts b/src/vs/workbench/api/node/extHostLanguageFeatures.ts index 02888100968..a5c89164b99 100644 --- a/src/vs/workbench/api/node/extHostLanguageFeatures.ts +++ b/src/vs/workbench/api/node/extHostLanguageFeatures.ts @@ -23,6 +23,7 @@ import { IPosition } from 'vs/editor/common/core/position'; import { IRange } from 'vs/editor/common/core/range'; import { isFalsyOrEmpty } from 'vs/base/common/arrays'; import { isObject } from 'vs/base/common/types'; +import { ISelection, Selection } from 'vs/editor/common/core/selection'; // --- adapter @@ -267,14 +268,16 @@ class CodeActionAdapter { private readonly _provider: vscode.CodeActionProvider ) { } - provideCodeActions(resource: URI, range: IRange, context: modes.CodeActionContext): TPromise { + provideCodeActions(resource: URI, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext): TPromise { const doc = this._documents.getDocumentData(resource).document; - const ran = typeConvert.Range.to(range); + const ran = Selection.isISelection(rangeOrSelection) + ? typeConvert.Selection.to(rangeOrSelection) + : typeConvert.Range.to(rangeOrSelection); const allDiagnostics: vscode.Diagnostic[] = []; for (const diagnostic of this._diagnostics.getDiagnostics(resource)) { - if (ran.contains(diagnostic.range)) { + if (ran.intersection(diagnostic.range)) { allDiagnostics.push(diagnostic); } } @@ -283,6 +286,7 @@ class CodeActionAdapter { diagnostics: allDiagnostics, only: context.only ? new CodeActionKind(context.only) : undefined }; + return asWinJsPromise(token => this._provider.provideCodeActions(doc, ran, codeActionContext, token) ).then(commandsOrActions => { @@ -1037,8 +1041,8 @@ export class ExtHostLanguageFeatures implements ExtHostLanguageFeaturesShape { } - $provideCodeActions(handle: number, resource: UriComponents, range: IRange, context: modes.CodeActionContext): TPromise { - return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.provideCodeActions(URI.revive(resource), range, context)); + $provideCodeActions(handle: number, resource: UriComponents, rangeOrSelection: IRange | ISelection, context: modes.CodeActionContext): TPromise { + return this._withAdapter(handle, CodeActionAdapter, adapter => adapter.provideCodeActions(URI.revive(resource), rangeOrSelection, context)); } // --- formatting diff --git a/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts b/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts index 40eeeba5d9a..69c4b023ee8 100644 --- a/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts +++ b/src/vs/workbench/test/electron-browser/api/extHostLanguageFeatures.test.ts @@ -654,7 +654,7 @@ suite('ExtHostLanguageFeatures', function () { })); return rpcProtocol.sync().then(() => { - return getCodeActions(model, model.getFullModelRange()).then(value => { + return getCodeActions(model, model.getFullModelRange(), undefined).then(value => { assert.equal(value.length, 2); const [first, second] = value; @@ -681,7 +681,7 @@ suite('ExtHostLanguageFeatures', function () { })); return rpcProtocol.sync().then(() => { - return getCodeActions(model, model.getFullModelRange()).then(value => { + return getCodeActions(model, model.getFullModelRange(), undefined).then(value => { assert.equal(value.length, 1); const [first] = value; @@ -707,7 +707,7 @@ suite('ExtHostLanguageFeatures', function () { })); return rpcProtocol.sync().then(() => { - return getCodeActions(model, model.getFullModelRange()).then(value => { + return getCodeActions(model, model.getFullModelRange(), undefined).then(value => { assert.equal(value.length, 1); }); }); @@ -727,7 +727,7 @@ suite('ExtHostLanguageFeatures', function () { })); return rpcProtocol.sync().then(() => { - return getCodeActions(model, model.getFullModelRange()).then(value => { + return getCodeActions(model, model.getFullModelRange(), undefined).then(value => { assert.equal(value.length, 1); }); });