Use more explicit states for code actions model

Switch to use a more explicit, more state-machine like approach to states for code actions

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);
 		}
 	}
 

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<CodeAction[] | undefined> {
 		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<CodeAction[]> | 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<CodeAction[]>,
+	) { }
 }
 
+export type CodeActionsState = typeof CodeActionsEmptyState | CodeActionsTriggeredState;
+
 export class CodeActionModel {
 
 	private _editor: ICodeEditor;
 	private _markerService: IMarkerService;
 	private _codeActionOracle?: CodeActionOracle;
-	private _onDidChangeFixes = new Emitter<CodeActionsComputeEvent>();
+	private _onDidChangeState = new Emitter<CodeActionsState>();
 	private _disposables: IDisposable[] = [];
 	private readonly _supportedCodeActions: IContextKey<string>;
 
@@ -188,8 +187,8 @@ export class CodeActionModel {
 		dispose(this._codeActionOracle);
 	}
 
-	get onDidChangeFixes(): Event<CodeActionsComputeEvent> {
-		return this._onDidChangeFixes.event;
+	get onDidChangeState(): Event<CodeActionsState> {
+		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();

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);
 	}

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 = <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;