From 9b320d3f1f722c439b3172b568ddc2d30a988faa Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 8 Nov 2019 16:26:13 -0800 Subject: [PATCH 1/2] Fix cells being erased when saving and then changing focus to another cell --- news/2 Fixes/8399.md | 1 + .../interactive-common/mainStateController.ts | 55 +++++++++---------- .../native-editor/nativeCell.tsx | 4 +- 3 files changed, 28 insertions(+), 32 deletions(-) create mode 100644 news/2 Fixes/8399.md diff --git a/news/2 Fixes/8399.md b/news/2 Fixes/8399.md new file mode 100644 index 000000000000..31329e4755d3 --- /dev/null +++ b/news/2 Fixes/8399.md @@ -0,0 +1 @@ +Fix cells being erased when saving and then changing focus to another cell. diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index 2f2edac75563..89b4cf899c2c 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -339,26 +339,21 @@ export class MainStateController implements IMessageHandler { this.clearAllSilent(); } - public updateCellSource = (cellId: string) => { - const models = monacoEditor.editor.getModels(); - const cvm = this.findCell(cellId); - if (cvm) { - const modelId = this.getMonacoId(cvm.cell.id); - if (modelId) { - const model = models.find(m => m.id === modelId); - if (model) { - cvm.cell.data.source = cvm.inputBlockText = model.getValue().replace(/\r/g, ''); - } - } - } - } - public save = () => { // We have to take the current value of each cell to make sure we have the correct text. - this.pendingState.cellVMs.forEach(c => this.updateCellSource(c.cell.id)); + const newVMs = [...this.pendingState.cellVMs]; + for (let i = 0; i < newVMs.length; i += 1) { + const text = this.getMonacoEditorContents(newVMs[i].cell.id); + if (text) { + newVMs[i] = { ...newVMs[i], inputBlockText: text, cell: { ...newVMs[i].cell, data: { ...newVMs[i].cell.data, source: text } } }; + } + } + this.setState({ + cellVMs: newVMs + }); // Then send the save with the new state. - this.sendMessage(InteractiveWindowMessages.SaveAll, { cells: this.getNonEditCellVMs().map(cvm => cvm.cell) }); + this.sendMessage(InteractiveWindowMessages.SaveAll, { cells: newVMs.map(cvm => cvm.cell) }); } public showPlot = (imageHtml: string) => { @@ -764,6 +759,20 @@ export class MainStateController implements IMessageHandler { return this.pendingState; } + public getMonacoEditorContents(cellId: string): string | undefined { + const index = this.findCellIndex(cellId); + if (index >= 0) { + // Get the model for the monaco editor + const monacoId = this.getMonacoId(cellId); + if (monacoId) { + const model = monacoEditor.editor.getModels().find(m => m.id === monacoId); + if (model) { + return model.getValue().replace(/\r/g, ''); + } + } + } + } + // Adjust the visibility or collapsed state of a cell protected alterCellVM(cellVM: ICellViewModel, visible: boolean, expanded: boolean): ICellViewModel { if (cellVM.cell.data.cell_type === 'code') { @@ -811,20 +820,6 @@ export class MainStateController implements IMessageHandler { return cellVM; } - protected getMonacoEditorContents(cellId: string): string | undefined { - const index = this.findCellIndex(cellId); - if (index >= 0) { - // Get the model for the monaco editor - const monacoId = this.getMonacoId(cellId); - if (monacoId) { - const model = monacoEditor.editor.getModels().find(m => m.id === monacoId); - if (model) { - return model.getValue().replace(/\r/g, ''); - } - } - } - } - protected onCodeLostFocus(_cellId: string) { // Default is do nothing. } diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index bafab09f9356..832beaee8c90 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -558,8 +558,8 @@ export class NativeCell extends React.Component { private renderMiddleToolbar = () => { const cellId = this.props.cellVM.cell.id; const runCell = () => { - this.props.stateController.updateCellSource(cellId); - this.runAndMove(concatMultilineStringInput(this.props.cellVM.cell.data.source)); + const contents = this.props.stateController.getMonacoEditorContents(cellId) || concatMultilineStringInput(this.props.cellVM.cell.data.source); + this.runAndMove(contents); this.props.stateController.sendCommand(NativeCommandType.Run, 'mouse'); }; const gatherCell = () => { From e275d432618261da4ca0f1a02aee8dea0d1d071a Mon Sep 17 00:00:00 2001 From: Rich Chiodo Date: Fri, 8 Nov 2019 16:42:30 -0800 Subject: [PATCH 2/2] Make sure to use the source even if it's empty --- src/datascience-ui/interactive-common/mainStateController.ts | 2 +- src/datascience-ui/native-editor/nativeCell.tsx | 4 ++-- .../native-editor/nativeEditorStateController.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/datascience-ui/interactive-common/mainStateController.ts b/src/datascience-ui/interactive-common/mainStateController.ts index 89b4cf899c2c..dad2105f645f 100644 --- a/src/datascience-ui/interactive-common/mainStateController.ts +++ b/src/datascience-ui/interactive-common/mainStateController.ts @@ -344,7 +344,7 @@ export class MainStateController implements IMessageHandler { const newVMs = [...this.pendingState.cellVMs]; for (let i = 0; i < newVMs.length; i += 1) { const text = this.getMonacoEditorContents(newVMs[i].cell.id); - if (text) { + if (text !== undefined) { newVMs[i] = { ...newVMs[i], inputBlockText: text, cell: { ...newVMs[i].cell, data: { ...newVMs[i].cell.data, source: text } } }; } } diff --git a/src/datascience-ui/native-editor/nativeCell.tsx b/src/datascience-ui/native-editor/nativeCell.tsx index 832beaee8c90..08500ae84dd0 100644 --- a/src/datascience-ui/native-editor/nativeCell.tsx +++ b/src/datascience-ui/native-editor/nativeCell.tsx @@ -482,7 +482,7 @@ export class NativeCell extends React.Component { let content: string | undefined ; // If inside editor, submit this code - if (possibleContents) { + if (possibleContents !== undefined) { content = possibleContents; } else { // Outside editor, just use the cell @@ -558,7 +558,7 @@ export class NativeCell extends React.Component { private renderMiddleToolbar = () => { const cellId = this.props.cellVM.cell.id; const runCell = () => { - const contents = this.props.stateController.getMonacoEditorContents(cellId) || concatMultilineStringInput(this.props.cellVM.cell.data.source); + const contents = this.props.stateController.getMonacoEditorContents(cellId); this.runAndMove(contents); this.props.stateController.sendCommand(NativeCommandType.Run, 'mouse'); }; diff --git a/src/datascience-ui/native-editor/nativeEditorStateController.ts b/src/datascience-ui/native-editor/nativeEditorStateController.ts index a588637fd79e..b4af3930b862 100644 --- a/src/datascience-ui/native-editor/nativeEditorStateController.ts +++ b/src/datascience-ui/native-editor/nativeEditorStateController.ts @@ -286,7 +286,7 @@ export class NativeEditorStateController extends MainStateController { if (index >= 0) { // Get the model source from the monaco editor const source = this.getMonacoEditorContents(cellId); - if (source) { + if (source !== undefined) { const newVMs = [...this.getState().cellVMs]; // Update our state