Skip to content

Commit 36c6c83

Browse files
mjbvzmjbvz
committed
Use more explicit states for code actions model
Switch to use a more explicit, more state-machine like approach to states for code actions
1 parent e3573bd commit 36c6c83

4 files changed

Lines changed: 76 additions & 83 deletions

File tree

‎src/vs/editor/contrib/codeAction/codeActionCommands.ts‎

Lines changed: 29 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ import { IContextMenuService } from 'vs/platform/contextview/browser/contextView
2121
import { IKeybindingService } from 'vs/platform/keybinding/common/keybinding';
2222
import { IMarkerService } from 'vs/platform/markers/common/markers';
2323
import { IProgressService } from 'vs/platform/progress/common/progress';
24-
import { CodeActionModel, CodeActionsComputeEvent, SUPPORTED_CODE_ACTIONS } from './codeActionModel';
24+
import { CodeActionModel, SUPPORTED_CODE_ACTIONS, CodeActionsState, CodeActionsTriggeredState } from './codeActionModel';
2525
import { CodeActionAutoApply, CodeActionFilter, CodeActionKind } from './codeActionTrigger';
2626
import { CodeActionContextMenu } from './codeActionWidget';
2727
import { LightBulbWidget } from './lightBulbWidget';
@@ -69,7 +69,7 @@ export class QuickFixController implements IEditorContribution {
6969
this._disposables.push(
7070
this._codeActionContextMenu.onDidExecuteCodeAction(_ => this._model.trigger({ type: 'auto', filter: {} })),
7171
this._lightBulbWidget.onClick(this._handleLightBulbSelect, this),
72-
this._model.onDidChangeFixes(e => this._onCodeActionsEvent(e)),
72+
this._model.onDidChangeState(e => this._onDidChangeCodeActionsState(e)),
7373
this._keybindingService.onDidUpdateKeybindings(this._updateLightBulbTitle, this)
7474
);
7575
}
@@ -79,56 +79,49 @@ export class QuickFixController implements IEditorContribution {
7979
dispose(this._disposables);
8080
}
8181

82-
private _onCodeActionsEvent(e: CodeActionsComputeEvent): void {
82+
private _onDidChangeCodeActionsState(newState: CodeActionsState): void {
8383
if (this._activeRequest) {
8484
this._activeRequest.cancel();
8585
this._activeRequest = undefined;
8686
}
8787

88-
const actions = e && e.actions;
89-
if (actions) {
90-
this._activeRequest = e.actions;
91-
}
92-
93-
if (actions && e.trigger.filter && e.trigger.filter.kind) {
94-
// Triggered for specific scope
95-
// Apply if we only have one action or requested autoApply, otherwise show menu
96-
actions.then(fixes => {
97-
if (fixes.length > 0 && e.trigger.autoApply === CodeActionAutoApply.First || (e.trigger.autoApply === CodeActionAutoApply.IfSingle && fixes.length === 1)) {
98-
this._onApplyCodeAction(fixes[0]);
88+
if (newState.type === CodeActionsTriggeredState.type) {
89+
this._activeRequest = newState.actions;
90+
91+
if (newState.trigger.filter && newState.trigger.filter.kind) {
92+
// Triggered for specific scope
93+
// Apply if we only have one action or requested autoApply, otherwise show menu
94+
newState.actions.then(fixes => {
95+
if (fixes.length > 0 && newState.trigger.autoApply === CodeActionAutoApply.First || (newState.trigger.autoApply === CodeActionAutoApply.IfSingle && fixes.length === 1)) {
96+
this._onApplyCodeAction(fixes[0]);
97+
} else {
98+
this._codeActionContextMenu.show(newState.actions, newState.position);
99+
}
100+
}).catch(onUnexpectedError);
101+
} else if (newState.trigger.type === 'manual') {
102+
this._codeActionContextMenu.show(newState.actions, newState.position);
103+
} else {
104+
// auto magically triggered
105+
// * update an existing list of code actions
106+
// * manage light bulb
107+
if (this._codeActionContextMenu.isVisible) {
108+
this._codeActionContextMenu.show(newState.actions, newState.position);
99109
} else {
100-
this._codeActionContextMenu.show(actions, e.position);
110+
this._lightBulbWidget.state = newState;
101111
}
102-
}).catch(onUnexpectedError);
103-
return;
104-
}
105-
106-
if (e && e.trigger.type === 'manual') {
107-
if (actions) {
108-
this._codeActionContextMenu.show(actions, e.position);
109-
return;
110-
}
111-
} else if (actions) {
112-
// auto magically triggered
113-
// * update an existing list of code actions
114-
// * manage light bulb
115-
if (this._codeActionContextMenu.isVisible) {
116-
this._codeActionContextMenu.show(actions, e.position);
117-
} else {
118-
this._lightBulbWidget.model = e;
119112
}
120-
return;
113+
} else {
114+
this._lightBulbWidget.hide();
121115
}
122-
this._lightBulbWidget.hide();
123116
}
124117

125118
public getId(): string {
126119
return QuickFixController.ID;
127120
}
128121

129122
private _handleLightBulbSelect(coords: { x: number, y: number }): void {
130-
if (this._lightBulbWidget.model && this._lightBulbWidget.model.actions) {
131-
this._codeActionContextMenu.show(this._lightBulbWidget.model.actions, coords);
123+
if (this._lightBulbWidget.state.type === CodeActionsTriggeredState.type) {
124+
this._codeActionContextMenu.show(this._lightBulbWidget.state.actions, coords);
132125
}
133126
}
134127

‎src/vs/editor/contrib/codeAction/codeActionModel.ts‎

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ export class CodeActionOracle {
2828
constructor(
2929
private _editor: ICodeEditor,
3030
private readonly _markerService: IMarkerService,
31-
private _signalChange: (e: CodeActionsComputeEvent) => any,
31+
private _signalChange: (newState: CodeActionsState) => void,
3232
private readonly _delay: number = 250,
3333
private readonly _progressService?: IProgressService,
3434
) {
@@ -115,23 +115,13 @@ export class CodeActionOracle {
115115
private _createEventAndSignalChange(trigger: CodeActionTrigger, selection: Selection | undefined): Promise<CodeAction[] | undefined> {
116116
if (!selection) {
117117
// cancel
118-
this._signalChange({
119-
trigger,
120-
rangeOrSelection: undefined,
121-
position: undefined,
122-
actions: undefined,
123-
});
118+
this._signalChange(CodeActionsEmptyState);
124119
return Promise.resolve(undefined);
125120
} else {
126121
const model = this._editor.getModel();
127122
if (!model) {
128123
// cancel
129-
this._signalChange({
130-
trigger,
131-
rangeOrSelection: undefined,
132-
position: undefined,
133-
actions: undefined,
134-
});
124+
this._signalChange(CodeActionsEmptyState);
135125
return Promise.resolve(undefined);
136126
}
137127

@@ -143,30 +133,39 @@ export class CodeActionOracle {
143133
this._progressService.showWhile(actions, 250);
144134
}
145135

146-
this._signalChange({
136+
this._signalChange(new CodeActionsTriggeredState(
147137
trigger,
148-
rangeOrSelection: selection,
138+
selection,
149139
position,
150140
actions
151-
});
141+
));
152142
return actions;
153143
}
154144
}
155145
}
156146

157-
export interface CodeActionsComputeEvent {
158-
trigger: CodeActionTrigger;
159-
rangeOrSelection: Range | Selection | undefined;
160-
position: Position | undefined;
161-
actions: CancelablePromise<CodeAction[]> | undefined;
147+
export const CodeActionsEmptyState = new class { readonly type = 'empty'; };
148+
149+
export class CodeActionsTriggeredState {
150+
static readonly type = 'triggered';
151+
readonly type = CodeActionsTriggeredState.type;
152+
153+
constructor(
154+
public readonly trigger: CodeActionTrigger,
155+
public readonly rangeOrSelection: Range | Selection,
156+
public readonly position: Position,
157+
public readonly actions: CancelablePromise<CodeAction[]>,
158+
) { }
162159
}
163160

161+
export type CodeActionsState = typeof CodeActionsEmptyState | CodeActionsTriggeredState;
162+
164163
export class CodeActionModel {
165164

166165
private _editor: ICodeEditor;
167166
private _markerService: IMarkerService;
168167
private _codeActionOracle?: CodeActionOracle;
169-
private _onDidChangeFixes = new Emitter<CodeActionsComputeEvent>();
168+
private _onDidChangeState = new Emitter<CodeActionsState>();
170169
private _disposables: IDisposable[] = [];
171170
private readonly _supportedCodeActions: IContextKey<string>;
172171

@@ -188,22 +187,23 @@ export class CodeActionModel {
188187
dispose(this._codeActionOracle);
189188
}
190189

191-
get onDidChangeFixes(): Event<CodeActionsComputeEvent> {
192-
return this._onDidChangeFixes.event;
190+
get onDidChangeState(): Event<CodeActionsState> {
191+
return this._onDidChangeState.event;
193192
}
194193

195194
private _update(): void {
196195

197196
if (this._codeActionOracle) {
198197
this._codeActionOracle.dispose();
199198
this._codeActionOracle = undefined;
200-
this._onDidChangeFixes.fire(undefined);
199+
this._onDidChangeState.fire(CodeActionsEmptyState);
201200
}
202201

203202
const model = this._editor.getModel();
204203
if (model
205204
&& CodeActionProviderRegistry.has(model)
206-
&& !this._editor.getConfiguration().readOnly) {
205+
&& !this._editor.getConfiguration().readOnly
206+
) {
207207

208208
const supportedActions: string[] = [];
209209
for (const provider of CodeActionProviderRegistry.all(model)) {
@@ -214,7 +214,7 @@ export class CodeActionModel {
214214

215215
this._supportedCodeActions.set(supportedActions.join(' '));
216216

217-
this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, p => this._onDidChangeFixes.fire(p), undefined, this._progressService);
217+
this._codeActionOracle = new CodeActionOracle(this._editor, this._markerService, newState => this._onDidChangeState.fire(newState), undefined, this._progressService);
218218
this._codeActionOracle.trigger({ type: 'auto' });
219219
} else {
220220
this._supportedCodeActions.reset();

‎src/vs/editor/contrib/codeAction/lightBulbWidget.ts‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import { dispose, IDisposable } from 'vs/base/common/lifecycle';
1111
import 'vs/css!./lightBulbWidget';
1212
import { ContentWidgetPositionPreference, ICodeEditor, IContentWidget, IContentWidgetPosition } from 'vs/editor/browser/editorBrowser';
1313
import { TextModel } from 'vs/editor/common/model/textModel';
14-
import { CodeActionsComputeEvent } from './codeActionModel';
14+
import { CodeActionsState, CodeActionsEmptyState, CodeActionsTriggeredState } from './codeActionModel';
1515

1616
export class LightBulbWidget implements IDisposable, IContentWidget {
1717

@@ -25,7 +25,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
2525
readonly onClick: Event<{ x: number, y: number }> = this._onClick.event;
2626

2727
private _position: IContentWidgetPosition | null;
28-
private _model: CodeActionsComputeEvent | null;
28+
private _state: CodeActionsState = CodeActionsEmptyState;
2929
private _futureFixes = new CancellationTokenSource();
3030

3131
constructor(editor: ICodeEditor) {
@@ -40,7 +40,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
4040
this._disposables.push(this._editor.onDidChangeModelContent(_ => {
4141
// cancel when the line in question has been removed
4242
const editorModel = this._editor.getModel();
43-
if (!this.model || !this.model.position || !editorModel || this.model.position.lineNumber >= editorModel.getLineCount()) {
43+
if (this.state.type !== CodeActionsTriggeredState.type || !editorModel || this.state.position.lineNumber >= editorModel.getLineCount()) {
4444
this._futureFixes.cancel();
4545
}
4646
}));
@@ -53,7 +53,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
5353
const { lineHeight } = this._editor.getConfiguration();
5454

5555
let pad = Math.floor(lineHeight / 3);
56-
if (this._position && this._model && this._model.position && this._position.position !== null && this._position.position.lineNumber < this._model.position.lineNumber) {
56+
if (this._position && this._state.type === CodeActionsTriggeredState.type && this._position.position !== null && this._position.position.lineNumber < this._state.position.lineNumber) {
5757
pad += lineHeight;
5858
}
5959

@@ -100,9 +100,9 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
100100
return this._position;
101101
}
102102

103-
set model(value: CodeActionsComputeEvent | null) {
103+
set state(newState: CodeActionsState) {
104104

105-
if (!value || this._position && (!value.position || this._position.position && this._position.position.lineNumber !== value.position.lineNumber)) {
105+
if (newState.type !== 'triggered' || this._position && (!newState.position || this._position.position && this._position.position.lineNumber !== newState.position.lineNumber)) {
106106
// hide when getting a 'hide'-request or when currently
107107
// showing on another line
108108
this.hide();
@@ -113,14 +113,14 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
113113

114114
this._futureFixes = new CancellationTokenSource();
115115
const { token } = this._futureFixes;
116-
this._model = value;
116+
this._state = newState;
117117

118-
if (!this._model || !this._model.actions) {
118+
if (this._state.type === CodeActionsEmptyState.type) {
119119
return;
120120
}
121121

122-
const selection = this._model.rangeOrSelection;
123-
this._model.actions.then(fixes => {
122+
const selection = this._state.rangeOrSelection;
123+
this._state.actions.then(fixes => {
124124
if (!token.isCancellationRequested && fixes && fixes.length > 0 && selection) {
125125
this._show();
126126
} else {
@@ -131,8 +131,8 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
131131
});
132132
}
133133

134-
get model(): CodeActionsComputeEvent | null {
135-
return this._model;
134+
get state(): CodeActionsState {
135+
return this._state;
136136
}
137137

138138
set title(value: string) {
@@ -148,10 +148,10 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
148148
if (!config.contribInfo.lightbulbEnabled) {
149149
return;
150150
}
151-
if (!this._model || !this._model.position) {
151+
if (this._state.type !== CodeActionsTriggeredState.type) {
152152
return;
153153
}
154-
const { lineNumber, column } = this._model.position;
154+
const { lineNumber, column } = this._state.position;
155155
const model = this._editor.getModel();
156156
if (!model) {
157157
return;
@@ -188,7 +188,7 @@ export class LightBulbWidget implements IDisposable, IContentWidget {
188188

189189
hide(): void {
190190
this._position = null;
191-
this._model = null;
191+
this._state = CodeActionsEmptyState;
192192
this._futureFixes.cancel();
193193
this._editor.layoutContentWidget(this);
194194
}

‎src/vs/editor/contrib/codeAction/test/codeActionModel.test.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { ICodeEditor } from 'vs/editor/browser/editorBrowser';
1010
import { Selection } from 'vs/editor/common/core/selection';
1111
import { TextModel } from 'vs/editor/common/model/textModel';
1212
import { CodeActionProviderRegistry, LanguageIdentifier } from 'vs/editor/common/modes';
13-
import { CodeActionOracle } from 'vs/editor/contrib/codeAction/codeActionModel';
13+
import { CodeActionOracle, CodeActionsTriggeredState } from 'vs/editor/contrib/codeAction/codeActionModel';
1414
import { createTestCodeEditor } from 'vs/editor/test/browser/testCodeEditor';
1515
import { MarkerService } from 'vs/platform/markers/common/markerService';
1616

@@ -47,7 +47,7 @@ suite('CodeAction', () => {
4747
const reg = CodeActionProviderRegistry.register(languageIdentifier.language, testProvider);
4848
disposables.push(reg);
4949

50-
const oracle = new CodeActionOracle(editor, markerService, e => {
50+
const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => {
5151
assert.equal(e.trigger.type, 'auto');
5252
assert.ok(e.actions);
5353

@@ -85,7 +85,7 @@ suite('CodeAction', () => {
8585

8686
return new Promise((resolve, reject) => {
8787

88-
const oracle = new CodeActionOracle(editor, markerService, e => {
88+
const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => {
8989
assert.equal(e.trigger.type, 'auto');
9090
assert.ok(e.actions);
9191
e.actions!.then(fixes => {
@@ -120,7 +120,7 @@ suite('CodeAction', () => {
120120
// case 1 - drag selection over multiple lines -> range of enclosed marker, position or marker
121121
await new Promise(resolve => {
122122

123-
let oracle = new CodeActionOracle(editor, markerService, e => {
123+
let oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => {
124124
assert.equal(e.trigger.type, 'auto');
125125
const selection = <Selection>e.rangeOrSelection;
126126
assert.deepEqual(selection.selectionStartLineNumber, 1);
@@ -142,7 +142,7 @@ suite('CodeAction', () => {
142142
disposables.push(reg);
143143

144144
let triggerCount = 0;
145-
const oracle = new CodeActionOracle(editor, markerService, e => {
145+
const oracle = new CodeActionOracle(editor, markerService, (e: CodeActionsTriggeredState) => {
146146
assert.equal(e.trigger.type, 'auto');
147147
++triggerCount;
148148

0 commit comments

Comments
 (0)