Skip to content

Commit 35d6f66

Browse files
DonJayamanneDonJayamanne
authored
Ids of new cells must be provided to actions (#9833)
* ids of new cells must be provided to actions * Comments * Oops
1 parent 74ec45f commit 35d6f66

File tree

7 files changed

+82
-56
lines changed

7 files changed

+82
-56
lines changed

‎src/client/datascience/interactive-common/interactiveWindowTypes.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
'use strict';
44
import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
55
import { IServerState } from '../../../datascience-ui/interactive-common/mainState';
6+
import { IAddCellAction } from '../../../datascience-ui/interactive-common/redux/reducers/types';
67
import { CssMessages, IGetCssRequest, IGetCssResponse, IGetMonacoThemeRequest } from '../messages';
78
import { ICell, IInteractiveWindowInfo, IJupyterVariable, IJupyterVariablesRequest, IJupyterVariablesResponse } from '../types';
89

@@ -307,7 +308,7 @@ export class IInteractiveWindowMapping {
307308
public [InteractiveWindowMessages.GetAllCells]: ICell;
308309
public [InteractiveWindowMessages.ReturnAllCells]: ICell[];
309310
public [InteractiveWindowMessages.DeleteCell]: never | undefined;
310-
public [InteractiveWindowMessages.DeleteAllCells]: never | undefined;
311+
public [InteractiveWindowMessages.DeleteAllCells]: IAddCellAction;
311312
public [InteractiveWindowMessages.Undo]: never | undefined;
312313
public [InteractiveWindowMessages.Redo]: never | undefined;
313314
public [InteractiveWindowMessages.ExpandAll]: never | undefined;

‎src/datascience-ui/history-react/redux/mapping.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { IMainState, IServerState } from '../../interactive-common/mainState';
99
import { IncomingMessageActions } from '../../interactive-common/redux/postOffice';
1010
import {
1111
CommonActionType,
12+
IAddCellAction,
1213
ICellAction,
1314
ICodeAction,
1415
IEditCellAction,
@@ -42,7 +43,7 @@ export class IInteractiveActionMapping {
4243
public [CommonActionType.GATHER_CELL]: InteractiveReducerFunc<ICellAction>;
4344
public [CommonActionType.EDIT_CELL]: InteractiveReducerFunc<IEditCellAction>;
4445
public [CommonActionType.SUBMIT_INPUT]: InteractiveReducerFunc<ICodeAction>;
45-
public [CommonActionType.DELETE_ALL_CELLS]: InteractiveReducerFunc<never | undefined>;
46+
public [CommonActionType.DELETE_ALL_CELLS]: InteractiveReducerFunc<IAddCellAction>;
4647
public [CommonActionType.EXPAND_ALL]: InteractiveReducerFunc<never | undefined>;
4748
public [CommonActionType.COLLAPSE_ALL]: InteractiveReducerFunc<never | undefined>;
4849
public [CommonActionType.EDITOR_LOADED]: InteractiveReducerFunc<never | undefined>;
@@ -63,7 +64,7 @@ export class IInteractiveActionMapping {
6364
public [IncomingMessageActions.GETALLCELLS]: InteractiveReducerFunc<never | undefined>;
6465
public [IncomingMessageActions.EXPANDALL]: InteractiveReducerFunc<never | undefined>;
6566
public [IncomingMessageActions.COLLAPSEALL]: InteractiveReducerFunc<never | undefined>;
66-
public [IncomingMessageActions.DELETEALLCELLS]: InteractiveReducerFunc<never | undefined>;
67+
public [IncomingMessageActions.DELETEALLCELLS]: InteractiveReducerFunc<IAddCellAction>;
6768
public [IncomingMessageActions.STARTPROGRESS]: InteractiveReducerFunc<never | undefined>;
6869
public [IncomingMessageActions.STOPPROGRESS]: InteractiveReducerFunc<never | undefined>;
6970
public [IncomingMessageActions.UPDATESETTINGS]: InteractiveReducerFunc<string>;

‎src/datascience-ui/interactive-common/redux/reducers/types.ts

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,14 @@ export interface ICellAction {
9494
cellId: string | undefined;
9595
}
9696

97+
export interface IAddCellAction {
98+
/**
99+
* Id of the new cell that is to be added.
100+
* If none provided, then generate a new id.
101+
*/
102+
newCellId: string;
103+
}
104+
97105
export interface ICodeAction extends ICellAction {
98106
code: string;
99107
}
@@ -103,9 +111,16 @@ export interface IEditCellAction extends ICodeAction {
103111
modelId: string;
104112
}
105113

106-
export interface IExecuteAction extends ICodeAction {
107-
moveOp: 'add' | 'select' | 'none';
108-
}
114+
// I.e. when using the operation `add`, we need the corresponding `IAddCellAction`.
115+
// They are mutually exclusive, if not `add`, then there's no `newCellId`.
116+
export type IExecuteAction =
117+
| (ICodeAction & {
118+
moveOp: 'select' | 'none';
119+
})
120+
| (ICodeAction &
121+
IAddCellAction & {
122+
moveOp: 'add';
123+
});
109124

110125
export interface ICodeCreatedAction extends ICellAction {
111126
modelId: string;

‎src/datascience-ui/native-editor/redux/actions.ts

Lines changed: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,13 +2,14 @@
22
// Licensed under the MIT License.
33
'use strict';
44
import * as monacoEditor from 'monaco-editor/esm/vs/editor/editor.api';
5-
5+
import * as uuid from 'uuid/v4';
66
import { NativeCommandType } from '../../../client/datascience/interactive-common/interactiveWindowTypes';
77
import { IJupyterVariable, IJupyterVariablesRequest } from '../../../client/datascience/types';
88
import { CursorPos } from '../../interactive-common/mainState';
99
import {
1010
CommonAction,
1111
CommonActionType,
12+
IAddCellAction,
1213
ICellAction,
1314
ICellAndCursorAction,
1415
IChangeCellTypeAction,
@@ -25,9 +26,9 @@ import {
2526

2627
// See https://react-redux.js.org/using-react-redux/connect-mapdispatch#defining-mapdispatchtoprops-as-an-object
2728
export const actionCreators = {
28-
insertAbove: (cellId: string | undefined): CommonAction<ICellAction> => ({ type: CommonActionType.INSERT_ABOVE, payload: { cellId } }),
29-
insertAboveFirst: (): CommonAction<never | undefined> => ({ type: CommonActionType.INSERT_ABOVE_FIRST }),
30-
insertBelow: (cellId: string | undefined): CommonAction<ICellAction> => ({ type: CommonActionType.INSERT_BELOW, payload: { cellId } }),
29+
insertAbove: (cellId: string | undefined): CommonAction<ICellAction & IAddCellAction> => ({ type: CommonActionType.INSERT_ABOVE, payload: { cellId, newCellId: uuid() } }),
30+
insertAboveFirst: (): CommonAction<never | undefined> => ({ type: CommonActionType.INSERT_ABOVE_FIRST, payload: { newCellId: uuid() } }),
31+
insertBelow: (cellId: string | undefined): CommonAction<ICellAction & IAddCellAction> => ({ type: CommonActionType.INSERT_BELOW, payload: { cellId, newCellId: uuid() } }),
3132
focusCell: (cellId: string, cursorPos: CursorPos = CursorPos.Current): CommonAction<ICellAndCursorAction> => ({
3233
type: CommonActionType.FOCUS_CELL,
3334
payload: { cellId, cursorPos }
@@ -37,11 +38,20 @@ export const actionCreators = {
3738
type: CommonActionType.SELECT_CELL,
3839
payload: { cellId, cursorPos }
3940
}),
40-
addCell: (): CommonAction<never | undefined> => ({ type: CommonActionType.ADD_NEW_CELL }),
41-
executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none'): CommonAction<IExecuteAction> => ({
42-
type: CommonActionType.EXECUTE_CELL,
43-
payload: { cellId, code, moveOp }
44-
}),
41+
addCell: (): CommonAction<never | undefined> => ({ type: CommonActionType.ADD_NEW_CELL, payload: { newCellId: uuid() } }),
42+
executeCell: (cellId: string, code: string, moveOp: 'add' | 'select' | 'none'): CommonAction<IExecuteAction> => {
43+
if (moveOp === 'add') {
44+
return {
45+
type: CommonActionType.EXECUTE_CELL,
46+
payload: { cellId, code, moveOp, newCellId: uuid() }
47+
};
48+
} else {
49+
return {
50+
type: CommonActionType.EXECUTE_CELL,
51+
payload: { cellId, code, moveOp }
52+
};
53+
}
54+
},
4555
executeAllCells: (): CommonAction<never | undefined> => ({ type: CommonActionType.EXECUTE_ALL_CELLS }),
4656
executeAbove: (cellId: string): CommonAction<ICellAction> => ({ type: CommonActionType.EXECUTE_ABOVE, payload: { cellId } }),
4757
executeCellAndBelow: (cellId: string, code: string): CommonAction<ICodeAction> => ({ type: CommonActionType.EXECUTE_CELL_AND_BELOW, payload: { cellId, code } }),

‎src/datascience-ui/native-editor/redux/mapping.ts

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { IMainState, IServerState } from '../../interactive-common/mainState';
99
import { IncomingMessageActions } from '../../interactive-common/redux/postOffice';
1010
import {
1111
CommonActionType,
12+
IAddCellAction,
1213
ICellAction,
1314
ICellAndCursorAction,
1415
IChangeCellTypeAction,
@@ -27,12 +28,12 @@ type NativeEditorReducerFunc<T> = ReducerFunc<IMainState, CommonActionType, T>;
2728
export type NativeEditorReducerArg<T = never | undefined> = ReducerArg<IMainState, CommonActionType, T>;
2829

2930
export class INativeEditorActionMapping {
30-
public [CommonActionType.INSERT_ABOVE]: NativeEditorReducerFunc<ICellAction>;
31-
public [CommonActionType.INSERT_BELOW]: NativeEditorReducerFunc<ICellAction>;
32-
public [CommonActionType.INSERT_ABOVE_FIRST]: NativeEditorReducerFunc<never | undefined>;
31+
public [CommonActionType.INSERT_ABOVE]: NativeEditorReducerFunc<ICellAction & IAddCellAction>;
32+
public [CommonActionType.INSERT_BELOW]: NativeEditorReducerFunc<ICellAction & IAddCellAction>;
33+
public [CommonActionType.INSERT_ABOVE_FIRST]: NativeEditorReducerFunc<IAddCellAction>;
3334
public [CommonActionType.FOCUS_CELL]: NativeEditorReducerFunc<ICellAndCursorAction>;
3435
public [CommonActionType.UNFOCUS_CELL]: NativeEditorReducerFunc<ICodeAction>;
35-
public [CommonActionType.ADD_NEW_CELL]: NativeEditorReducerFunc<never | undefined>;
36+
public [CommonActionType.ADD_NEW_CELL]: NativeEditorReducerFunc<IAddCellAction>;
3637
public [CommonActionType.EXECUTE_CELL]: NativeEditorReducerFunc<IExecuteAction>;
3738
public [CommonActionType.EXECUTE_ALL_CELLS]: NativeEditorReducerFunc<never | undefined>;
3839
public [CommonActionType.EXECUTE_ABOVE]: NativeEditorReducerFunc<ICellAction>;
@@ -74,9 +75,9 @@ export class INativeEditorActionMapping {
7475
public [IncomingMessageActions.LOADALLCELLS]: NativeEditorReducerFunc<ILoadAllCells>;
7576
public [IncomingMessageActions.NOTEBOOKRUNALLCELLS]: NativeEditorReducerFunc<never | undefined>;
7677
public [IncomingMessageActions.NOTEBOOKRUNSELECTEDCELL]: NativeEditorReducerFunc<never | undefined>;
77-
public [IncomingMessageActions.NOTEBOOKADDCELLBELOW]: NativeEditorReducerFunc<never | undefined>;
78+
public [IncomingMessageActions.NOTEBOOKADDCELLBELOW]: NativeEditorReducerFunc<IAddCellAction>;
7879
public [IncomingMessageActions.DOSAVE]: NativeEditorReducerFunc<never | undefined>;
79-
public [IncomingMessageActions.DELETEALLCELLS]: NativeEditorReducerFunc<never | undefined>;
80+
public [IncomingMessageActions.DELETEALLCELLS]: NativeEditorReducerFunc<IAddCellAction>;
8081
public [IncomingMessageActions.UNDO]: NativeEditorReducerFunc<never | undefined>;
8182
public [IncomingMessageActions.REDO]: NativeEditorReducerFunc<never | undefined>;
8283
public [IncomingMessageActions.STARTPROGRESS]: NativeEditorReducerFunc<never | undefined>;

‎src/datascience-ui/native-editor/redux/reducers/creation.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { ICell, IDataScienceExtraSettings } from '../../../../client/datascience
88
import { createCellVM, createEmptyCell, CursorPos, extractInputText, ICellViewModel, IMainState } from '../../../interactive-common/mainState';
99
import { createPostableAction } from '../../../interactive-common/redux/postOffice';
1010
import { Helpers } from '../../../interactive-common/redux/reducers/helpers';
11-
import { ICellAction } from '../../../interactive-common/redux/reducers/types';
11+
import { IAddCellAction, ICellAction } from '../../../interactive-common/redux/reducers/types';
1212
import { actionCreators } from '../actions';
1313
import { NativeEditorReducerArg } from '../mapping';
1414

@@ -37,8 +37,8 @@ export namespace Creation {
3737
}
3838
}
3939

40-
export function insertAbove(arg: NativeEditorReducerArg<ICellAction>): IMainState {
41-
const newVM = prepareCellVM(createEmptyCell(uuid(), null), false, arg.prevState.settings);
40+
export function insertAbove(arg: NativeEditorReducerArg<ICellAction & IAddCellAction>): IMainState {
41+
const newVM = prepareCellVM(createEmptyCell(arg.payload.newCellId || uuid(), null), false, arg.prevState.settings);
4242
const newList = [...arg.prevState.cellVMs];
4343

4444
// Find the position where we want to insert
@@ -69,8 +69,8 @@ export namespace Creation {
6969
return result;
7070
}
7171

72-
export function insertBelow(arg: NativeEditorReducerArg<ICellAction>): IMainState {
73-
const newVM = prepareCellVM(createEmptyCell(uuid(), null), false, arg.prevState.settings);
72+
export function insertBelow(arg: NativeEditorReducerArg<ICellAction & IAddCellAction>): IMainState {
73+
const newVM = prepareCellVM(createEmptyCell(arg.payload.newCellId || uuid(), null), false, arg.prevState.settings);
7474
const newList = [...arg.prevState.cellVMs];
7575

7676
// Find the position where we want to insert
@@ -104,17 +104,17 @@ export namespace Creation {
104104
return result;
105105
}
106106

107-
export function insertAboveFirst(arg: NativeEditorReducerArg): IMainState {
107+
export function insertAboveFirst(arg: NativeEditorReducerArg<IAddCellAction>): IMainState {
108108
// Get the first cell id
109109
const firstCellId = arg.prevState.cellVMs.length > 0 ? arg.prevState.cellVMs[0].cell.id : undefined;
110110

111111
// Do what an insertAbove does
112-
return insertAbove({ ...arg, payload: { cellId: firstCellId } });
112+
return insertAbove({ ...arg, payload: { cellId: firstCellId, newCellId: arg.payload.newCellId } });
113113
}
114114

115-
export function addNewCell(arg: NativeEditorReducerArg): IMainState {
115+
export function addNewCell(arg: NativeEditorReducerArg<IAddCellAction>): IMainState {
116116
// Do the same thing that an insertBelow does using the currently selected cell.
117-
return insertBelow({ ...arg, payload: { cellId: arg.prevState.selectedCellId } });
117+
return insertBelow({ ...arg, payload: { cellId: arg.prevState.selectedCellId, newCellId: arg.payload.newCellId } });
118118
}
119119

120120
export function startCell(arg: NativeEditorReducerArg<ICell>): IMainState {
@@ -129,13 +129,13 @@ export namespace Creation {
129129
return Helpers.updateOrAdd(arg, (c: ICell, s: IMainState) => prepareCellVM(c, true, s.settings));
130130
}
131131

132-
export function deleteAllCells(arg: NativeEditorReducerArg): IMainState {
132+
export function deleteAllCells(arg: NativeEditorReducerArg<IAddCellAction>): IMainState {
133133
// Send messages to other side to indicate the deletes
134134
arg.queueAction(createPostableAction(InteractiveWindowMessages.DeleteAllCells));
135135

136136
// Just leave one single blank empty cell
137137
const newVM: ICellViewModel = {
138-
cell: createEmptyCell(uuid(), null),
138+
cell: createEmptyCell(arg.payload.newCellId, null),
139139
editable: true,
140140
inputBlockOpen: true,
141141
inputBlockShow: true,

‎src/datascience-ui/native-editor/redux/reducers/execution.ts

Lines changed: 22 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -69,30 +69,28 @@ export namespace Execution {
6969
const executeResult = executeRange(arg.prevState, index, index, [arg.payload.code], arg.queueAction);
7070

7171
// Modify the execute result if moving
72-
switch (arg.payload.moveOp) {
73-
case 'add':
74-
// Add a new cell below
75-
return Creation.insertBelow({ ...arg, prevState: executeResult });
76-
77-
case 'select':
78-
// Select the cell below this one, but don't focus it
79-
if (index < arg.prevState.cellVMs.length - 1) {
80-
return Effects.selectCell({
81-
...arg,
82-
prevState: {
83-
...executeResult
84-
},
85-
payload: {
86-
...arg.payload,
87-
cellId: arg.prevState.cellVMs[index + 1].cell.id,
88-
cursorPos: CursorPos.Current
89-
}
90-
});
91-
}
92-
return executeResult;
93-
94-
default:
95-
return executeResult;
72+
// Use `if` instead of `switch case` to ensure type safety.
73+
if (arg.payload.moveOp === 'add') {
74+
// Add a new cell below
75+
return Creation.insertBelow({ ...arg, prevState: executeResult, payload: { ...arg.payload } });
76+
} else if (arg.payload.moveOp === 'select') {
77+
// Select the cell below this one, but don't focus it
78+
if (index < arg.prevState.cellVMs.length - 1) {
79+
return Effects.selectCell({
80+
...arg,
81+
prevState: {
82+
...executeResult
83+
},
84+
payload: {
85+
...arg.payload,
86+
cellId: arg.prevState.cellVMs[index + 1].cell.id,
87+
cursorPos: CursorPos.Current
88+
}
89+
});
90+
}
91+
return executeResult;
92+
} else {
93+
return executeResult;
9694
}
9795
}
9896
return arg.prevState;

0 commit comments

Comments
 (0)