Nb/show notebooks as untitled (#6376)

* open notebooks as untitled files and removed the saving to home directory

* return just filename without extension since save is adding the .ipynb ext

* Changes to open bdc notebooks as untitled docs. Updated tests to align with the changes

* changes: not to show untitled file as dirty and cleaned up tests in notebookService

* changes to address untitled name conflicts with open editors.

* comment cleanup
This commit is contained in:
Maddy
2019-07-16 16:00:38 -07:00
committed by GitHub
parent 095a4b17f7
commit caff76b723
9 changed files with 70 additions and 74 deletions

View File

@@ -340,17 +340,21 @@ async function handleOpenNotebookTask(profile: azdata.IConnectionProfile): Promi
} }
async function handleOpenClusterStatusNotebookTask(profile: azdata.IConnectionProfile, appContext: AppContext): Promise<void> { async function handleOpenClusterStatusNotebookTask(profile: azdata.IConnectionProfile, appContext: AppContext): Promise<void> {
const notebookRelativePath = 'notebooks/tsg/cluster-status.ipynb'; const notebookRelativePath: string = 'notebooks/tsg/cluster-status.ipynb';
const notebookFullPath = path.join(appContext.extensionContext.extensionPath, notebookRelativePath); const notebookFullPath: string = path.join(appContext.extensionContext.extensionPath, notebookRelativePath);
if (!Utils.fileExists(notebookFullPath)) { if (!Utils.fileExists(notebookFullPath)) {
vscode.window.showErrorMessage(localize("fileNotFound", "Unable to find the file specified")); vscode.window.showErrorMessage(localize("fileNotFound", "Unable to find the file specified"));
} else { } else {
const targetFile = Utils.getTargetFileName(notebookFullPath); const title: string = Utils.findNextUntitledEditorName(notebookFullPath);
Utils.copyFile(notebookFullPath, targetFile); const untitledFileName: vscode.Uri = vscode.Uri.parse(`untitled:${title}`);
let fileUri = vscode.Uri.file(targetFile); vscode.workspace.openTextDocument(notebookFullPath).then((document) => {
await azdata.nb.showNotebookDocument(fileUri, { let initialContent = document.getText();
connectionProfile: profile, azdata.nb.showNotebookDocument(untitledFileName, {
preview: false connectionProfile: profile,
preview: true,
initialContent: initialContent,
initialDirtyState: false
});
}); });
} }
} }

View File

@@ -35,19 +35,18 @@ export function getAppDataPath() {
* @param filePath source notebook file name * @param filePath source notebook file name
* @param fileExtension file type * @param fileExtension file type
*/ */
export function getTargetFileName(filePath: string): string { export function findNextUntitledEditorName(filePath: string): string {
const targetDirectory = os.homedir();
const fileExtension = path.extname(filePath); const fileExtension = path.extname(filePath);
const baseName = path.basename(filePath, fileExtension); const baseName = path.basename(filePath, fileExtension);
let targetFileName;
let idx = 0; let idx = 0;
let title = `${baseName}`;
do { do {
const suffix = idx === 0 ? '' : `-${idx}`; const suffix = idx === 0 ? '' : `-${idx}`;
targetFileName = path.join(targetDirectory, `${baseName}${suffix}${fileExtension}`); title = `${baseName}${suffix}`;
idx++; idx++;
} while (fs.existsSync(targetFileName)); } while (azdata.nb.notebookDocuments.findIndex(doc => doc.isUntitled && doc.fileName === title) > -1);
return targetFileName; return title;
} }
export function fileExists(file: string): boolean { export function fileExists(file: string): boolean {

View File

@@ -4,9 +4,10 @@
*--------------------------------------------------------------------------------------------*/ *--------------------------------------------------------------------------------------------*/
'use strict'; 'use strict';
import * as vscode from 'vscode';
import * as azdata from 'azdata';
import { NotebookInfo } from '../interfaces'; import { NotebookInfo } from '../interfaces';
import { isString } from 'util'; import { isString } from 'util';
import * as os from 'os';
import * as path from 'path'; import * as path from 'path';
import * as nls from 'vscode-nls'; import * as nls from 'vscode-nls';
import { IPlatformService } from './platformService'; import { IPlatformService } from './platformService';
@@ -28,9 +29,7 @@ export class NotebookService implements INotebookService {
const notebookRelativePath = this.getNotebook(notebook); const notebookRelativePath = this.getNotebook(notebook);
const notebookFullPath = path.join(__dirname, '../../', notebookRelativePath); const notebookFullPath = path.join(__dirname, '../../', notebookRelativePath);
if (notebookRelativePath && this.platformService.fileExists(notebookFullPath)) { if (notebookRelativePath && this.platformService.fileExists(notebookFullPath)) {
const targetFileName = this.getTargetNotebookFileName(notebookFullPath, os.homedir()); this.showNotebookAsUntitled(notebookFullPath);
this.platformService.copyFile(notebookFullPath, targetFileName);
this.platformService.openFile(targetFileName);
} }
else { else {
this.platformService.showErrorMessage(localize('resourceDeployment.notebookNotFound', 'The notebook {0} does not exist', notebookFullPath)); this.platformService.showErrorMessage(localize('resourceDeployment.notebookNotFound', 'The notebook {0} does not exist', notebookFullPath));
@@ -58,22 +57,31 @@ export class NotebookService implements INotebookService {
return notebookPath; return notebookPath;
} }
/** findNextUntitledEditorName(filePath: string): string {
* Get a file name that is not already used in the target directory const fileExtension = path.extname(filePath);
* @param notebook source notebook file name const baseName = path.basename(filePath, fileExtension);
* @param targetDirectory target directory
*/
getTargetNotebookFileName(notebook: string, targetDirectory: string): string {
const notebookFileExtension = '.ipynb';
const baseName = path.basename(notebook, notebookFileExtension);
let targetFileName;
let idx = 0; let idx = 0;
let title = `${baseName}`;
do { do {
const suffix = idx === 0 ? '' : `-${idx}`; const suffix = idx === 0 ? '' : `-${idx}`;
targetFileName = path.join(targetDirectory, `${baseName}${suffix}${notebookFileExtension}`); title = `${baseName}${suffix}`;
idx++; idx++;
} while (this.platformService.fileExists(targetFileName)); } while (this.platformService.isNotebookNameUsed(title));
return targetFileName; return title;
}
showNotebookAsUntitled(notebookPath: string): void {
let targetFileName: string = this.findNextUntitledEditorName(notebookPath);
const untitledFileName: vscode.Uri = vscode.Uri.parse(`untitled:${targetFileName}`);
vscode.workspace.openTextDocument(notebookPath).then((document) => {
let initialContent = document.getText();
azdata.nb.showNotebookDocument(untitledFileName, {
connectionProfile: undefined,
preview: false,
initialContent: initialContent,
initialDirtyState: false
});
});
} }
} }

View File

@@ -6,6 +6,7 @@
import * as fs from 'fs'; import * as fs from 'fs';
import * as vscode from 'vscode'; import * as vscode from 'vscode';
import * as azdata from 'azdata';
/** /**
* Abstract of platform dependencies * Abstract of platform dependencies
@@ -16,6 +17,7 @@ export interface IPlatformService {
fileExists(file: string): boolean; fileExists(file: string): boolean;
openFile(filePath: string): void; openFile(filePath: string): void;
showErrorMessage(message: string): void; showErrorMessage(message: string): void;
isNotebookNameUsed(title: string): boolean;
} }
export class PlatformService implements IPlatformService { export class PlatformService implements IPlatformService {
@@ -38,4 +40,8 @@ export class PlatformService implements IPlatformService {
showErrorMessage(message: string): void { showErrorMessage(message: string): void {
vscode.window.showErrorMessage(message); vscode.window.showErrorMessage(message);
} }
isNotebookNameUsed(title: string): boolean {
return (azdata.nb.notebookDocuments.findIndex(doc => doc.isUntitled && doc.fileName === title) > -1);
}
} }

View File

@@ -7,8 +7,6 @@
import * as TypeMoq from 'typemoq'; import * as TypeMoq from 'typemoq';
import 'mocha'; import 'mocha';
import * as path from 'path';
import * as os from 'os';
import { NotebookService } from '../services/NotebookService'; import { NotebookService } from '../services/NotebookService';
import assert = require('assert'); import assert = require('assert');
import { NotebookInfo } from '../interfaces'; import { NotebookInfo } from '../interfaces';
@@ -62,59 +60,30 @@ suite('Notebook Service Tests', function (): void {
mockPlatformService.verify((service) => service.platform(), TypeMoq.Times.once()); mockPlatformService.verify((service) => service.platform(), TypeMoq.Times.once());
}); });
test('launchNotebook', () => { test('findNextUntitledEditorName with no name conflict', () => {
const mockPlatformService = TypeMoq.Mock.ofType<IPlatformService>();
const notebookService = new NotebookService(mockPlatformService.object);
const notebookFileName = 'mynotebook.ipynb';
const notebookPath = `./notebooks/${notebookFileName}`;
let actualSourceFile;
const expectedSourceFile = path.join(__dirname, '../../', notebookPath);
let actualTargetFile;
const expectedTargetFile = path.join(os.homedir(), notebookFileName);
mockPlatformService.setup((service) => service.platform()).returns(() => { return 'win32'; });
mockPlatformService.setup((service) => service.openFile(TypeMoq.It.isAnyString()));
mockPlatformService.setup((service) => service.fileExists(TypeMoq.It.isAnyString()))
.returns((path) => {
if (path === expectedSourceFile) {
return true;
}
return false;
});
mockPlatformService.setup((service) => service.copyFile(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString()))
.returns((source, target) => { actualSourceFile = source; actualTargetFile = target; });
notebookService.launchNotebook(notebookPath);
mockPlatformService.verify((service) => service.copyFile(TypeMoq.It.isAnyString(), TypeMoq.It.isAnyString()), TypeMoq.Times.once());
mockPlatformService.verify((service) => service.openFile(TypeMoq.It.isAnyString()), TypeMoq.Times.once());
assert.equal(actualSourceFile, expectedSourceFile, 'source file is not correct');
assert.equal(actualTargetFile, expectedTargetFile, 'target file is not correct');
});
test('getTargetNotebookFileName with no name conflict', () => {
const mockPlatformService = TypeMoq.Mock.ofType<IPlatformService>(); const mockPlatformService = TypeMoq.Mock.ofType<IPlatformService>();
const notebookService = new NotebookService(mockPlatformService.object); const notebookService = new NotebookService(mockPlatformService.object);
const notebookFileName = 'mynotebook.ipynb'; const notebookFileName = 'mynotebook.ipynb';
const sourceNotebookPath = `./notebooks/${notebookFileName}`; const sourceNotebookPath = `./notebooks/${notebookFileName}`;
const expectedTargetFile = path.join(os.homedir(), notebookFileName); const expectedTargetFile = 'mynotebook';
mockPlatformService.setup((service) => service.fileExists(TypeMoq.It.isAnyString())) mockPlatformService.setup((service) => service.isNotebookNameUsed(TypeMoq.It.isAnyString()))
.returns((path) => { return false; }); .returns((path) => { return false; });
const actualFileName = notebookService.getTargetNotebookFileName(sourceNotebookPath, os.homedir()); const actualFileName = notebookService.findNextUntitledEditorName(sourceNotebookPath);
mockPlatformService.verify((service) => service.fileExists(TypeMoq.It.isAnyString()), TypeMoq.Times.once()); mockPlatformService.verify((service) => service.isNotebookNameUsed(TypeMoq.It.isAnyString()), TypeMoq.Times.once());
assert.equal(actualFileName, expectedTargetFile, 'target file name is not correct'); assert.equal(actualFileName, expectedTargetFile, 'target file name is not correct');
}); });
test('getTargetNotebookFileName with name conflicts', () => { test('findNextUntitledEditorName with name conflicts', () => {
const mockPlatformService = TypeMoq.Mock.ofType<IPlatformService>(); const mockPlatformService = TypeMoq.Mock.ofType<IPlatformService>();
const notebookService = new NotebookService(mockPlatformService.object); const notebookService = new NotebookService(mockPlatformService.object);
const notebookFileName = 'mynotebook.ipynb'; const notebookFileName = 'mynotebook.ipynb';
const sourceNotebookPath = `./notebooks/${notebookFileName}`; const sourceNotebookPath = `./notebooks/${notebookFileName}`;
const expectedFileName = 'mynotebook-2.ipynb'; const expectedFileName = 'mynotebook-2';
const expected1stAttemptTargetFile = path.join(os.homedir(), notebookFileName); const expected1stAttemptTargetFile = 'mynotebook';
const expected2ndAttemptTargetFile = path.join(os.homedir(), 'mynotebook-1.ipynb'); const expected2ndAttemptTargetFile = 'mynotebook-1';
const expectedTargetFile = path.join(os.homedir(), expectedFileName); mockPlatformService.setup((service) => service.isNotebookNameUsed(TypeMoq.It.isAnyString()))
mockPlatformService.setup((service) => service.fileExists(TypeMoq.It.isAnyString()))
.returns((path) => { .returns((path) => {
// list all the possible values here and handle them // list all the possible values here and handle them
// if we only handle the expected value and return true for anything else, the test might run forever until times out // if we only handle the expected value and return true for anything else, the test might run forever until times out
@@ -123,8 +92,8 @@ suite('Notebook Service Tests', function (): void {
} }
return false; return false;
}); });
const actualFileName = notebookService.getTargetNotebookFileName(sourceNotebookPath, os.homedir()); const actualFileName = notebookService.findNextUntitledEditorName(sourceNotebookPath);
mockPlatformService.verify((service) => service.fileExists(TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(3)); mockPlatformService.verify((service) => service.isNotebookNameUsed(TypeMoq.It.isAnyString()), TypeMoq.Times.exactly(3));
assert.equal(actualFileName, expectedTargetFile, 'target file name is not correct'); assert.equal(actualFileName, expectedFileName, 'target file name is not correct');
}); });
}); });

View File

@@ -4485,6 +4485,11 @@ declare module 'azdata' {
* Optional content used to give an initial notebook state * Optional content used to give an initial notebook state
*/ */
initialContent?: nb.INotebookContents | string; initialContent?: nb.INotebookContents | string;
/**
* A optional boolean value indicating the dirty state after the intial content is loaded, default value is true
*/
initialDirtyState?: boolean;
} }
/** /**

View File

@@ -460,6 +460,9 @@ export class MainThreadNotebookDocumentsAndEditors extends Disposable implements
let untitledModel = await input.textInput.resolve(); let untitledModel = await input.textInput.resolve();
await untitledModel.load(); await untitledModel.load();
input.untitledEditorModel = untitledModel; input.untitledEditorModel = untitledModel;
if (options.initialDirtyState === false) {
input.untitledEditorModel.setDirty(false);
}
} }
let editor = await this._editorService.openEditor(input, editorOptions, viewColumnToEditorGroup(this._editorGroupService, options.position)); let editor = await this._editorService.openEditor(input, editorOptions, viewColumnToEditorGroup(this._editorGroupService, options.position));
if (!editor) { if (!editor) {

View File

@@ -206,6 +206,7 @@ export class ExtHostNotebookDocumentsAndEditors implements ExtHostNotebookDocume
options.initialContent = showOptions.initialContent; options.initialContent = showOptions.initialContent;
} }
} }
options.initialDirtyState = showOptions.initialDirtyState;
} }
let id = await this._proxy.$tryShowNotebookDocument(uri, options); let id = await this._proxy.$tryShowNotebookDocument(uri, options);
let editor = this.getEditor(id); let editor = this.getEditor(id);

View File

@@ -908,6 +908,7 @@ export interface INotebookShowOptions {
connectionProfile?: azdata.IConnectionProfile; connectionProfile?: azdata.IConnectionProfile;
defaultKernel?: azdata.nb.IKernelSpec; defaultKernel?: azdata.nb.IKernelSpec;
initialContent?: string; initialContent?: string;
initialDirtyState?: boolean;
} }
export interface ExtHostNotebookDocumentsAndEditorsShape { export interface ExtHostNotebookDocumentsAndEditorsShape {