diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index 17598f634c..c77a9afaa0 100644 --- a/extensions/notebook/src/book/bookTreeView.ts +++ b/extensions/notebook/src/book/bookTreeView.ts @@ -29,8 +29,6 @@ interface BookSearchResults { export class BookTreeViewProvider implements vscode.TreeDataProvider, azdata.nb.NavigationProvider { private _onDidChangeTreeData: vscode.EventEmitter = new vscode.EventEmitter(); readonly onDidChangeTreeData: vscode.Event = this._onDidChangeTreeData.event; - private _throttleTimer: any; - private _resource: string; private _extensionContext: vscode.ExtensionContext; private prompter: IPrompter; private _initializeDeferred: Deferred = new Deferred(); @@ -231,19 +229,17 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - document.setTrusted(true); - this._visitedNotebooks = this._visitedNotebooks.concat([normalizedResource]); - openDocumentListenerUnsubscriber.dispose(); - }); + let document = azdata.nb.notebookDocuments.find(document => document.fileName === resource); + document?.setTrusted(true); + this._visitedNotebooks = this._visitedNotebooks.concat([normalizedResource]); } - azdata.nb.showNotebookDocument(vscode.Uri.file(resource)); } } catch (e) { vscode.window.showErrorMessage(loc.openNotebookError(resource, e instanceof Error ? e.message : e)); @@ -297,26 +293,22 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - try { - vscode.commands.executeCommand('markdown.showPreview', vscode.Uri.file(resource)); - } catch (e) { - vscode.window.showErrorMessage(loc.openMarkdownError(resource, e instanceof Error ? e.message : e)); - } - }); + try { + vscode.commands.executeCommand('markdown.showPreview', vscode.Uri.file(resource)); + } catch (e) { + vscode.window.showErrorMessage(loc.openMarkdownError(resource, e instanceof Error ? e.message : e)); + } } async openNotebookAsUntitled(resource: string): Promise { try { await vscode.commands.executeCommand(constants.BuiltInCommands.SetContext, constants.unsavedBooksContextKey, true); let untitledFileName: vscode.Uri = this.getUntitledNotebookUri(resource); - vscode.workspace.openTextDocument(resource).then((document) => { - let initialContent = document.getText(); - azdata.nb.showNotebookDocument(untitledFileName, { - connectionProfile: null, - initialContent: initialContent, - initialDirtyState: false - }); + let document: vscode.TextDocument = await vscode.workspace.openTextDocument(resource); + await azdata.nb.showNotebookDocument(untitledFileName, { + connectionProfile: null, + initialContent: document.getText(), + initialDirtyState: false }); } catch (e) { vscode.window.showErrorMessage(loc.openUntitledNotebookError(resource, e instanceof Error ? e.message : e)); @@ -455,35 +447,9 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider void) { - const isResourceChange = resource !== this._resource; - if (isResourceChange) { - this.clearAndResetThrottleTimer(); - } - - this._resource = resource; - - // Schedule update if none is pending - if (!this._throttleTimer) { - if (isResourceChange) { - action(); - } else { - this._throttleTimer = setTimeout(() => { - action(); - this.clearAndResetThrottleTimer(); - }, 300); - } - } - } - - private clearAndResetThrottleTimer(): void { - clearTimeout(this._throttleTimer); - this._throttleTimer = undefined; - } - - openExternalLink(resource: string): void { + async openExternalLink(resource: string): Promise { try { - vscode.env.openExternal(vscode.Uri.parse(resource)); + await vscode.env.openExternal(vscode.Uri.parse(resource)); } catch (e) { vscode.window.showErrorMessage(loc.openExternalLinkError(resource, e instanceof Error ? e.message : e)); } diff --git a/extensions/notebook/src/test/book/book.test.ts b/extensions/notebook/src/test/book/book.test.ts index cd7486f1ef..6755744ba3 100644 --- a/extensions/notebook/src/test/book/book.test.ts +++ b/extensions/notebook/src/test/book/book.test.ts @@ -170,7 +170,7 @@ describe('BooksTreeViewTests', function () { await bookTreeViewProvider.openBook(bookFolderPath, undefined, false, false); }); - afterEach(function(): void { + afterEach(function (): void { sinon.restore(); }); @@ -276,7 +276,7 @@ describe('BooksTreeViewTests', function () { await providedbookTreeViewProvider.openBook(bookFolderPath, undefined, false, false); }); - afterEach(function(): void { + afterEach(function (): void { sinon.restore(); }); @@ -505,13 +505,17 @@ describe('BooksTreeViewTests', function () { let contentFolderPath = path.join(rootFolderPath, 'content'); let configFile = path.join(rootFolderPath, '_config.yml'); tableOfContentsFile = path.join(dataFolderPath, 'toc.yml'); + let notebook1File = path.join(contentFolderPath, 'notebook1.ipynb'); let notebook2File = path.join(contentFolderPath, 'notebook2.ipynb'); + let markdownFile = path.join(contentFolderPath, 'readme.md'); await fs.mkdir(rootFolderPath); await fs.mkdir(dataFolderPath); await fs.mkdir(contentFolderPath); await fs.writeFile(configFile, 'title: Test Book'); - await fs.writeFile(tableOfContentsFile, '- title: Notebook1\n url: /notebook1\n- title: Notebook2\n url: /notebook2'); + await fs.writeFile(tableOfContentsFile, '- title: Home\n url: /readme\n- title: Notebook1\n url: /notebook1\n- title: Notebook2\n url: /notebook2'); + await fs.writeFile(notebook1File, ''); await fs.writeFile(notebook2File, ''); + await fs.writeFile(markdownFile, ''); const mockExtensionContext = new MockExtensionContext(); bookTreeViewProvider = new BookTreeViewProvider([], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); @@ -519,8 +523,9 @@ describe('BooksTreeViewTests', function () { await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); }); - afterEach(function(): void { + afterEach(async function (): Promise { sinon.restore(); + await vscode.commands.executeCommand('workbench.action.closeAllEditors'); }); it('should add book and initialize book on openBook', async () => { @@ -533,6 +538,34 @@ describe('BooksTreeViewTests', function () { should(showPreviewSpy.notCalled).be.true('Should not call showPreviewFile when showPreview isn\' true'); }); + it('openMarkdown should open markdown in the editor', async () => { + let executeCommandSpy = sinon.spy(vscode.commands, 'executeCommand'); + let notebookPath = path.join(rootFolderPath, 'content', 'readme.md'); + bookTreeViewProvider.openMarkdown(notebookPath); + should(executeCommandSpy.calledWith('markdown.showPreview')).be.true('openMarkdown should have called markdown.showPreview'); + }); + + // TODO: Need to investigate why it's failing on linux. + it.skip('openNotebook should open notebook in the editor', async () => { + let showNotebookSpy = sinon.spy(azdata.nb, 'showNotebookDocument'); + let notebookPath = path.join(rootFolderPath, 'content', 'notebook2.ipynb'); + await bookTreeViewProvider.openNotebook(notebookPath); + should(showNotebookSpy.calledWith(vscode.Uri.file(notebookPath))).be.true(`Should have opened the notebook from ${notebookPath} in the editor.`); + }); + + it('openNotebookAsUntitled should open a notebook as untitled file in the editor', async () => { + let notebookPath = path.join(rootFolderPath, 'content', 'notebook2.ipynb'); + await bookTreeViewProvider.openNotebookAsUntitled(notebookPath); + should(azdata.nb.notebookDocuments.find(doc => doc.uri.scheme === 'untitled')).not.be.undefined(); + }); + + it('openExternalLink should open link', async () => { + let executeCommandSpy = sinon.spy(vscode.commands, 'executeCommand'); + let notebookPath = path.join(rootFolderPath, 'content', 'readme.md'); + bookTreeViewProvider.openMarkdown(notebookPath); + should(executeCommandSpy.calledWith('markdown.showPreview')).be.true('openMarkdown should have called markdown.showPreview'); + }); + 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'); @@ -612,7 +645,7 @@ describe('BooksTreeViewTests', function () { it('should remove book on closeBook', async () => { let length: number = bookTreeViewProvider.books.length; await bookTreeViewProvider.closeBook(bookTreeViewProvider.books[0].bookItems[0]); - should(bookTreeViewProvider.books.length).equal(length-1, 'Failed to remove the book on close'); + should(bookTreeViewProvider.books.length).equal(length - 1, 'Failed to remove the book on close'); }); this.afterAll(async function (): Promise {