Update action run return type (#15568)

* Update action run return type

* fix tests

* Update rest

* Add back null checks
This commit is contained in:
Charles Gagnon
2021-05-25 14:52:39 -07:00
committed by GitHub
parent 25352fa39c
commit 399406b732
29 changed files with 379 additions and 637 deletions

View File

@@ -52,17 +52,9 @@ export class EditCellAction extends ToggleableAction {
this.toggle(value);
}
public run(context: CellContext): Promise<boolean> {
let self = this;
return new Promise<boolean>((resolve, reject) => {
try {
self.editMode = !self.editMode;
context.cell.isEditMode = self.editMode;
resolve(true);
} catch (e) {
reject(e);
}
});
public async run(context: CellContext): Promise<void> {
this.editMode = !this.editMode;
context.cell.isEditMode = this.editMode;
}
}
@@ -358,13 +350,12 @@ export class ToggleMoreActions extends Action {
super(ToggleMoreActions.ID, ToggleMoreActions.LABEL, ToggleMoreActions.ICON);
}
run(context: StandardKeyboardEvent): Promise<boolean> {
async run(context: StandardKeyboardEvent): Promise<void> {
this._contextMenuService.showContextMenu({
getAnchor: () => context.target,
getActions: () => this._actions,
getActionsContext: () => this._context
});
return Promise.resolve(true);
}
}

View File

@@ -53,11 +53,10 @@ export abstract class CellActionBase extends Action {
return true;
}
public run(context: CellContext): Promise<boolean> {
public async run(context: CellContext): Promise<void> {
if (hasModelAndCell(context, this.notificationService)) {
return this.doRun(context).then(() => true);
return this.doRun(context);
}
return Promise.resolve(true);
}
abstract doRun(context: CellContext): Promise<void>;
@@ -172,8 +171,8 @@ export class RunCellAction extends MultiStateAction<CellExecutionState> {
this.ensureContextIsUpdated(context);
}
public run(context?: CellContext): Promise<boolean> {
return this.doRun(context).then(() => true);
public async run(context?: CellContext): Promise<void> {
return this.doRun(context);
}
public async doRun(context: CellContext): Promise<void> {

View File

@@ -31,14 +31,13 @@ export class TransformMarkdownAction extends Action {
super(id, label, cssClass);
this._tooltip = tooltip;
}
public async run(context: any): Promise<boolean> {
public async run(context: any): Promise<void> {
if (!context?.cellModel?.showMarkdown && context?.cellModel?.showPreview) {
this.transformDocumentCommand();
} else {
let markdownTextTransformer = new MarkdownTextTransformer(this._notebookService, this._cellModel);
await markdownTextTransformer.transformText(this._type);
}
return true;
}
private transformDocumentCommand() {
@@ -607,7 +606,7 @@ export class ToggleViewAction extends Action {
this._tooltip = tooltip;
}
public async run(context: MarkdownToolbarComponent): Promise<boolean> {
public async run(context: MarkdownToolbarComponent): Promise<void> {
context.removeActiveClassFromModeActions();
this.class += ' active';
context.cellModel.showPreview = this.showPreview;
@@ -618,6 +617,5 @@ export class ToggleViewAction extends Action {
} else {
context.showLinkAndImageButtons();
}
return true;
}
}

View File

@@ -126,9 +126,9 @@ export class ClearAllOutputsAction extends TooltipFromLabelAction {
});
}
public run(context: URI): Promise<boolean> {
public async run(context: URI): Promise<void> {
const editor = this._notebookService.findNotebookEditor(context);
return editor.clearAllOutputs();
await editor.clearAllOutputs();
}
}
@@ -208,11 +208,10 @@ export class TrustedAction extends ToggleableAction {
this.toggle(value);
}
public async run(context: URI): Promise<boolean> {
public async run(context: URI): Promise<void> {
const editor = this._notebookService.findNotebookEditor(context);
this.trusted = !this.trusted;
editor.model.trustedMode = this.trusted;
return true;
}
}
@@ -226,15 +225,13 @@ export class RunAllCellsAction extends Action {
) {
super(id, label, cssClass);
}
public async run(context: URI): Promise<boolean> {
public async run(context: URI): Promise<void> {
try {
this._telemetryService.sendActionEvent(TelemetryKeys.TelemetryView.Notebook, TelemetryKeys.NbTelemetryAction.RunAll);
const editor = this._notebookService.findNotebookEditor(context);
await editor.runAllCells();
return true;
} catch (e) {
this.notificationService.error(getErrorMessage(e));
return false;
}
}
}
@@ -270,13 +267,12 @@ export class CollapseCellsAction extends ToggleableAction {
this.expanded = !value;
}
public async run(context: URI): Promise<boolean> {
public async run(context: URI): Promise<void> {
const editor = this._notebookService.findNotebookEditor(context);
this.setCollapsed(!this.isCollapsed);
editor.cells.forEach(cell => {
cell.isCollapsed = this.isCollapsed;
});
return true;
}
}

View File

@@ -557,7 +557,7 @@ export class NotebookChartAction extends ToggleableAction {
});
}
public async run(context: IGridActionContext): Promise<boolean> {
public async run(context: IGridActionContext): Promise<void> {
this.resourceTable.toggleChartVisibility();
this.toggle(!this.state.isOn);
if (this.state.isOn) {
@@ -573,6 +573,5 @@ export class NotebookChartAction extends ToggleableAction {
).send();
this.resourceTable.updateChartData(Math.min(rowCount, maxRowCount), columnCount, context.gridDataProvider);
}
return true;
}
}

View File

@@ -151,16 +151,14 @@ suite('Notebook Actions', function (): void {
// Normal use case
mockNotebookEditor.setup(c => c.clearAllOutputs()).returns(() => Promise.resolve(true));
let result = await action.run(testUri);
assert.ok(result, 'Clear All Outputs Action should succeed');
await action.run(testUri);
mockNotebookEditor.verify(c => c.clearAllOutputs(), TypeMoq.Times.once());
// Handle failure case
mockNotebookEditor.reset();
mockNotebookEditor.setup(c => c.clearAllOutputs()).returns(() => Promise.resolve(false));
result = await action.run(testUri);
assert.strictEqual(result, false, 'Clear All Outputs Action should have failed');
await action.run(testUri);
mockNotebookEditor.verify(c => c.clearAllOutputs(), TypeMoq.Times.once());
});
@@ -177,14 +175,12 @@ suite('Notebook Actions', function (): void {
mockNotebookEditor.setup(x => x.model).returns(() => testNotebookModel);
// Normal use case
let result = await action.run(testUri);
assert.ok(result, 'Trusted Action should succeed');
await action.run(testUri);
assert.strictEqual(action.trusted, true, 'Should be trusted after toggling trusted state');
assert.strictEqual(testNotebookModel.trustedMode, true, 'Model should be true after toggling trusted state');
// Should toggle trusted to false on subsequent action
result = await action.run(testUri);
assert.ok(result, 'Trusted Action should succeed again');
await action.run(testUri);
assert.strictEqual(action.trusted, false, 'Should toggle trusted to false');
assert.strictEqual(testNotebookModel.trustedMode, false, 'Model should be false again after toggling trusted state');
});
@@ -198,16 +194,14 @@ suite('Notebook Actions', function (): void {
// Normal use case
mockNotebookEditor.setup(c => c.runAllCells()).returns(() => Promise.resolve(true));
let result = await action.run(testUri);
assert.ok(result, 'Run All Cells Action should succeed');
await action.run(testUri);
mockNotebookEditor.verify(c => c.runAllCells(), TypeMoq.Times.once());
// Handle errors
mockNotebookEditor.reset();
mockNotebookEditor.setup(c => c.runAllCells()).throws(new Error('Test Error'));
result = await action.run(testUri);
assert.strictEqual(result, false, 'Run All Cells Action should fail on error');
await action.run(testUri);
});
test('Collapse Cells Action', async function (): Promise<void> {
@@ -225,8 +219,7 @@ suite('Notebook Actions', function (): void {
mockNotebookEditor.setup(x => x.cells).returns(() => testCells);
// Collapse cells case
let result = await action.run(testUri);
assert.ok(result, 'Collapse Cells Action should succeed');
await action.run(testUri);
assert.strictEqual(action.isCollapsed, true, 'Action should be collapsed after first toggle');
testCells.forEach(cell => {
@@ -234,8 +227,7 @@ suite('Notebook Actions', function (): void {
});
// Toggle cells to uncollapsed
result = await action.run(testUri);
assert.ok(result, 'Collapse Cells Action should succeed');
await action.run(testUri);
assert.strictEqual(action.isCollapsed, false, 'Action should not be collapsed after second toggle');
testCells.forEach(cell => {
@@ -254,7 +246,7 @@ suite('Notebook Actions', function (): void {
});
let action = new NewNotebookAction('TestId', 'TestLabel', mockCommandService.object, undefined, new NullAdsTelemetryService());
action.run(undefined);
await action.run(undefined);
assert.strictEqual(actualCmdId, NewNotebookAction.INTERNAL_NEW_NOTEBOOK_CMD_ID);
});