diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index 175d6299c5..08429a2344 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -186,7 +186,7 @@ } }, { - "command": "notebook.command.searchUntitledBook", + "command": "notebook.command.searchProvidedBook", "title": "%title.searchJupyterBook%", "category": "%books-preview-category%", "icon": { @@ -319,7 +319,7 @@ "when": "false" }, { - "command": "notebook.command.searchUntitledBook", + "command": "notebook.command.searchProvidedBook", "when": "false" }, { @@ -381,7 +381,7 @@ "group": "inline" }, { - "command": "notebook.command.searchUntitledBook", + "command": "notebook.command.searchProvidedBook", "when": "view == providedBooksView && viewItem == providedBook && providedBooks", "group": "inline" }, diff --git a/extensions/notebook/src/book/bookModel.ts b/extensions/notebook/src/book/bookModel.ts index 5220cac58c..d0dbd4f78b 100644 --- a/extensions/notebook/src/book/bookModel.ts +++ b/extensions/notebook/src/book/bookModel.ts @@ -3,7 +3,6 @@ * Licensed under the Source EULA. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -import * as azdata from 'azdata'; import * as vscode from 'vscode'; import * as yaml from 'js-yaml'; import { BookTreeItem, BookTreeItemType } from './bookTreeItem'; @@ -17,11 +16,10 @@ import { ApiWrapper } from '../common/apiWrapper'; const fsPromises = fileServices.promises; -export class BookModel implements azdata.nb.NavigationProvider { +export class BookModel { private _bookItems: BookTreeItem[]; private _allNotebooks = new Map(); private _tableOfContentsPath: string; - readonly providerId: string = 'BookNavigator'; private _errorMessage: string; private apiWrapper: ApiWrapper = new ApiWrapper(); @@ -32,7 +30,6 @@ export class BookModel implements azdata.nb.NavigationProvider { public readonly isNotebook: boolean, private _extensionContext: vscode.ExtensionContext) { this._bookItems = []; - this._extensionContext.subscriptions.push(azdata.nb.registerNavigationProvider(this)); } public async initializeContents(): Promise { @@ -251,30 +248,6 @@ export class BookModel implements azdata.nb.NavigationProvider { return this._tableOfContentsPath; } - getNavigation(uri: vscode.Uri): Thenable { - let notebook = !this.openAsUntitled ? this._allNotebooks.get(uri.fsPath) : this._allNotebooks.get(path.basename(uri.fsPath)); - let result: azdata.nb.NavigationResult; - if (notebook) { - result = { - hasNavigation: true, - previous: notebook.previousUri ? - this.openAsUntitled ? this.getUntitledUri(notebook.previousUri) : vscode.Uri.file(notebook.previousUri) : undefined, - next: notebook.nextUri ? this.openAsUntitled ? this.getUntitledUri(notebook.nextUri) : vscode.Uri.file(notebook.nextUri) : undefined - }; - } else { - result = { - hasNavigation: false, - previous: undefined, - next: undefined - }; - } - return Promise.resolve(result); - } - - getUntitledUri(resource: string): vscode.Uri { - return vscode.Uri.parse(`untitled:${resource}`); - } - public get errorMessage(): string { return this._errorMessage; } diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index 6a34c66dac..bcc558eb0c 100644 --- a/extensions/notebook/src/book/bookTreeView.ts +++ b/extensions/notebook/src/book/bookTreeView.ts @@ -27,7 +27,7 @@ interface BookSearchResults { bookPaths: string[]; } -export class BookTreeViewProvider implements vscode.TreeDataProvider { +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; @@ -43,7 +43,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - await vscode.commands.executeCommand('setContext', 'providedBooks', this._openAsUntitled); await Promise.all(workspaceFolders.map(async (workspaceFolder) => { try { await this.loadNotebooksInFolder(workspaceFolder.uri.fsPath); @@ -204,8 +205,9 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { try { + await vscode.commands.executeCommand(constants.BuiltInCommands.SetContext, constants.unsavedBooksContextKey, false); if (this._openAsUntitled) { - this.openNotebookAsUntitled(resource); + await this.openNotebookAsUntitled(resource); } else { // let us keep a list of already visited notebooks so that we do not trust them again, potentially // overriding user changes @@ -256,8 +258,9 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { 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(); @@ -493,4 +496,29 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { + let result: azdata.nb.NavigationResult; + let notebook = this.currentBook?.getNotebook(uri.fsPath); + if (notebook) { + result = { + hasNavigation: true, + previous: notebook.previousUri ? + this.currentBook?.openAsUntitled ? this.getUntitledNotebookUri(notebook.previousUri) : vscode.Uri.file(notebook.previousUri) : undefined, + next: notebook.nextUri ? this.currentBook?.openAsUntitled ? this.getUntitledNotebookUri(notebook.nextUri) : vscode.Uri.file(notebook.nextUri) : undefined + }; + } else { + result = { + hasNavigation: false, + previous: undefined, + next: undefined + }; + } + return Promise.resolve(result); + } + + public getBookFromItemPath(itemPath: string): BookModel | undefined { + let selectedBook = this.books.find(b => itemPath.toLowerCase().indexOf(b.bookPath.toLowerCase()) > -1); + return selectedBook; + } } diff --git a/extensions/notebook/src/common/constants.ts b/extensions/notebook/src/common/constants.ts index b3f0e6491f..9a81283c74 100644 --- a/extensions/notebook/src/common/constants.ts +++ b/extensions/notebook/src/common/constants.ts @@ -56,6 +56,13 @@ export enum PythonPkgType { Anaconda = 'Anaconda' } +export enum NavigationProviders { + NotebooksNavigator = 'BookNavigator.Notebooks', + ProvidedBooksNavigator = 'BookNavigator.ProvidedBooks' +} + +export const unsavedBooksContextKey = 'unsavedBooks'; + export const pythonWindowsInstallUrl = 'https://go.microsoft.com/fwlink/?linkid=2110625'; export const pythonMacInstallUrl = 'https://go.microsoft.com/fwlink/?linkid=2128152'; export const pythonLinuxInstallUrl = 'https://go.microsoft.com/fwlink/?linkid=2110524'; diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index ecc51607d1..4bf531b47c 100644 --- a/extensions/notebook/src/extension.ts +++ b/extensions/notebook/src/extension.ts @@ -15,6 +15,7 @@ import { IExtensionApi, IPackageManageProvider } from './types'; import { CellType } from './contracts/content'; import { NotebookUriHandler } from './protocol/notebookUriHandler'; import { BookTreeViewProvider } from './book/bookTreeView'; +import { NavigationProviders } from './common/constants'; import { newNotebook, openNotebook, runActiveCell, runAllCells, clearActiveCellOutput, addCell, analyzeNotebook } from './common/notebookUtils'; const localize = nls.loadMessageBundle(); @@ -34,7 +35,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.saveBook', () => providedBookTreeViewProvider.saveJupyterBooks())); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.trustBook', (resource) => bookTreeViewProvider.trustBook(resource))); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.searchBook', (item) => bookTreeViewProvider.searchJupyterBooks(item))); - extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.searchUntitledBook', () => providedBookTreeViewProvider.searchJupyterBooks())); + extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.searchProvidedBook', () => providedBookTreeViewProvider.searchJupyterBooks())); 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))); @@ -109,9 +110,6 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi await vscode.commands.executeCommand('vscode.open', vscode.Uri.parse(urlToOpen)); })); - extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.revealInBooksViewlet', (uri: vscode.Uri, shouldReveal: boolean) => bookTreeViewProvider.revealActiveDocumentInViewlet(uri, shouldReveal))); - extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.revealInUntitledBooksViewlet', (uri: vscode.Uri, shouldReveal: boolean) => providedBookTreeViewProvider.revealActiveDocumentInViewlet(uri, shouldReveal))); - let appContext = new AppContext(extensionContext, new ApiWrapper()); controller = new JupyterController(appContext); let result = await controller.activate(); @@ -120,9 +118,9 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi } let workspaceFolders = vscode.workspace.workspaceFolders?.slice() ?? []; - const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, workspaceFolders, extensionContext, false, BOOKS_VIEWID); + const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, workspaceFolders, extensionContext, false, BOOKS_VIEWID, NavigationProviders.NotebooksNavigator); await bookTreeViewProvider.initialized; - const providedBookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], extensionContext, true, PROVIDED_BOOKS_VIEWID); + const providedBookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], extensionContext, true, PROVIDED_BOOKS_VIEWID, NavigationProviders.ProvidedBooksNavigator); await providedBookTreeViewProvider.initialized; azdata.nb.onDidChangeActiveNotebookEditor(e => { diff --git a/extensions/notebook/src/test/book/book.test.ts b/extensions/notebook/src/test/book/book.test.ts index c9485d1ee7..c6346b571b 100644 --- a/extensions/notebook/src/test/book/book.test.ts +++ b/extensions/notebook/src/test/book/book.test.ts @@ -19,6 +19,7 @@ import { AppContext } from '../../common/appContext'; import { ApiWrapper } from '../../common/apiWrapper'; import { BookModel } from '../../book/bookModel'; import { BookTrustManager } from '../../book/bookTrustManager'; +import { NavigationProviders } from '../../common/constants'; export interface IExpectedBookItem { title: string; @@ -119,7 +120,7 @@ describe('BookTreeViewProviderTests', function () { }); it('should initialize correctly with empty workspace array', async () => { - const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, false, 'bookTreeView'); + const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); await bookTreeViewProvider.initialized; }); @@ -129,7 +130,7 @@ describe('BookTreeViewProviderTests', function () { name: '', index: 0 }; - const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView'); + const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); await bookTreeViewProvider.initialized; }); @@ -144,13 +145,14 @@ describe('BookTreeViewProviderTests', function () { name: '', index: 0 }; - const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [book, nonBook], mockExtensionContext, false, 'bookTreeView'); + const bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [book, nonBook], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); await bookTreeViewProvider.initialized; should(bookTreeViewProvider.books.length).equal(1, 'Expected book was not initialized'); }); - describe('BookTreeViewProvider.getChildren', function (): void { + describe('getChildren', function (): void { let bookTreeViewProvider: BookTreeViewProvider; + let providedbookTreeViewProvider: BookTreeViewProvider; let book: BookTreeItem; let notebook1: BookTreeItem; @@ -160,12 +162,15 @@ describe('BookTreeViewProviderTests', function () { name: '', index: 0 }; - bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView'); + bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); + providedbookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, true, 'providedBooksView', NavigationProviders.ProvidedBooksNavigator); let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000)); await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); + await Promise.race([providedbookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('ProvidedBooksTreeViewProvider did not initialize in time'); })]); + await providedbookTreeViewProvider.openBook(bookFolderPath, undefined, false, false); }); - it('should return all book nodes when element is undefined', async function (): Promise { + it('bookTreeViewProvider should return all book nodes when element is undefined', async function (): Promise { const children = await bookTreeViewProvider.getChildren(); should(children).be.Array(); should(children.length).equal(1); @@ -173,7 +178,7 @@ describe('BookTreeViewProviderTests', function () { should(book.title).equal(expectedBook.title); }); - it('should return all page nodes when element is a book', async function (): Promise { + it('bookTreeViewProvider should return all page nodes when element is a book', async function (): Promise { const children = await bookTreeViewProvider.getChildren(book); should(children).be.Array(); should(children.length).equal(3); @@ -185,7 +190,7 @@ describe('BookTreeViewProviderTests', function () { equalBookItems(externalLink, expectedExternalLink); }); - it('should return all sections when element is a notebook', async function (): Promise { + it('bookTreeViewProvider should return all sections when element is a notebook', async function (): Promise { const children = await bookTreeViewProvider.getChildren(notebook1); should(children).be.Array(); should(children.length).equal(2); @@ -195,7 +200,7 @@ describe('BookTreeViewProviderTests', function () { equalBookItems(notebook3, expectedNotebook3); }); - it('should set notebooks trusted to true on trustBook', async () => { + it('bookTreeViewProvider should set notebooks trusted to true on trustBook', async () => { let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb'); let bookTrustManager: BookTrustManager = new BookTrustManager(bookTreeViewProvider.books, appContext.apiWrapper); let isTrusted = bookTrustManager.isNotebookTrustedByDefault(vscode.Uri.file(notebook1Path).fsPath); @@ -207,25 +212,65 @@ describe('BookTreeViewProviderTests', function () { }); - it('getNavigation should get previous and next urls correctly from the bookModel', async () => { + it('bookTreeViewProvider getNavigation should get previous and next urls correctly from the bookModel', async () => { let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb'); let notebook2Path = path.join(rootFolderPath, 'Book', 'content', 'notebook2.ipynb'); let notebook3Path = path.join(rootFolderPath, 'Book', 'content', 'notebook3.ipynb'); - const result = await bookTreeViewProvider.currentBook.getNavigation(vscode.Uri.file(notebook2Path)); + const result = await bookTreeViewProvider.getNavigation(vscode.Uri.file(notebook2Path)); should(result.hasNavigation).be.true('getNavigation failed to get previous and next urls'); should(result.next.fsPath).equal(vscode.Uri.file(notebook3Path).fsPath, 'getNavigation failed to get the next url'); should(result.previous.fsPath).equal(vscode.Uri.file(notebook1Path).fsPath, 'getNavigation failed to get the previous url'); }); - - this.afterAll(async function (): Promise { - console.log('Removing temporary files...'); - if (await exists(rootFolderPath)) { - await promisify(rimraf)(rootFolderPath); - } - console.log('Successfully removed temporary files.'); + it('providedBookTreeViewProvider should return all book nodes when element is undefined', async function (): Promise { + const children = await providedbookTreeViewProvider.getChildren(); + should(children).be.Array(); + should(children.length).equal(1); + book = children[0]; + should(book.title).equal(expectedBook.title); }); + + it('providedBookTreeViewProvider should return all page nodes when element is a book', async function (): Promise { + const children = await providedbookTreeViewProvider.getChildren(book); + should(children).be.Array(); + should(children.length).equal(3); + notebook1 = children[0]; + const markdown = children[1]; + const externalLink = children[2]; + equalBookItems(notebook1, expectedNotebook1); + equalBookItems(markdown, expectedMarkdown); + equalBookItems(externalLink, expectedExternalLink); + }); + + it('providedBookTreeViewProvider should return all sections when element is a notebook', async function (): Promise { + const children = await providedbookTreeViewProvider.getChildren(notebook1); + should(children).be.Array(); + should(children.length).equal(2); + const notebook2 = children[0]; + const notebook3 = children[1]; + equalBookItems(notebook2, expectedNotebook2); + equalBookItems(notebook3, expectedNotebook3); + }); + + it('providedBookTreeViewProvider getNavigation should get previous and next urls correctly from the bookModel', async () => { + let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb'); + let notebook2Path = path.join(rootFolderPath, 'Book', 'content', 'notebook2.ipynb'); + let notebook3Path = path.join(rootFolderPath, 'Book', 'content', 'notebook3.ipynb'); + const result = await providedbookTreeViewProvider.getNavigation(vscode.Uri.file(notebook2Path)); + should(result.hasNavigation).be.true('getNavigation failed to get previous and next urls'); + should(result.next.fsPath).equal(notebook3Path, 'getNavigation failed to get the next url'); + should(result.previous.fsPath).equal(notebook1Path, 'getNavigation failed to get the previous url'); + }); + + }); + + this.afterAll(async function (): Promise { + console.log('Removing temporary files...'); + if (await exists(rootFolderPath)) { + await promisify(rimraf)(rootFolderPath); + } + console.log('Successfully removed temporary files.'); }); }); @@ -253,7 +298,7 @@ describe('BookTreeViewProviderTests', function () { index: 0 }; appContext = new AppContext(mockExtensionContext, new ApiWrapper()); - bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView'); + bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000)); await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); appContext = new AppContext(undefined, new ApiWrapper()); @@ -296,7 +341,7 @@ describe('BookTreeViewProviderTests', function () { index: 0 }; appContext = new AppContext(mockExtensionContext, new ApiWrapper()); - bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView'); + bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000)); await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); appContext = new AppContext(undefined, new ApiWrapper()); @@ -356,7 +401,7 @@ describe('BookTreeViewProviderTests', function () { index: 0 }; appContext = new AppContext(mockExtensionContext, new ApiWrapper()); - bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView'); + bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [folder], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000)); await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); appContext = new AppContext(undefined, new ApiWrapper()); @@ -399,7 +444,7 @@ describe('BookTreeViewProviderTests', function () { const mockExtensionContext = new MockExtensionContext(); appContext = new AppContext(mockExtensionContext, new ApiWrapper()); - bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, false, 'bookTreeView'); + bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000)); await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); appContext = new AppContext(undefined, new ApiWrapper()); @@ -483,7 +528,7 @@ describe('BookTreeViewProviderTests', function () { const mockExtensionContext = new MockExtensionContext(); appContext = new AppContext(mockExtensionContext, new ApiWrapper()); - bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, false, 'bookTreeView'); + bookTreeViewProvider = new BookTreeViewProvider(appContext.apiWrapper, [], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); let errorCase = new Promise((resolve, reject) => setTimeout(() => resolve(), 5000)); await Promise.race([bookTreeViewProvider.initialized, errorCase.then(() => { throw new Error('BookTreeViewProvider did not initialize in time'); })]); appContext = new AppContext(undefined, new ApiWrapper()); diff --git a/src/sql/workbench/contrib/notebook/browser/cellViews/linkHandler.directive.ts b/src/sql/workbench/contrib/notebook/browser/cellViews/linkHandler.directive.ts index 25c20166d5..a4622a1959 100644 --- a/src/sql/workbench/contrib/notebook/browser/cellViews/linkHandler.directive.ts +++ b/src/sql/workbench/contrib/notebook/browser/cellViews/linkHandler.directive.ts @@ -4,7 +4,6 @@ *--------------------------------------------------------------------------------------------*/ import { Directive, Inject, HostListener, Input } from '@angular/core'; - import { URI } from 'vs/base/common/uri'; import { IOpenerService } from 'vs/platform/opener/common/opener'; import { onUnexpectedError } from 'vs/base/common/errors'; diff --git a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts index a1545876d3..563849e4f9 100644 --- a/src/sql/workbench/contrib/notebook/browser/notebook.component.ts +++ b/src/sql/workbench/contrib/notebook/browser/notebook.component.ts @@ -55,7 +55,6 @@ import { CodeCellComponent } from 'sql/workbench/contrib/notebook/browser/cellVi import { TextCellComponent } from 'sql/workbench/contrib/notebook/browser/cellViews/textCell.component'; import { NotebookInput } from 'sql/workbench/contrib/notebook/browser/models/notebookInput'; import { IColorTheme } from 'vs/platform/theme/common/themeService'; -import { ICommandService } from 'vs/platform/commands/common/commands'; import { IAdsTelemetryService } from 'sql/platform/telemetry/common/telemetry'; import { IConfigurationService } from 'vs/platform/configuration/common/configuration'; @@ -105,7 +104,6 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe @Inject(ICapabilitiesService) private capabilitiesService: ICapabilitiesService, @Inject(ITextFileService) private textFileService: ITextFileService, @Inject(ILogService) private readonly logService: ILogService, - @Inject(ICommandService) private commandService: ICommandService, @Inject(IAdsTelemetryService) private adstelemetryService: IAdsTelemetryService, @Inject(IConfigurationService) private _configurationService: IConfigurationService ) { @@ -516,8 +514,6 @@ export class NotebookComponent extends AngularDisposable implements OnInit, OnDe this._navProvider = this.notebookService.getNavigationProvider(this._notebookParams.notebookUri); if (this.contextKeyService.getContextKeyValue('bookOpened') && this._navProvider) { - // If there's a book opened but the current notebook isn't part of the book, this is a no-op - this.commandService.executeCommand('notebook.command.revealInBooksViewlet', this._notebookParams.notebookUri, false); this._navProvider.getNavigation(this._notebookParams.notebookUri).then(result => { this.navigationResult = result; this.addButton(localize('previousButtonLabel', "< Previous"), diff --git a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts index cc38985813..c1571a6f4b 100644 --- a/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts +++ b/src/sql/workbench/contrib/notebook/test/browser/notebookService.test.ts @@ -16,6 +16,7 @@ import { INotebookService } from 'sql/workbench/services/notebook/browser/notebo import { Emitter } from 'vs/base/common/event'; import { Registry } from 'vs/platform/registry/common/platform'; import { NotebookProviderRegistration, INotebookProviderRegistry, Extensions } from 'sql/workbench/services/notebook/common/notebookRegistry'; +import { MockContextKeyService } from 'vs/platform/keybinding/test/common/mockKeybindingService'; suite('Notebook Service Tests', function (): void { let notebookService: INotebookService; @@ -31,6 +32,7 @@ suite('Notebook Service Tests', function (): void { const extensionService = new TestExtensionService(); const fileService = new TestFileService(); const logService = new NullLogService(); + const contextService = new MockContextKeyService(); const queryManagementService = new NBTestQueryManagementService(); const instantiationService = new TestInstantiationService(); @@ -47,7 +49,7 @@ suite('Notebook Service Tests', function (): void { instantiationService.stub(IExtensionManagementService, 'onDidUninstallExtension', didUninstallEvent.event); const extensionManagementService = instantiationService.get(IExtensionManagementService); - notebookService = new NotebookService(lifecycleService, storageService, extensionService, extensionManagementService, instantiationService, fileService, logService, queryManagementService); + notebookService = new NotebookService(lifecycleService, storageService, extensionService, extensionManagementService, instantiationService, fileService, logService, queryManagementService, contextService); }); test('Validate default properties on create', async function (): Promise { diff --git a/src/sql/workbench/services/notebook/browser/notebookService.ts b/src/sql/workbench/services/notebook/browser/notebookService.ts index 03359db6db..46800ac684 100644 --- a/src/sql/workbench/services/notebook/browser/notebookService.ts +++ b/src/sql/workbench/services/notebook/browser/notebookService.ts @@ -34,6 +34,16 @@ export interface ILanguageMagic { executionTarget?: string; } +/** +* Valid navigation providers. +*/ +export enum NavigationProviders { + NotebooksNavigator = 'BookNavigator.Notebooks', + ProvidedBooksNavigator = 'BookNavigator.ProvidedBooks' +} + +export const unsavedBooksContextKey = 'unsavedBooks'; + export interface INotebookService { _serviceBrand: undefined; diff --git a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts index 9aee89d546..9bdac99513 100644 --- a/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts +++ b/src/sql/workbench/services/notebook/browser/notebookServiceImpl.ts @@ -10,7 +10,7 @@ import { Registry } from 'vs/platform/registry/common/platform'; import { INotebookService, INotebookManager, INotebookProvider, - DEFAULT_NOTEBOOK_FILETYPE, INotebookEditor, SQL_NOTEBOOK_PROVIDER, INavigationProvider, ILanguageMagic + DEFAULT_NOTEBOOK_FILETYPE, INotebookEditor, SQL_NOTEBOOK_PROVIDER, INavigationProvider, ILanguageMagic, NavigationProviders, unsavedBooksContextKey } from 'sql/workbench/services/notebook/browser/notebookService'; import { RenderMimeRegistry } from 'sql/workbench/services/notebook/browser/outputs/registry'; import { standardRendererFactories } from 'sql/workbench/services/notebook/browser/outputs/factories'; @@ -36,6 +36,7 @@ import { NotebookChangeType } from 'sql/workbench/services/notebook/common/contr import { find, firstIndex } from 'vs/base/common/arrays'; import { onUnexpectedError } from 'vs/base/common/errors'; import { notebookConstants } from 'sql/workbench/services/notebook/browser/interfaces'; +import { IContextKeyService } from 'vs/platform/contextkey/common/contextkey'; export interface NotebookProviderProperties { provider: string; @@ -115,7 +116,8 @@ export class NotebookService extends Disposable implements INotebookService { @IInstantiationService private _instantiationService: IInstantiationService, @IFileService private readonly _fileService: IFileService, @ILogService private readonly _logService: ILogService, - @IQueryManagementService private readonly _queryManagementService: IQueryManagementService + @IQueryManagementService private readonly _queryManagementService: IQueryManagementService, + @IContextKeyService private contextKeyService: IContextKeyService, ) { super(); this._providersMemento = new Memento('notebookProviders', this._storageService); @@ -220,7 +222,11 @@ export class NotebookService extends Disposable implements INotebookService { } getNavigationProvider(): INavigationProvider { - let provider = this._navigationProviders.size > 0 ? this._navigationProviders.values().next().value : undefined; + let provider; + if (this._navigationProviders.size > 0) { + const providerName = this.contextKeyService.getContextKeyValue(unsavedBooksContextKey) ? NavigationProviders.ProvidedBooksNavigator : NavigationProviders.NotebooksNavigator; + provider = this._navigationProviders.get(providerName); + } return provider; }