ckaczor/azuredatastudio
1
0
Fork 0
mirror of https://github.com/ckaczor/azuredatastudio.git synced 2026-02-16 10:58:30 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix/prev next links issue (#10523)

Browse Source 
* intital test code

* added tests

* remove commented code

* fix for failing tests

* reuse exported enum

* changes to address comments

* add back onVisibility highlight

* port highlight fix from Chris

* fix tests
This commit is contained in:
Maddy
2020-06-12 18:42:17 -07:00
committed by GitHub
parent c9569d8573
commit 26a00696d4
11 changed files with 138 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

29
extensions/notebook/src/book/bookModel.ts
View File

@@ -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<string, BookTreeItem>();
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<void> {
@@ -251,30 +248,6 @@ export class BookModel implements azdata.nb.NavigationProvider {
return this._tableOfContentsPath;
}
getNavigation(uri: vscode.Uri): Thenable<azdata.nb.NavigationResult> {
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;
}

38
extensions/notebook/src/book/bookTreeView.ts
View File

@@ -27,7 +27,7 @@ interface BookSearchResults {
bookPaths: string[];
}
export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeItem> {
export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeItem>, azdata.nb.NavigationProvider {
private _onDidChangeTreeData: vscode.EventEmitter<BookTreeItem | undefined> = new vscode.EventEmitter<BookTreeItem | undefined>();
readonly onDidChangeTreeData: vscode.Event<BookTreeItem | undefined> = this._onDidChangeTreeData.event;
private _throttleTimer: any;
@@ -43,7 +43,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
public books: BookModel[];
public currentBook: BookModel;
constructor(private _apiWrapper: ApiWrapper, workspaceFolders: vscode.WorkspaceFolder[], extensionContext: vscode.ExtensionContext, openAsUntitled: boolean, view: string) {
constructor(private _apiWrapper: ApiWrapper, workspaceFolders: vscode.WorkspaceFolder[], extensionContext: vscode.ExtensionContext, openAsUntitled: boolean, view: string, public providerId: string) {
this._openAsUntitled = openAsUntitled;
this._extensionContext = extensionContext;
this.books = [];
@@ -51,10 +51,11 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
this.viewId = view;
this.prompter = new CodeAdapter();
this._bookTrustManager = new BookTrustManager(this.books, _apiWrapper);
this._extensionContext.subscriptions.push(azdata.nb.registerNavigationProvider(this));
}
private async initialize(workspaceFolders: vscode.WorkspaceFolder[]): Promise<void> {
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<BookTreeIte
async openNotebook(resource: string): Promise<void> {
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<BookTreeIte
});
}
openNotebookAsUntitled(resource: string): void {
async openNotebookAsUntitled(resource: string): Promise<void> {
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<BookTreeIte
default: false
});
}
getNavigation(uri: vscode.Uri): Thenable<azdata.nb.NavigationResult> {
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;
}
}

7
extensions/notebook/src/common/constants.ts
View File

@@ -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';

10
extensions/notebook/src/extension.ts
View File

@@ -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 => {

91
extensions/notebook/src/test/book/book.test.ts
View File

@@ -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<void> {
it('bookTreeViewProvider should return all book nodes when element is undefined', async function (): Promise<void> {
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<void> {
it('bookTreeViewProvider should return all page nodes when element is a book', async function (): Promise<void> {
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<void> {
it('bookTreeViewProvider should return all sections when element is a notebook', async function (): Promise<void> {
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<void> {
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<void> {
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<void> {
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<void> {
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<void> {
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());