Skip to content

Conversation

rchiodo
Copy link

@rchiodo rchiodo commented Nov 9, 2019

… cell

For #8399

Root cause here is we were updating the state in line instead of calling setState.

Redux refactor would have fixed this as well.

@rchiodo
Copy link
Author

rchiodo commented Nov 9, 2019

This is high priority, so I'll be porting this over to the release branch too.

const newVMs = [...this.pendingState.cellVMs];
for (let i = 0; i < newVMs.length; i += 1) {
const text = this.getMonacoEditorContents(newVMs[i].cell.id);
if (text) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this bit work if you delete the text to empty out the string? If I'm reading it right you could delete the text, the '' would be falsy and not saved?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think you're right. I should check for undefined, not falsy


In reply to: 344416085 [](ancestors = 344416085)

Copy link
Member

@IanMatthewHuff IanMatthewHuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🕐

@codecov-io
Copy link

codecov-io commented Nov 9, 2019

Codecov Report

Merging #8482 into master will decrease coverage by 0.29%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #8482     +/-   ##
=========================================
- Coverage   59.39%   59.09%   -0.3%     
=========================================
  Files         509      510      +1     
  Lines       23422    23561    +139     
  Branches     3786     3813     +27     
=========================================
+ Hits        13912    13924     +12     
- Misses       8604     8729    +125     
- Partials      906      908      +2
Impacted Files Coverage Δ
...client/datascience/jupyter/jupyterCommandFinder.ts 73.59% <0%> (-1.81%) ⬇️
...rc/client/common/process/pythonExecutionFactory.ts 53.42% <0%> (-0.87%) ⬇️
src/client/datascience/types.ts 100% <0%> (ø) ⬆️
src/client/common/process/pythonDaemon.ts 7.69% <0%> (ø)
src/client/datascience/jupyter/jupyterCommand.ts 95.65% <0%> (+0.41%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9862c6a...e275d43. Read the comment docs.

Copy link
Member

@IanMatthewHuff IanMatthewHuff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@rchiodo rchiodo merged commit b0e8994 into master Nov 9, 2019
@rchiodo rchiodo deleted the rchiodo/autocomplete_code_loss branch November 9, 2019 01:46
rchiodo added a commit that referenced this pull request Nov 9, 2019
#8482)

* Fix cells being erased when saving and then changing focus to another cell

* Make sure to use the source even if it's empty
rchiodo added a commit that referenced this pull request Nov 9, 2019
#8482) (#8485)

* Fix cells being erased when saving and then changing focus to another cell

* Make sure to use the source even if it's empty
@lock lock bot locked as resolved and limited conversation to collaborators Nov 16, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants