Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions news/2 Fixes/8481.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Creating a new blank notebook should not require a search for jupyter.
1 change: 1 addition & 0 deletions news/2 Fixes/8594.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Typing 'z' in a cell causes the cell to disappear.
61 changes: 37 additions & 24 deletions src/client/datascience/interactive-ipynb/nativeEditor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
return this.close();
}

public get contents(): string {
public getContents(): Promise<string> {
return this.generateNotebookContent(this.visibleCells);
}

Expand Down Expand Up @@ -432,31 +432,18 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
* @memberof NativeEditor
*/
private async updateVersionInfoInNotebook(): Promise<void> {
// Make sure we have notebook json
await this.ensureNotebookJson();

// Use the active interpreter
const usableInterpreter = await this.jupyterExecution.getUsableJupyterPython();
if (usableInterpreter && usableInterpreter.version && this.notebookJson.metadata && this.notebookJson.metadata.language_info) {
this.notebookJson.metadata.language_info.version = `${usableInterpreter.version.major}.${usableInterpreter.version.minor}.${usableInterpreter.version.patch}`;
}
}

private async loadContents(contents: string | undefined, forceDirty: boolean): Promise<void> {
// tslint:disable-next-line: no-any
const json = contents ? JSON.parse(contents) as any : undefined;

// Double check json (if we have any)
if (json && !json.cells) {
throw new InvalidNotebookFileError(this.file.fsPath);
}

// Then compute indent. It's computed from the contents
if (contents) {
this.indentAmount = detectIndent(contents).indent;
}

// Then save the contents. We'll stick our cells back into this format when we save
if (json) {
this.notebookJson = json;
} else {
private async ensureNotebookJson(): Promise<void> {
if (!this.notebookJson || !this.notebookJson.metadata) {
const pythonNumber = await this.extractPythonMainVersion(this.notebookJson);
// Use this to build our metadata object
// Use these as the defaults unless we have been given some in the options.
Expand Down Expand Up @@ -484,6 +471,26 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
metadata: metadata
};
}
}

private async loadContents(contents: string | undefined, forceDirty: boolean): Promise<void> {
// tslint:disable-next-line: no-any
const json = contents ? JSON.parse(contents) as any : undefined;

// Double check json (if we have any)
if (json && !json.cells) {
throw new InvalidNotebookFileError(this.file.fsPath);
}

// Then compute indent. It's computed from the contents
if (contents) {
this.indentAmount = detectIndent(contents).indent;
}

// Then save the contents. We'll stick our cells back into this format when we save
if (json) {
this.notebookJson = json;
}

// Extract cells from the json
const cells = contents ? json.cells as (nbformat.ICodeCell | nbformat.IRawCell | nbformat.IMarkdownCell)[] : [];
Expand Down Expand Up @@ -705,9 +712,12 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
}

private async setDirty(): Promise<void> {
// Always update storage. Don't wait for results.
this.storeContents(this.generateNotebookContent(this.visibleCells))
.catch(ex => traceError('Failed to generate notebook content to store in state', ex));
// Update storage if not untitled. Don't wait for results.
if (!this.isUntitled) {
this.generateNotebookContent(this.visibleCells)
.then(c => this.storeContents(c).catch(ex => traceError('Failed to generate notebook content to store in state', ex)))
.ignoreErrors();
}

// Then update dirty flag.
if (!this._dirty) {
Expand Down Expand Up @@ -789,7 +799,10 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {
return usableInterpreter && usableInterpreter.version ? usableInterpreter.version.major : 3;
}

private generateNotebookContent(cells: ICell[]): string {
private async generateNotebookContent(cells: ICell[]): Promise<string> {
// Make sure we have some
await this.ensureNotebookJson();

// Reuse our original json except for the cells.
const json = {
...(this.notebookJson as nbformat.INotebookContent),
Expand Down Expand Up @@ -825,7 +838,7 @@ export class NativeEditor extends InteractiveBase implements INotebookEditor {

if (fileToSaveTo && isDirty) {
// Write out our visible cells
await this.fileSystem.writeFile(fileToSaveTo.fsPath, this.generateNotebookContent(this.visibleCells));
await this.fileSystem.writeFile(fileToSaveTo.fsPath, await this.generateNotebookContent(this.visibleCells));

// Update our file name and dirty state
this._file = fileToSaveTo;
Expand Down
18 changes: 10 additions & 8 deletions src/datascience-ui/native-editor/nativeEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -367,14 +367,16 @@ export class NativeEditor extends React.Component<INativeEditorProps, IMainState
}
case 'z':
case 'Z':
if (event.shiftKey && !event.ctrlKey && !event.altKey && this.stateController.canRedo()) {
event.stopPropagation();
this.stateController.redo();
this.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
} else if (!event.shiftKey && !event.altKey && !event.ctrlKey && this.stateController.canUndo()) {
event.stopPropagation();
this.stateController.undo();
this.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
if (this.stateController.getState().focusedCellId === undefined) {
if (event.shiftKey && !event.ctrlKey && !event.altKey && this.stateController.canRedo()) {
event.stopPropagation();
this.stateController.redo();
this.stateController.sendCommand(NativeCommandType.Redo, 'keyboard');
} else if (!event.shiftKey && !event.altKey && !event.ctrlKey && this.stateController.canUndo()) {
event.stopPropagation();
this.stateController.undo();
this.stateController.sendCommand(NativeCommandType.Undo, 'keyboard');
}
}
break;
default:
Expand Down
48 changes: 33 additions & 15 deletions src/test/datascience/interactive-ipynb/nativeEditor.unit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ suite('Data Science - Native Editor', () => {
test('Create new editor and add some cells', async () => {
const editor = createEditor();
await editor.load(baseFile, Uri.parse('file:///foo.ipynb'));
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);
editor.onMessage(InteractiveWindowMessages.InsertCell, { index: 0, cell: createEmptyCell('1', 1) });
expect(editor.cells).to.be.lengthOf(4);
expect(editor.isDirty).to.be.equal(true, 'Editor should be dirty');
Expand All @@ -273,7 +273,7 @@ suite('Data Science - Native Editor', () => {
test('Move cells around', async () => {
const editor = createEditor();
await editor.load(baseFile, Uri.parse('file:///foo.ipynb'));
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);
editor.onMessage(InteractiveWindowMessages.SwapCells, { firstCellId: 'NotebookImport#0', secondCellId: 'NotebookImport#1' });
expect(editor.cells).to.be.lengthOf(3);
expect(editor.isDirty).to.be.equal(true, 'Editor should be dirty');
Expand All @@ -283,7 +283,7 @@ suite('Data Science - Native Editor', () => {
test('Edit/delete cells', async () => {
const editor = createEditor();
await editor.load(baseFile, Uri.parse('file:///foo.ipynb'));
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);
expect(editor.isDirty).to.be.equal(false, 'Editor should not be dirty');
editor.onMessage(InteractiveWindowMessages.EditCell, {
changes: [{
Expand Down Expand Up @@ -313,7 +313,7 @@ suite('Data Science - Native Editor', () => {
async function loadEditorAddCellAndWaitForMementoUpdate(file: Uri) {
const editor = createEditor();
await editor.load(baseFile, file);
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);
storageUpdateSpy.resetHistory();
editor.onMessage(InteractiveWindowMessages.InsertCell, { index: 0, cell: createEmptyCell('1', 1) });
expect(editor.cells).to.be.lengthOf(4);
Expand All @@ -325,7 +325,7 @@ suite('Data Science - Native Editor', () => {

// Confirm contents were saved.
expect(storage.get(`notebook-storage-${file.toString()}`)).not.to.be.undefined;
expect(editor.contents).not.to.be.equal(baseFile);
expect(await editor.getContents()).not.to.be.equal(baseFile);

return editor;
}
Expand Down Expand Up @@ -359,8 +359,8 @@ suite('Data Science - Native Editor', () => {

// Verify contents are different.
// Meaning it was not loaded from file, but loaded from our storage.
expect(newEditor.contents).not.to.be.equal(baseFile);
const notebook = JSON.parse(newEditor.contents);
expect(await newEditor.getContents()).not.to.be.equal(baseFile);
const notebook = JSON.parse(await newEditor.getContents());
// 4 cells (1 extra for what was added)
expect(notebook.cells).to.be.lengthOf(4);
});
Expand All @@ -387,8 +387,8 @@ suite('Data Science - Native Editor', () => {

// Verify contents are different.
// Meaning it was not loaded from file, but loaded from our storage.
expect(newEditor.contents).not.to.be.equal(baseFile);
const notebook = JSON.parse(newEditor.contents);
expect(await newEditor.getContents()).not.to.be.equal(baseFile);
const notebook = JSON.parse(await newEditor.getContents());
// 4 cells (1 extra for what was added)
expect(notebook.cells).to.be.lengthOf(4);
});
Expand Down Expand Up @@ -417,18 +417,36 @@ suite('Data Science - Native Editor', () => {

// Verify contents are different.
// Meaning it was not loaded from file, but loaded from our storage.
expect(newEditor.contents).to.be.equal(baseFile);
expect(await newEditor.getContents()).to.be.equal(baseFile);
expect(newEditor.cells).to.be.lengthOf(3);
});

test('Python version info is not queried on creating a blank editor', async () => {
const file = Uri.parse('file:///Untitled1.ipynb');

// When a cell is executed, then ensure we store the python version info in the notebook data.
when(executionProvider.getUsableJupyterPython()).thenReject();

const editor = createEditor();
await editor.load('', file);

try {
await editor.getContents();
expect(false, 'Did not throw an error');
} catch {
// This should throw an error
noop();
}
});

test('Pyton version info will be updated in notebook when a cell has been executed', async () => {
const file = Uri.parse('file:///foo.ipynb');

const editor = createEditor();
await editor.load(baseFile, file);
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);
// At the begining version info is NOT in the file (at least not the same as what we are using to run cells).
let contents = JSON.parse(editor.contents) as nbformat.INotebookContent;
let contents = JSON.parse(await editor.getContents()) as nbformat.INotebookContent;
expect(contents.metadata!.language_info!.version).to.not.equal('10.11.12');

// When a cell is executed, then ensure we store the python version info in the notebook data.
Expand All @@ -453,7 +471,7 @@ suite('Data Science - Native Editor', () => {
}, 5_000, 'Timeout');

// Verify the version info is in the notbook.
contents = JSON.parse(editor.contents) as nbformat.INotebookContent;
contents = JSON.parse(await editor.getContents()) as nbformat.INotebookContent;
expect(contents.metadata!.language_info!.version).to.equal('10.11.12');
});

Expand All @@ -471,7 +489,7 @@ suite('Data Science - Native Editor', () => {
await editor.load('', file);

// It should load with that value
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);
expect(editor.cells).to.be.lengthOf(3);
});

Expand All @@ -496,7 +514,7 @@ suite('Data Science - Native Editor', () => {

const editor = createEditor();
await editor.load(baseFile, file);
expect(editor.contents).to.be.equal(baseFile);
expect(await editor.getContents()).to.be.equal(baseFile);

// Make our call to actually export
editor.onMessage(InteractiveWindowMessages.Export, editor.cells);
Expand Down