From 36c6c834a024e5ef7fb6c15cc535e73f0c406d8f Mon Sep 17 00:00:00 2001 From: Matt Bierner Date: Thu, 3 Jan 2019 15:25:47 -0800 Subject: [PATCH] Use more explicit states for code actions model Switch to use a more explicit, more state-machine like approach to states for code actions --- .../contrib/codeAction/codeActionCommands.ts | 65 +++++++++---------- .../contrib/codeAction/codeActionModel.ts | 54 +++++++-------- .../contrib/codeAction/lightBulbWidget.ts | 30 ++++----- .../codeAction/test/codeActionModel.test.ts | 10 +-- 4 files changed, 76 insertions(+), 83 deletions(-) diff --git a/src/vs/editor/contrib/codeAction/codeActionCommands.ts b/src/vs/editor/contrib/codeAction/codeActionCommands.ts index a5b3f684379..23402e3c3de 100644 --- a/src/vs/editor/contrib/codeAction/codeActionCommands.ts +++ b/src/vs/editor/contrib/codeAction/codeActionCommands.ts @@ -21,7 +21,7 @@ import { IContextMenuService } from 'vs/platform/contextview/browser/contextView import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding'; import { IMarkerService } from 'vs/platform/markers/common/markers'; import { IProgressService } from 'vs/platform/progress/common/progress'; -import { CodeActionModel, CodeActionsComputeEvent, SUPPORTED_CODE_ACTIONS } from './codeActionModel'; +import { CodeActionModel, SUPPORTED_CODE_ACTIONS, CodeActionsState, CodeActionsTriggeredState } from './codeActionModel'; import { CodeActionAutoApply, CodeActionFilter, CodeActionKind } from './codeActionTrigger'; import { CodeActionContextMenu } from './codeActionWidget'; import { LightBulbWidget } from './lightBulbWidget'; @@ -69,7 +69,7 @@ export class QuickFixController implements IEditorContribution { this._disposables.push( this._codeActionContextMenu.onDidExecuteCodeAction(_ => this._model.trigger({ type: 'auto', filter: {} })), this._lightBulbWidget.onClick(this._handleLightBulbSelect, this), - this._model.onDidChangeFixes(e => this._onCodeActionsEvent(e)), + this._model.onDidChangeState(e => this._onDidChangeCodeActionsState(e)), this._keybindingService.onDidUpdateKeybindings(this._updateLightBulbTitle, this) ); } @@ -79,47 +79,40 @@ export class QuickFixController implements IEditorContribution { dispose(this._disposables); } - private _onCodeActionsEvent(e: CodeActionsComputeEvent): void { + private _onDidChangeCodeActionsState(newState: CodeActionsState): void { if (this._activeRequest) { this._activeRequest.cancel(); this._activeRequest = undefined; } - const actions = e && e.actions; - if (actions) { - this._activeRequest = e.actions; - } - - if (actions && e.trigger.filter && e.trigger.filter.kind) { - // Triggered for specific scope - // Apply if we only have one action or requested autoApply, otherwise show menu - actions.then(fixes => { - if (fixes.length > 0 && e.trigger.autoApply === CodeActionAutoApply.First || (e.trigger.autoApply === CodeActionAutoApply.IfSingle && fixes.length === 1)) { - this._onApplyCodeAction(fixes[0]); + if (newState.type === CodeActionsTriggeredState.type) { + this._activeRequest = newState.actions; + + if (newState.trigger.filter && newState.trigger.filter.kind) { + // Triggered for specific scope + // Apply if we only have one action or requested autoApply, otherwise show menu + newState.actions.then(fixes => { + if (fixes.length > 0 && newState.trigger.autoApply === CodeActionAutoApply.First || (newState.trigger.autoApply === CodeActionAutoApply.IfSingle && fixes.length === 1)) { + this._onApplyCodeAction(fixes[0]); + } else { + this._codeActionContextMenu.show(newState.actions, newState.position); + } + }).catch(onUnexpectedError); + } else if (newState.trigger.type === 'manual') { + this._codeActionContextMenu.show(newState.actions, newState.position); + } else { + // auto magically triggered + // * update an existing list of code actions + // * manage light bulb + if (this._codeActionContextMenu.isVisible) { + this._codeActionContextMenu.show(newState.actions, newState.position); } else { - this._codeActionContextMenu.show(actions, e.position); + this._lightBulbWidget.state = newState; } - }).catch(onUnexpectedError); - return; - } - - if (e && e.trigger.type === 'manual') { - if (actions) { - this._codeActionContextMenu.show(actions, e.position); - return; - } - } else if (actions) { - // auto magically triggered - // * update an existing list of code actions - // * manage light bulb - if (this._codeActionContextMenu.isVisible) { - this._codeActionContextMenu.show(actions, e.position); - } else { - this._lightBulbWidget.model = e; } - return; + } else { + this._lightBulbWidget.hide(); } - this._lightBulbWidget.hide(); } public getId(): string { @@ -127,8 +120,8 @@ export class QuickFixController implements IEditorContribution { } private _handleLightBulbSelect(coords: { x: number, y: number }): void { - if (this._lightBulbWidget.model && this._lightBulbWidget.model.actions) { - this._codeActionContextMenu.show(this._lightBulbWidget.model.actions, coords); + if (this._lightBulbWidget.state.type === CodeActionsTriggeredState.type) { + this._codeActionContextMenu.show(this._lightBulbWidget.state.actions, coords); } } diff --git a/src/vs/editor/contrib/codeAction/codeActionModel.ts b/src/vs/editor/contrib/codeAction/codeActionModel.ts index 84d29a8949f..11d2ad02dab 100644 --- a/src/vs/editor/contrib/codeAction/codeActionModel.ts +++ b/src/vs/editor/contrib/codeAction/codeActionModel.ts @@ -28,7 +28,7 @@ export class CodeActionOracle { constructor( private _editor: ICodeEditor, private readonly _markerService: IMarkerService, - private _signalChange: (e: CodeActionsComputeEvent) => any, + private _signalChange: (newState: CodeActionsState) => void, private readonly _delay: number = 250, private readonly _progressService?: IProgressService, ) { @@ -115,23 +115,13 @@ export class CodeActionOracle { private _createEventAndSignalChange(trigger: CodeActionTrigger, selection: Selection | undefined): Promise { if (!selection) { // cancel - this._signalChange({ - trigger, - rangeOrSelection: undefined, - position: undefined, - actions: undefined, - }); + this._signalChange(CodeActionsEmptyState); return Promise.resolve(undefined); } else { const model = this._editor.getModel(); if (!model) { // cancel - this._signalChange({ - trigger, - rangeOrSelection: undefined, - position: undefined, - actions: undefined, - }); + this._signalChange(CodeActionsEmptyState); return Promise.resolve(undefined); } @@ -143,30 +133,39 @@ export class CodeActionOracle { this._progressService.showWhile(actions, 250); } - this._signalChange({ + this._signalChange(new CodeActionsTriggeredState( trigger, - rangeOrSelection: selection, + selection, position, actions - }); + )); return actions; } } } -export interface CodeActionsComputeEvent { - trigger: CodeActionTrigger; - rangeOrSelection: Range | Selection | undefined; - position: Position | undefined; - actions: CancelablePromise | undefined; +export const CodeActionsEmptyState = new class { readonly type = 'empty'; }; + +export class CodeActionsTriggeredState { + static readonly type = 'triggered'; + readonly type = CodeActionsTriggeredState.type; + + constructor( + public readonly trigger: CodeActionTrigger, + public readonly rangeOrSelection: Range | Selection, + public readonly position: Position, + public readonly actions: CancelablePromise, + ) { } } +export type CodeActionsState = typeof CodeActionsEmptyState | CodeActionsTriggeredState; + export class CodeActionModel { private _editor: ICodeEditor; private _markerService: IMarkerService; private _codeActionOracle?: CodeActionOracle; - private _onDidChangeFixes = new Emitter(); + private _onDidChangeState = new Emitter(); private _disposables: IDisposable[] = []; private readonly _supportedCodeActions: IContextKey; @@ -188,8 +187,8 @@ export class CodeActionModel { dispose(this._codeActionOracle); } - get onDidChangeFixes(): Event { - return this._onDidChangeFixes.event; + get onDidChangeState(): Event { + return this._onDidChangeState.event; } private _update(): void { @@ -197,13 +196,14 @@ export class CodeActionModel { if (this._codeActionOracle) { this._codeActionOracle.dispose(); this._codeActionOracle = undefined; - this._onDidChangeFixes.fire(undefined); + this._onDidChangeState.fire(CodeActionsEmptyState); } const model = this._editor.getModel(); if (model && CodeActionProviderRegistry.has(model) - && !this._editor.getConfiguration().readOnly) { + && !this._editor.getConfiguration().readOnly + ) { const supportedActions: string[] = []; for (const provider of CodeActionProviderRegistry.all(model)) { @@ -214,7 +214,7 @@ export class CodeActionModel { this._supportedCodeActions.set(supportedActions.join(' ')); - this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, p => this._onDidChangeFixes.fire(p), undefined, this._progressService); + this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, newState => this._onDidChangeState.fire(newState), undefined, this._progressService); this._codeActionOracle.trigger({ type: 'auto' }); } else { this._supportedCodeActions.reset(); diff --git a/src/vs/editor/contrib/codeAction/lightBulbWidget.ts b/src/vs/editor/contrib/codeAction/lightBulbWidget.ts index ce8ef441c47..4b859a21cad 100644 --- a/src/vs/editor/contrib/codeAction/lightBulbWidget.ts +++ b/src/vs/editor/contrib/codeAction/lightBulbWidget.ts @@ -11,7 +11,7 @@ import { dispose, IDisposable } from 'vs/base/common/lifecycle'; import 'vs/css!./lightBulbWidget'; import { ContentWidgetPositionPreference, ICodeEditor, IContentWidget, IContentWidgetPosition } from 'vs/editor/browser/editorBrowser'; import { TextModel } from 'vs/editor/common/model/textModel'; -import { CodeActionsComputeEvent } from './codeActionModel'; +import { CodeActionsState, CodeActionsEmptyState, CodeActionsTriggeredState } from './codeActionModel'; export class LightBulbWidget implements IDisposable, IContentWidget { @@ -25,7 +25,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget { readonly onClick: Event<{ x: number, y: number }> = this._onClick.event; private _position: IContentWidgetPosition | null; - private _model: CodeActionsComputeEvent | null; + private _state: CodeActionsState = CodeActionsEmptyState; private _futureFixes = new CancellationTokenSource(); constructor(editor: ICodeEditor) { @@ -40,7 +40,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget { this._disposables.push(this._editor.onDidChangeModelContent(_ => { // cancel when the line in question has been removed const editorModel = this._editor.getModel(); - if (!this.model || !this.model.position || !editorModel || this.model.position.lineNumber >= editorModel.getLineCount()) { + if (this.state.type !== CodeActionsTriggeredState.type || !editorModel || this.state.position.lineNumber >= editorModel.getLineCount()) { this._futureFixes.cancel(); } })); @@ -53,7 +53,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget { const { lineHeight } = this._editor.getConfiguration(); let pad = Math.floor(lineHeight / 3); - if (this._position && this._model && this._model.position && this._position.position !== null && this._position.position.lineNumber < this._model.position.lineNumber) { + if (this._position && this._state.type === CodeActionsTriggeredState.type && this._position.position !== null && this._position.position.lineNumber < this._state.position.lineNumber) { pad += lineHeight; } @@ -100,9 +100,9 @@ export class LightBulbWidget implements IDisposable, IContentWidget { return this._position; } - set model(value: CodeActionsComputeEvent | null) { + set state(newState: CodeActionsState) { - if (!value || this._position && (!value.position || this._position.position && this._position.position.lineNumber !== value.position.lineNumber)) { + if (newState.type !== 'triggered' || this._position && (!newState.position || this._position.position && this._position.position.lineNumber !== newState.position.lineNumber)) { // hide when getting a 'hide'-request or when currently // showing on another line this.hide(); @@ -113,14 +113,14 @@ export class LightBulbWidget implements IDisposable, IContentWidget { this._futureFixes = new CancellationTokenSource(); const { token } = this._futureFixes; - this._model = value; + this._state = newState; - if (!this._model || !this._model.actions) { + if (this._state.type === CodeActionsEmptyState.type) { return; } - const selection = this._model.rangeOrSelection; - this._model.actions.then(fixes => { + const selection = this._state.rangeOrSelection; + this._state.actions.then(fixes => { if (!token.isCancellationRequested && fixes && fixes.length > 0 && selection) { this._show(); } else { @@ -131,8 +131,8 @@ export class LightBulbWidget implements IDisposable, IContentWidget { }); } - get model(): CodeActionsComputeEvent | null { - return this._model; + get state(): CodeActionsState { + return this._state; } set title(value: string) { @@ -148,10 +148,10 @@ export class LightBulbWidget implements IDisposable, IContentWidget { if (!config.contribInfo.lightbulbEnabled) { return; } - if (!this._model || !this._model.position) { + if (this._state.type !== CodeActionsTriggeredState.type) { return; } - const { lineNumber, column } = this._model.position; + const { lineNumber, column } = this._state.position; const model = this._editor.getModel(); if (!model) { return; @@ -188,7 +188,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget { hide(): void { this._position = null; - this._model = null; + this._state = CodeActionsEmptyState; this._futureFixes.cancel(); this._editor.layoutContentWidget(this); } diff --git a/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts b/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts index 98b9de1f317..05ea6cd32fd 100644 --- a/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts +++ b/src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts @@ -10,7 +10,7 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser'; import { Selection } from 'vs/editor/common/core/selection'; import { TextModel } from 'vs/editor/common/model/textModel'; import { CodeActionProviderRegistry, LanguageIdentifier } from 'vs/editor/common/modes'; -import { CodeActionOracle } from 'vs/editor/contrib/codeAction/codeActionModel'; +import { CodeActionOracle, CodeActionsTriggeredState } from 'vs/editor/contrib/codeAction/codeActionModel'; import { createTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor'; import { MarkerService } from 'vs/platform/markers/common/markerService'; @@ -47,7 +47,7 @@ suite('CodeAction', () => { const reg = CodeActionProviderRegistry.register(languageIdentifier.language, testProvider); disposables.push(reg); - const oracle = new CodeActionOracle(editor, markerService, e => { + const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => { assert.equal(e.trigger.type, 'auto'); assert.ok(e.actions); @@ -85,7 +85,7 @@ suite('CodeAction', () => { return new Promise((resolve, reject) => { - const oracle = new CodeActionOracle(editor, markerService, e => { + const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => { assert.equal(e.trigger.type, 'auto'); assert.ok(e.actions); e.actions!.then(fixes => { @@ -120,7 +120,7 @@ suite('CodeAction', () => { // case 1 - drag selection over multiple lines -> range of enclosed marker, position or marker await new Promise(resolve => { - let oracle = new CodeActionOracle(editor, markerService, e => { + let oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => { assert.equal(e.trigger.type, 'auto'); const selection = e.rangeOrSelection; assert.deepEqual(selection.selectionStartLineNumber, 1); @@ -142,7 +142,7 @@ suite('CodeAction', () => { disposables.push(reg); let triggerCount = 0; - const oracle = new CodeActionOracle(editor, markerService, e => { + const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => { assert.equal(e.trigger.type, 'auto'); ++triggerCount; -- GitLab