Skip to content

Conversation

joyceerhl
Copy link
Contributor

Fix #7280

Root cause is code assumed the placeholder cell was the last one in the notebook document and didn't check any of the other cells, so if you added a cell via the input box while the kernel was starting, it would update the wrong cell. We are still assuming there's only one placeholder cell in the entire interactive window.

We could also just check the first cell, but that seems equally brittle e.g. if on a kernel restart we decide to show the placeholder cell as well, that won't work correctly.

@joyceerhl joyceerhl requested a review from a team as a code owner August 26, 2021 17:54
@codecov-commenter
Copy link

Codecov Report

Merging #7307 (5bbe27f) into main (a954838) will increase coverage by 2%.
The diff coverage is 0%.

@@          Coverage Diff           @@
##            main   #7307    +/-   ##
======================================
+ Coverage     62%     64%    +2%     
======================================
  Files        360     360            
  Lines      22535   22536     +1     
  Branches    3402    3403     +1     
======================================
+ Hits       14077   14625   +548     
+ Misses      7282    6609   -673     
- Partials    1176    1302   +126     
Impacted Files Coverage Δ
src/client/datascience/jupyter/kernels/kernel.ts 65% <0%> (-1%) ⬇️
src/client/common/cancellation.ts 72% <0%> (-4%) ⬇️
...t/datascience/notebook/vscodeNotebookController.ts 78% <0%> (-2%) ⬇️
...atascience/jupyter/kernels/jupyterKernelService.ts 80% <0%> (-2%) ⬇️
.../datascience/notebook/notebookControllerManager.ts 84% <0%> (-1%) ⬇️
...lient/datascience/jupyter/kernels/cellExecution.ts 68% <0%> (-1%) ⬇️
...ence/interactive-window/nativeInteractiveWindow.ts 6% <0%> (ø)
src/client/datascience/commands/commandRegistry.ts 35% <0%> (+<1%) ⬆️
...datascience/editor-integration/cellhashprovider.ts 64% <0%> (+<1%) ⬆️
src/client/datascience/baseJupyterSession.ts 62% <0%> (+<1%) ⬆️
... and 30 more

@joyceerhl joyceerhl merged commit 64ba681 into main Aug 26, 2021
@joyceerhl joyceerhl deleted the dev/joyceerhl/iw-sysinfo branch August 26, 2021 18:31
joyceerhl added a commit that referenced this pull request Aug 31, 2021
joyceerhl added a commit that referenced this pull request Aug 31, 2021
* Revert "Fix interactive window placeholder cell updates (#7340) (#7343)"

This reverts commit 8bdf492.

* Revert "Fix placeholder sys info cell not being updated correctly (#7307)"

This reverts commit 64ba681.

* Update changelog

* Tweak interactive window test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Output order is incorrect
3 participants