Merge pull request #76917 from mjbvz/fixes-76851

Fix lifecycle for code actions that are updated while the code action menu is already showing
microsoft · Jul 8, 2019 · 2213894 · 2213894
2 parents a3d6fcf + efac204
commit 2213894
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 15 deletions.
diff --git a/src/vs/editor/contrib/codeAction/codeActionCommands.ts b/src/vs/editor/contrib/codeAction/codeActionCommands.ts
@@ -4,7 +4,7 @@
  *--------------------------------------------------------------------------------------------*/
 
 import { KeyCode, KeyMod } from 'vs/base/common/keyCodes';
-import { Disposable, MutableDisposable } from 'vs/base/common/lifecycle';
+import { Disposable } from 'vs/base/common/lifecycle';
 import { escapeRegExpCharacters } from 'vs/base/common/strings';
 import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
 import { EditorAction, EditorCommand, ServicesAccessor } from 'vs/editor/browser/editorExtensions';
@@ -26,7 +26,6 @@ import { CodeActionWidget } from './codeActionWidget';
 import { LightBulbWidget } from './lightBulbWidget';
 import { KeybindingWeight } from 'vs/platform/keybinding/common/keybindingsRegistry';
 import { onUnexpectedError } from 'vs/base/common/errors';
-import { CodeActionSet } from 'vs/editor/contrib/codeAction/codeAction';
 
 function contextKeyForSupportedActions(kind: CodeActionKind) {
 	return ContextKeyExpr.regex(
@@ -46,7 +45,6 @@ export class QuickFixController extends Disposable implements IEditorContributio
 	private readonly _model: CodeActionModel;
 	private readonly _codeActionWidget: CodeActionWidget;
 	private readonly _lightBulbWidget: LightBulbWidget;
-	private readonly _currentCodeActions = this._register(new MutableDisposable<CodeActionSet>());
 
 	constructor(
 		editor: ICodeEditor,
@@ -81,29 +79,30 @@ export class QuickFixController extends Disposable implements IEditorContributio
 		this._register(this._keybindingService.onDidUpdateKeybindings(this._updateLightBulbTitle, this));
 	}
 
-
 	private _onDidChangeCodeActionsState(newState: CodeActionsState.State): void {
 		if (newState.type === CodeActionsState.Type.Triggered) {
 			newState.actions.then(actions => {
-				this._currentCodeActions.value = actions;
-
 				if (!actions.actions.length && newState.trigger.context) {
 					MessageController.get(this._editor).showMessage(newState.trigger.context.notAvailableMessage, newState.trigger.context.position);
+					actions.dispose();
 				}
 			});
 
 			if (newState.trigger.filter && newState.trigger.filter.kind) {
 				// Triggered for specific scope
-				newState.actions.then(codeActions => {
+				newState.actions.then(async codeActions => {
 					if (codeActions.actions.length > 0) {
 						// Apply if we only have one action or requested autoApply
 						if (newState.trigger.autoApply === CodeActionAutoApply.First || (newState.trigger.autoApply === CodeActionAutoApply.IfSingle && codeActions.actions.length === 1)) {
-							this._applyCodeAction(codeActions.actions[0]);
+							try {
+								await this._applyCodeAction(codeActions.actions[0]);
+							} finally {
+								codeActions.dispose();
+							}
 							return;
 						}
 					}
 					this._codeActionWidget.show(newState.actions, newState.position);
-
 				}).catch(onUnexpectedError);
 			} else if (newState.trigger.type === 'manual') {
 				this._codeActionWidget.show(newState.actions, newState.position);
@@ -118,7 +117,6 @@ export class QuickFixController extends Disposable implements IEditorContributio
 				}
 			}
 		} else {
-			this._currentCodeActions.clear();
 			this._lightBulbWidget.hide();
 		}
 	}

diff --git a/src/vs/editor/contrib/codeAction/codeActionWidget.ts b/src/vs/editor/contrib/codeAction/codeActionWidget.ts
@@ -12,35 +12,49 @@ import { ScrollType } from 'vs/editor/common/editorCommon';
 import { CodeAction } from 'vs/editor/common/modes';
 import { IContextMenuService } from 'vs/platform/contextview/browser/contextView';
 import { CodeActionSet } from 'vs/editor/contrib/codeAction/codeAction';
+import { Disposable, MutableDisposable } from 'vs/base/common/lifecycle';
 
 interface CodeActionWidgetDelegate {
 	onSelectCodeAction: (action: CodeAction) => Promise<any>;
 }
 
-export class CodeActionWidget {
+export class CodeActionWidget extends Disposable {
 
 	private _visible: boolean;
+	private readonly _showingActions = this._register(new MutableDisposable<CodeActionSet>());
 
 	constructor(
 		private readonly _editor: ICodeEditor,
 		private readonly _contextMenuService: IContextMenuService,
-		private readonly _delegate: CodeActionWidgetDelegate
-	) { }
+		private readonly _delegate: CodeActionWidgetDelegate,
+	) {
+		super();
+	}
 
 	public async show(actionsToShow: Promise<CodeActionSet>, at?: { x: number; y: number } | Position): Promise<void> {
-		const codeActions = await actionsToShow;
+		let codeActions: CodeActionSet | undefined = await actionsToShow;
 		if (!codeActions.actions.length) {
+			codeActions.dispose();
 			this._visible = false;
 			return;
 		}
 		if (!this._editor.getDomNode()) {
 			// cancel when editor went off-dom
 			this._visible = false;
+			codeActions.dispose();
 			return Promise.reject(canceled());
 		}
 
+		if (this._visible) {
+			// TODO: Figure out if we should update the showing menu?
+			codeActions.dispose();
+			return;
+		}
+
 		this._visible = true;
 		const actions = codeActions.actions.map(action => this.codeActionToAction(action));
+
+		this._showingActions.value = codeActions;
 		this._contextMenuService.showContextMenu({
 			getAnchor: () => {
 				if (Position.isIPosition(at)) {

diff --git a/src/vs/editor/contrib/codeAction/lightBulbWidget.ts b/src/vs/editor/contrib/codeAction/lightBulbWidget.ts
@@ -7,7 +7,7 @@ import * as dom from 'vs/base/browser/dom';
 import { GlobalMouseMoveMonitor, IStandardMouseMoveEventData, standardMouseMoveMerger } from 'vs/base/browser/globalMouseMoveMonitor';
 import { CancellationTokenSource } from 'vs/base/common/cancellation';
 import { Emitter } from 'vs/base/common/event';
-import { Disposable } from 'vs/base/common/lifecycle';
+import { Disposable, MutableDisposable } 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';
@@ -27,6 +27,7 @@ export class LightBulbWidget extends Disposable implements IContentWidget {
 	private _position: IContentWidgetPosition | null;
 	private _state: CodeActionsState.State = CodeActionsState.Empty;
 	private _futureFixes = new CancellationTokenSource();
+	private readonly _showingActions = this._register(new MutableDisposable<CodeActionSet>());
 
 	constructor(editor: ICodeEditor) {
 		super();
@@ -116,13 +117,15 @@ export class LightBulbWidget extends Disposable implements IContentWidget {
 			// cancel pending show request in any case
 			this._futureFixes.cancel();
 		}
+		this._showingActions.clear();
 
 		this._futureFixes = new CancellationTokenSource();
 		const { token } = this._futureFixes;
 		this._state = newState;
 
 		const selection = this._state.rangeOrSelection;
 		this._state.actions.then(fixes => {
+			this._showingActions.value = fixes;
 			if (!token.isCancellationRequested && fixes.actions.length > 0 && selection) {
 				this._show(fixes);
 			} else {