From 21cf89fb7ed79aecec51bd4eabfdabe4669403dd Mon Sep 17 00:00:00 2001 From: Maddy <12754347+MaddyDev@users.noreply.github.com> Date: Wed, 22 Jul 2020 22:23:37 -0700 Subject: [PATCH] Fix/open book error (#11379) * add isNotebook param and showPreview option * showPreview changes * update OpenNotebookFolder to open a specific path * added test for showPreviewFile * test name typo * remove isNotebook from openBook --- extensions/notebook/src/book/bookTreeView.ts | 76 ++++++++++++------- extensions/notebook/src/extension.ts | 2 +- .../notebook/src/test/book/book.test.ts | 24 +++++- 3 files changed, 74 insertions(+), 28 deletions(-) diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index d1854fc3bf..17598f634c 100644 --- a/extensions/notebook/src/book/bookTreeView.ts +++ b/extensions/notebook/src/book/bookTreeView.ts @@ -190,17 +190,37 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { if (this.currentBook) { - const bookRoot = this.currentBook.bookItems[0]; - const sectionToOpen = bookRoot.findChildSection(urlToOpen); - const urlPath = sectionToOpen ? sectionToOpen.url : bookRoot.tableOfContents.sections[0].url; - const sectionToOpenMarkdown: string = path.join(this.currentBook.bookPath, Content, urlPath.concat('.md')); - // The Notebook editor expects a posix path for the resource (it will still resolve to the correct fsPath based on OS) - const sectionToOpenNotebook: string = path.posix.join(this.currentBook.bookPath, Content, urlPath.concat('.ipynb')); - if (await fs.pathExists(sectionToOpenMarkdown)) { - this.openMarkdown(sectionToOpenMarkdown); + let urlPath: string; + if (this.currentBook.isNotebook) { + urlPath = urlToOpen && this.currentBook.bookPath === urlToOpen ? this.currentBook.bookPath : undefined; + } else { + if (urlToOpen) { + const bookRoot = this.currentBook.bookItems[0]; + const sectionToOpen = bookRoot.findChildSection(urlToOpen); + urlPath = sectionToOpen?.url; + } else { + urlPath = this.currentBook.bookItems[0].tableOfContents.sections[0].url; + } } - else if (await fs.pathExists(sectionToOpenNotebook)) { - await this.openNotebook(sectionToOpenNotebook); + if (urlPath) { + if (this.currentBook.isNotebook) { + if (urlPath.endsWith('.md')) { + this.openMarkdown(urlPath); + } + else if (urlPath.endsWith('.ipynb')) { + await this.openNotebook(urlPath); + } + } else { + // The Notebook editor expects a posix path for the resource (it will still resolve to the correct fsPath based on OS) + const sectionToOpenMarkdown: string = path.posix.join(this.currentBook.bookPath, Content, urlPath.concat('.md')); + const sectionToOpenNotebook: string = path.posix.join(this.currentBook.bookPath, Content, urlPath.concat('.ipynb')); + if (await fs.pathExists(sectionToOpenMarkdown)) { + this.openMarkdown(sectionToOpenMarkdown); + } + else if (await fs.pathExists(sectionToOpenNotebook)) { + await this.openNotebook(sectionToOpenNotebook); + } + } } } } @@ -383,29 +403,33 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - const allFilesFilter = loc.allFiles; - let filter: any = {}; - filter[allFilesFilter] = '*'; - let uris = await vscode.window.showOpenDialog({ - filters: filter, - canSelectFiles: false, - canSelectMany: false, - canSelectFolders: true, - openLabel: loc.labelSelectFolder - }); - if (uris && uris.length > 0) { - await this.loadNotebooksInFolder(uris[0]?.fsPath); + public async openNotebookFolder(folderPath?: string, urlToOpen?: string, showPreview?: boolean): Promise { + if (!folderPath) { + const allFilesFilter = loc.allFiles; + let filter: any = {}; + filter[allFilesFilter] = '*'; + let uris = await vscode.window.showOpenDialog({ + filters: filter, + canSelectFiles: false, + canSelectMany: false, + canSelectFolders: true, + openLabel: loc.labelSelectFolder + }); + folderPath = uris && uris.length > 0 ? uris[0].fsPath : undefined; + } + + if (folderPath) { + await this.loadNotebooksInFolder(folderPath, urlToOpen, showPreview); } } - public async loadNotebooksInFolder(folderPath: string) { + public async loadNotebooksInFolder(folderPath: string, urlToOpen?: string, showPreview?: boolean) { let bookCollection = await this.getNotebooksInTree(folderPath); for (let i = 0; i < bookCollection.bookPaths.length; i++) { - await this.openBook(bookCollection.bookPaths[i], undefined, false); + await this.openBook(bookCollection.bookPaths[i], urlToOpen, showPreview); } for (let i = 0; i < bookCollection.notebookPaths.length; i++) { - await this.openBook(bookCollection.notebookPaths[i], undefined, false, true); + await this.openBook(bookCollection.notebookPaths[i], urlToOpen, showPreview, true); } } diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index f9597cecbf..4c2241d0fa 100644 --- a/extensions/notebook/src/extension.ts +++ b/extensions/notebook/src/extension.ts @@ -35,7 +35,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openBook', () => bookTreeViewProvider.openNewBook())); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.closeBook', (book: any) => bookTreeViewProvider.closeBook(book))); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.closeNotebook', (book: any) => bookTreeViewProvider.closeBook(book))); - extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openNotebookFolder', () => bookTreeViewProvider.openNotebookFolder())); + extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openNotebookFolder', (folderPath?: string, urlToOpen?: string, showPreview?: boolean,) => bookTreeViewProvider.openNotebookFolder(folderPath, urlToOpen, showPreview))); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.createBook', async () => { let untitledFileName: vscode.Uri = vscode.Uri.parse(`untitled:${createBookPath}`); diff --git a/extensions/notebook/src/test/book/book.test.ts b/extensions/notebook/src/test/book/book.test.ts index a6156bc9e1..cd7486f1ef 100644 --- a/extensions/notebook/src/test/book/book.test.ts +++ b/extensions/notebook/src/test/book/book.test.ts @@ -525,8 +525,21 @@ describe('BooksTreeViewTests', function () { it('should add book and initialize book on openBook', async () => { should(bookTreeViewProvider.books.length).equal(0, 'Invalid books on initialize.'); + + let showPreviewSpy = sinon.spy(bookTreeViewProvider, 'showPreviewFile'); + await bookTreeViewProvider.openBook(rootFolderPath); should(bookTreeViewProvider.books.length).equal(1, 'Failed to initialize the book on open'); + should(showPreviewSpy.notCalled).be.true('Should not call showPreviewFile when showPreview isn\' true'); + }); + + it('should call showPreviewFile on openBook when showPreview flag is set', async () => { + await bookTreeViewProvider.closeBook(bookTreeViewProvider.books[0].bookItems[0]); + let showPreviewSpy = sinon.spy(bookTreeViewProvider, 'showPreviewFile'); + + await bookTreeViewProvider.openBook(rootFolderPath, undefined, true); + should(bookTreeViewProvider.books.length).equal(1, 'Failed to initialize the book on open'); + should(showPreviewSpy.calledOnce).be.true('Should have called showPreviewFile.'); }); it('should add book when bookPath contains special characters on openBook', async () => { @@ -554,11 +567,20 @@ describe('BooksTreeViewTests', function () { should(notebookUri.scheme).equal('untitled', 'Failed to get untitled uri of the resource'); }); - it('openNotebookFolder should prompt for notebook path and invoke loadNotebooksInFolder', async () => { + it('openNotebookFolder without folderPath should prompt for folder path and invoke loadNotebooksInFolder', async () => { sinon.stub(vscode.window, 'showOpenDialog').returns(Promise.resolve([vscode.Uri.file(rootFolderPath)])); let loadNotebooksSpy = sinon.spy(bookTreeViewProvider, 'loadNotebooksInFolder'); await bookTreeViewProvider.openNotebookFolder(); + should(loadNotebooksSpy.calledWith(rootFolderPath)).be.true('openNotebookFolder should have called loadNotebooksInFolder passing the folderPath'); + }); + + it('openNotebookFolder with folderPath shouldn\'t prompt for folder path but invoke loadNotebooksInFolder with the provided folderPath', async () => { + let showOpenDialogSpy = sinon.spy(vscode.window, 'showOpenDialog'); + let loadNotebooksSpy = sinon.spy(bookTreeViewProvider, 'loadNotebooksInFolder'); + await bookTreeViewProvider.openNotebookFolder(rootFolderPath); + + should(showOpenDialogSpy.notCalled).be.true('openNotebookFolder with folderPath shouldn\'t prompt to select folder'); should(loadNotebooksSpy.calledOnce).be.true('openNotebookFolder should have called loadNotebooksInFolder'); });