Skip to content

Minimize the changes to ipynb files when saving#7982

Merged
rchiodo merged 6 commits into
masterfrom
rchiodo/fix_save_format
Oct 14, 2019
Merged

Minimize the changes to ipynb files when saving#7982
rchiodo merged 6 commits into
masterfrom
rchiodo/fix_save_format

Conversation

@rchiodo
Copy link
Copy Markdown

@rchiodo rchiodo commented Oct 14, 2019

For #7960

Make sure internal structure of an ipynb file is maintained between jupyter and us.

file: string;
line: number;
state: CellState;
type: 'preview' | 'execute';
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Got rid of this as we don't use it. It was a hack for a 'preview' mode that never really worked.

// Source is usually a single string on input. Convert back to an array
return {
...cell,
source: this.convertSource(cell.source)
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

This should actually be the splitMultilineString function. I'll change.

@codecov-io
Copy link
Copy Markdown

codecov-io commented Oct 14, 2019

Codecov Report

Merging #7982 into master will decrease coverage by <.01%.
The diff coverage is 56.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7982      +/-   ##
==========================================
- Coverage   59.26%   59.26%   -0.01%     
==========================================
  Files         499      499              
  Lines       22400    22411      +11     
  Branches     3593     3597       +4     
==========================================
+ Hits        13275    13281       +6     
- Misses       8299     8301       +2     
- Partials      826      829       +3
Impacted Files Coverage Δ
src/client/datascience/cellFactory.ts 89.65% <ø> (ø) ⬆️
.../datascience/interactive-common/interactiveBase.ts 6.22% <ø> (ø) ⬆️
src/client/datascience/types.ts 100% <ø> (ø) ⬆️
...atascience/interactive-window/interactiveWindow.ts 23.91% <0%> (ø) ⬆️
src/client/datascience/gather/gather.ts 65.45% <100%> (ø) ⬆️
...ve-common/intellisense/baseIntellisenseProvider.ts 37.35% <100%> (ø) ⬆️
src/client/datascience/jupyter/jupyterNotebook.ts 6.07% <25%> (ø) ⬆️
src/datascience-ui/interactive-common/mainState.ts 46% <50%> (ø) ⬆️
src/client/datascience/jupyter/jupyterExporter.ts 19.44% <50%> (ø) ⬆️
src/client/datascience/common.ts 88.65% <61.11%> (-4.37%) ⬇️

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 e807b04...a1404da. Read the comment docs.

this.notebookJson = json;
}

// Double check json (if we have any)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Look like this check should go right after the JSON.parse. I wouldn't think we would want indentAmount and notebookJson set for an invalid file.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

That's true. I'll move.


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

Copy link
Copy Markdown
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 0d7d3ca into master Oct 14, 2019
@rchiodo rchiodo deleted the rchiodo/fix_save_format branch October 14, 2019 23:34
@lock lock Bot locked as resolved and limited conversation to collaborators Oct 21, 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.

3 participants