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

Select notebook in viewlet when opened in editor (#14854)

Browse Source 
* test changes

* code clean up

* remove extra line

* remove commented code

* remove unnecessary import

* update expand logic

* implement parent,children on bookitem

* merge conflicts

* refactor code

* fix trustBook test

* typo on condition

* feedback

* indexOf to include

* fix undefined error on expand

* remove set parent

* revert getTreeItem logic

* Fix duplicated nodes

* remove debug

* Clean up logic

* Fix tests

* Fix logic

* cleanup

* Cleanup findAndExpandParentNode (#14960)

Co-authored-by: chgagnon <chgagnon@microsoft.com>
This commit is contained in:
Maddy
2021-04-02 19:33:41 -07:00
committed by GitHub
parent 684dfc9760
commit 13ad4c9497
5 changed files with 150 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -19,7 +19,7 @@ const fsPromises = fileServices.promises;
const content = 'content';
export class BookModel {
private _bookItems: BookTreeItem[];
private _bookItems: BookTreeItem[] = [];
private _allNotebooks = new Map<string, BookTreeItem>();
private _tableOfContentsPath: string;
private _contentFolderPath: string;
@@ -28,6 +28,10 @@ export class BookModel {
private _errorMessage: string;
private _activePromise: Deferred<void> | undefined = undefined;
private _queuedPromises: Deferred<void>[] = [];
/**
* The root tree item for this model
*/
private _rootNode: BookTreeItem;
constructor(
public readonly bookPath: string,
@@ -35,9 +39,7 @@ export class BookModel {
public readonly isNotebook: boolean,
private _extensionContext: vscode.ExtensionContext,
private _onDidChangeTreeData: vscode.EventEmitter<BookTreeItem | undefined>,
public readonly notebookRootPath?: string) {
this._bookItems = [];
}
public readonly notebookRootPath?: string) { }
public unwatchTOC(): void {
fs.unwatchFile(this.tableOfContentsPath);
@@ -154,6 +156,7 @@ export class BookModel {
}
);
this._bookItems.push(notebookItem);
this._rootNode = notebookItem;
if (this.openAsUntitled && !this._allNotebooks.get(pathDetails.base)) {
this._allNotebooks.set(pathDetails.base, notebookItem);
} else {
@@ -199,6 +202,7 @@ export class BookModel {
dark: this._extensionContext.asAbsolutePath('resources/dark/book_inverse.svg')
}
);
this._rootNode = book;
this._bookItems.push(book);
} catch (e) {
this._errorMessage = loc.readBookError(this.bookPath, e instanceof Error ? e.message : e);
@@ -212,8 +216,21 @@ export class BookModel {
return this._bookItems;
}
public async getSections(tableOfContents: IJupyterBookToc, sections: JupyterBookSection[], root: string, book: BookTreeItemFormat): Promise<BookTreeItem[]> {
let notebooks: BookTreeItem[] = [];
public set bookItems(bookItems: BookTreeItem[]) {
bookItems.forEach(b => {
// only add unique notebooks
if (!this._bookItems.includes(b)) {
this._bookItems.push(b);
}
});
}
public async getSections(element: BookTreeItem): Promise<BookTreeItem[]> {
let tableOfContents: IJupyterBookToc = element.tableOfContents;
let sections: JupyterBookSection[] = element.sections;
let root: string = element.root;
let book: BookTreeItemFormat = element.book;
let treeItems: BookTreeItem[] = [];
for (let i = 0; i < sections.length; i++) {
if (sections[i].url) {
let externalLink: BookTreeItem = new BookTreeItem({
@@ -225,7 +242,8 @@ export class BookModel {
type: BookTreeItemType.ExternalLink,
treeItemCollapsibleState: vscode.TreeItemCollapsibleState.Collapsed,
isUntitled: this.openAsUntitled,
version: book.version
version: book.version,
parent: element
},
{
light: this._extensionContext.asAbsolutePath('resources/light/link.svg'),
@@ -233,7 +251,7 @@ export class BookModel {
}
);
notebooks.push(externalLink);
treeItems.push(externalLink);
} else if (sections[i].file) {
const pathToNotebook: string = getContentPath(book.version, book.root, sections[i].file.concat('.ipynb'));
const pathToMarkdown: string = getContentPath(book.version, book.root, sections[i].file.concat('.md'));
@@ -250,7 +268,8 @@ export class BookModel {
type: BookTreeItemType.Notebook,
treeItemCollapsibleState: vscode.TreeItemCollapsibleState.Collapsed,
isUntitled: this.openAsUntitled,
version: book.version
version: book.version,
parent: element
},
{
light: this._extensionContext.asAbsolutePath('resources/light/notebook.svg'),
@@ -262,14 +281,14 @@ export class BookModel {
if (!this._allNotebooks.get(path.basename(pathToNotebook))) {
this._allNotebooks.set(path.basename(pathToNotebook), notebook);
}
notebooks.push(notebook);
treeItems.push(notebook);
} else {
// convert to URI to avoid causing issue with drive letters when getting navigation links
let uriToNotebook: vscode.Uri = vscode.Uri.file(pathToNotebook);
if (!this._allNotebooks.get(uriToNotebook.fsPath)) {
this._allNotebooks.set(uriToNotebook.fsPath, notebook);
}
notebooks.push(notebook);
treeItems.push(notebook);
}
} else if (await fs.pathExists(pathToMarkdown)) {
let markdown: BookTreeItem = new BookTreeItem({
@@ -281,21 +300,35 @@ export class BookModel {
type: BookTreeItemType.Markdown,
treeItemCollapsibleState: vscode.TreeItemCollapsibleState.Collapsed,
isUntitled: this.openAsUntitled,
version: book.version
version: book.version,
parent: element
},
{
light: this._extensionContext.asAbsolutePath('resources/light/markdown.svg'),
dark: this._extensionContext.asAbsolutePath('resources/dark/markdown_inverse.svg')
}
);
notebooks.push(markdown);
if (this.openAsUntitled) {
if (!this._allNotebooks.get(path.basename(pathToMarkdown))) {
this._allNotebooks.set(path.basename(pathToMarkdown), markdown);
}
} else {
// convert to URI to avoid causing issue with drive letters when getting navigation links
let uriToNotebook: vscode.Uri = vscode.Uri.file(pathToMarkdown);
if (!this._allNotebooks.get(uriToNotebook.fsPath)) {
this._allNotebooks.set(uriToNotebook.fsPath, markdown);
}
}
treeItems.push(markdown);
} else {
this._errorMessage = loc.missingFileError(sections[i].title, book.title);
vscode.window.showErrorMessage(this._errorMessage);
}
}
}
return notebooks;
element.children = treeItems;
this.bookItems = treeItems;
return treeItems;
}
/**
@@ -334,4 +367,7 @@ export class BookModel {
public get version(): BookVersion {
return this._bookVersion;
}
public get rootNode(): BookTreeItem {
return this._rootNode;
}
}

20
extensions/notebook/src/book/bookTreeItem.ts
View File

@@ -34,10 +34,12 @@ export interface BookTreeItemFormat {
treeItemCollapsibleState: number;
isUntitled: boolean;
version?: BookVersion;
parent?: BookTreeItem;
children?: BookTreeItem[];
}
export class BookTreeItem extends vscode.TreeItem {
private _sections: JupyterBookSection[];
private _sections: JupyterBookSection[] | undefined;
private _uri: string | undefined;
private _previousUri: string;
private _nextUri: string;
@@ -204,6 +206,22 @@ export class BookTreeItem extends vscode.TreeItem {
this._tableOfContentsPath = tocPath;
}
public get children(): BookTreeItem[] | undefined {
return this.book.children;
}
public set children(children: BookTreeItem[] | undefined) {
this.book.children = children;
}
public get parent(): BookTreeItem {
return this.book.parent;
}
public set parent(parent: BookTreeItem) {
this.book.parent = parent;
}
/**
* Helper method to find a child section with a specified URL
* @param url The url of the section we're searching for

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

@@ -42,20 +42,34 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
private _bookViewer: vscode.TreeView<BookTreeItem>;
public viewId: string;
public books: BookModel[];
public books: BookModel[] = [];
public currentBook: BookModel;
constructor(workspaceFolders: vscode.WorkspaceFolder[], extensionContext: vscode.ExtensionContext, openAsUntitled: boolean, view: string, public providerId: string) {
this._openAsUntitled = openAsUntitled;
this._extensionContext = extensionContext;
this.books = [];
this.bookPinManager = new BookPinManager();
this.viewId = view;
this.initialize(workspaceFolders).catch(e => console.error(e));
this.prompter = new CodeAdapter();
this._bookTrustManager = new BookTrustManager(this.books);
this.bookTocManager = new BookTocManager();
this._bookViewer = vscode.window.createTreeView(this.viewId, { showCollapseAll: true, treeDataProvider: this });
this._bookViewer.onDidChangeVisibility(async e => {
// Whenever the viewer changes visibility then try and reveal the currently active document
// in the tree view
let openDocument = azdata.nb.activeNotebookEditor;
let notebookPath = openDocument?.document.uri;
// Find the book that this notebook belongs to
const book = this.books.find(book => notebookPath?.fsPath.replace(/\\/g, '/').indexOf(book.bookPath) >= -1);
// Only reveal if...
if (e.visible && // If the view is currently visible - if not then we'll just wait until this is called when the view is made visible
book && // The notebook is part of a book in the viewlet (otherwise nothing to reveal)
(this._openAsUntitled ? notebookPath?.scheme === 'untitled' : notebookPath?.scheme !== 'untitled')) // The notebook is of the correct type for this tree view
{
await this.revealDocumentInTreeView(notebookPath);
}
});
this._extensionContext.subscriptions.push(azdata.nb.registerNavigationProvider(this));
}
@@ -216,10 +230,10 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
this.currentBook = existingBook;
} else {
await this.createAndAddBookModel(bookPath, !!isNotebook);
this.currentBook = this.books.find(book => book.bookPath === bookPath);
}
if (showPreview) {
this.currentBook = this.books.find(book => book.bookPath === bookPath);
this._bookViewer.reveal(this.currentBook.bookItems[0], { expand: vscode.TreeItemCollapsibleState.Expanded, focus: true, select: true });
await this.showPreviewFile(urlToOpen);
}
@@ -310,15 +324,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
if (!this.currentBook) {
this.currentBook = book;
}
this._bookViewer = vscode.window.createTreeView(this.viewId, { showCollapseAll: true, treeDataProvider: this });
this._bookViewer.onDidChangeVisibility(e => {
let openDocument = azdata.nb.activeNotebookEditor;
let notebookPath = openDocument?.document.uri;
// call reveal only once on the correct view
if (e.visible && ((!this._openAsUntitled && notebookPath?.scheme !== 'untitled') || (this._openAsUntitled && notebookPath?.scheme === 'untitled'))) {
this.revealActiveDocumentInViewlet();
}
});
this._onDidChangeTreeData.fire(undefined);
}
}
@@ -383,17 +389,17 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
}
}
async revealActiveDocumentInViewlet(uri?: vscode.Uri, shouldReveal: boolean = true): Promise<BookTreeItem | undefined> {
async revealDocumentInTreeView(uri?: vscode.Uri, shouldReveal: boolean = true): Promise<BookTreeItem | undefined> {
let bookItem: BookTreeItem;
let notebookPath: string;
// If no uri is passed in, try to use the current active notebook editor
if (!uri) {
let openDocument = azdata.nb.activeNotebookEditor;
if (openDocument) {
notebookPath = openDocument.document.uri.fsPath;
notebookPath = openDocument.document.uri.fsPath.replace(/\\/g, '/');
}
} else if (uri.fsPath) {
notebookPath = uri.fsPath;
notebookPath = uri.fsPath.replace(/\\/g, '/');
}
if (shouldReveal || this._bookViewer?.visible) {
@@ -407,24 +413,51 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
return bookItem;
}
async findAndExpandParentNode(notebookPath: string): Promise<BookTreeItem> {
let bookItem: BookTreeItem = this.currentBook?.getNotebook(notebookPath);
// if the node is not expanded getNotebook returns undefined, try to expand the parent node or getChildren of
// the root node.
if (!bookItem) {
// get the parent node and expand it if it's not already
let allNodes = this.currentBook?.getAllNotebooks();
let book = allNodes ? Array.from(allNodes?.keys())?.filter(x => x.indexOf(notebookPath.substring(0, notebookPath.lastIndexOf(path.sep))) > -1) : undefined;
let bookNode = book?.length > 0 ? this.currentBook?.getNotebook(book.find(x => x.substring(0, x.lastIndexOf(path.sep)) === notebookPath.substring(0, notebookPath.lastIndexOf(path.sep)))) : undefined;
if (bookNode) {
if (this._bookViewer?.visible) {
await this._bookViewer.reveal(bookNode, { select: true, focus: false, expand: 3 });
} else {
await this.getChildren(bookNode);
}
bookItem = this.currentBook?.getNotebook(notebookPath);
async findAndExpandParentNode(notebookPath: string): Promise<BookTreeItem | undefined> {
const parentBook = this.books.find(b => notebookPath.indexOf(b.bookPath) > -1);
if (!parentBook) {
// No parent book, likely because the Notebook is at the top level and not under a Notebook.
// Nothing to expand in that case so just return immediately
return undefined;
}
this.currentBook = parentBook;
let bookItem: BookTreeItem = parentBook.getNotebook(notebookPath);
if (bookItem) {
// We already have the Notebook loaded so just return it immediately
return bookItem;
}
// We couldn't find the Notebook which may mean that we don't have it loaded yet, starting from
// the top we'll expand nodes until we find the parent of the Notebook we're looking for
// get the children of root node and expand the nodes to the notebook level.
await this.getChildren(parentBook.rootNode);
// The path to the parent of the Notebook we're looking for (this is the node we're looking to expand)
const parentPath = notebookPath.substring(0, notebookPath.lastIndexOf(path.posix.sep));
// Find number of directories between the Notebook path and the root of the book it's contained in
// so we know how many parent nodes to expand
let depthOfNotebookInBook: number = path.relative(notebookPath, parentBook.bookPath).split(path.sep).length;
// Walk the tree, expanding parent nodes as needed to load the child nodes until
// we find the one for our Notebook
while (depthOfNotebookInBook > 0) {
// check if the notebook is available in already expanded levels.
bookItem = parentBook.bookItems.find(b => b.tooltip === notebookPath);
if (bookItem) {
return bookItem;
}
// Search for the parent item
// notebook can be inside the same folder as parent and can be in a different folder as well
// so check for both scenarios.
let bookItemToExpand = parentBook.bookItems.find(b => b.tooltip.indexOf(parentPath) > -1) ??
parentBook.bookItems.find(b => path.relative(notebookPath, b.tooltip)?.split(path.sep)?.length === depthOfNotebookInBook);
if (!bookItemToExpand) {
break;
}
if (!bookItemToExpand.children) {
// We haven't loaded children of this node yet so do that now so we can
// continue expanding and search its children
await this.getChildren(bookItemToExpand);
}
await this._bookViewer.reveal(bookItemToExpand, { select: false, focus: true, expand: 3 });
depthOfNotebookInBook--;
}
return bookItem;
}
@@ -548,7 +581,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
}
}
public async loadNotebooksInFolder(folderPath: string, urlToOpen?: string, showPreview?: boolean) {
public async loadNotebooksInFolder(folderPath: string, urlToOpen?: string, showPreview?: boolean): Promise<void> {
let bookCollection = await this.getNotebooksInTree(folderPath);
for (let i = 0; i < bookCollection.bookPaths.length; i++) {
await this.openBook(bookCollection.bookPaths[i], urlToOpen, showPreview);
@@ -604,23 +637,19 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
}
}
getTreeItem(element: BookTreeItem): vscode.TreeItem {
async getTreeItem(element: BookTreeItem): Promise<vscode.TreeItem> {
return element;
}
getChildren(element?: BookTreeItem): Thenable<BookTreeItem[]> {
if (element) {
if (element.sections) {
return Promise.resolve(this.currentBook.getSections(element.tableOfContents, element.sections, element.root, element.book).then(sections => { return sections; }));
return Promise.resolve(this.currentBook.getSections(element));
} else {
return Promise.resolve([]);
}
} else {
let bookItems: BookTreeItem[] = [];
this.books.map(book => {
bookItems = bookItems.concat(book.bookItems);
});
return Promise.resolve(bookItems);
return Promise.resolve(this.books.map(book => book.rootNode));
}
}
@@ -630,8 +659,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
* throwing error if it is not implemented.
*/
getParent(element?: BookTreeItem): vscode.ProviderResult<BookTreeItem> {
// Remove it for perf issues.
return undefined;
return element?.parent;
}
getUntitledNotebookUri(resource: string): vscode.Uri {

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

@@ -150,9 +150,9 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
azdata.nb.onDidChangeActiveNotebookEditor(e => {
if (e.document.uri.scheme === 'untitled') {
providedBookTreeViewProvider.revealActiveDocumentInViewlet(e.document.uri, false);
providedBookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false);
} else {
bookTreeViewProvider.revealActiveDocumentInViewlet(e.document.uri, false);
bookTreeViewProvider.revealDocumentInTreeView(e.document.uri, false);
}
});

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

@@ -184,6 +184,7 @@ describe('BooksTreeViewTests', function () {
should(children).be.Array();
should(children.length).equal(1);
book = children[0];
should(book).equal(bookTreeViewProvider.currentBook.rootNode);
should(book.title).equal(expectedBook.title);
});
@@ -210,13 +211,13 @@ describe('BooksTreeViewTests', function () {
});
it('should set notebooks trusted to true on trustBook', async () => {
let notebook1Path = path.join(rootFolderPath, 'Book', 'content', 'notebook1.ipynb');
let notebook1Path = notebook1.tooltip;
let bookTrustManager: BookTrustManager = new BookTrustManager(bookTreeViewProvider.books);
let isTrusted = bookTrustManager.isNotebookTrustedByDefault(vscode.Uri.file(notebook1Path).fsPath);
let isTrusted = bookTrustManager.isNotebookTrustedByDefault(notebook1Path);
should(isTrusted).equal(false, 'Notebook should not be trusted by default');
bookTreeViewProvider.trustBook(bookTreeViewProvider.currentBook.bookItems[0]);
isTrusted = bookTrustManager.isNotebookTrustedByDefault(vscode.Uri.file(notebook1Path).fsPath);
bookTreeViewProvider.trustBook(notebook1);
isTrusted = bookTrustManager.isNotebookTrustedByDefault(notebook1Path);
should(isTrusted).equal(true, 'Failed to set trust on trustBook');
});
@@ -252,7 +253,7 @@ describe('BooksTreeViewTests', function () {
let notebook2Path = path.join(rootFolderPath, 'Book', 'content', 'notebook2.ipynb');
let notebookUri = vscode.Uri.file(notebook2Path);
let revealActiveDocumentInViewletSpy = sinon.spy(bookTreeViewProvider, 'revealActiveDocumentInViewlet');
let revealActiveDocumentInViewletSpy = sinon.spy(bookTreeViewProvider, 'revealDocumentInTreeView');
await azdata.nb.showNotebookDocument(notebookUri);
should(azdata.nb.notebookDocuments.find(doc => doc.fileName === notebookUri.fsPath)).not.be.undefined();
should(revealActiveDocumentInViewletSpy.calledOnce).be.true('revealActiveDocumentInViewlet should have been called');
@@ -335,7 +336,7 @@ describe('BooksTreeViewTests', function () {
const untitledNotebook1Uri = vscode.Uri.parse(`untitled:notebook1.ipynb`);
const untitledNotebook2Uri = vscode.Uri.parse(`untitled:notebook2.ipynb`);
let revealActiveDocumentInViewletSpy = sinon.spy(providedbookTreeViewProvider, 'revealActiveDocumentInViewlet');
let revealActiveDocumentInViewletSpy = sinon.spy(providedbookTreeViewProvider, 'revealDocumentInTreeView');
await azdata.nb.showNotebookDocument(untitledNotebook1Uri);
should(azdata.nb.notebookDocuments.find(doc => doc.fileName === untitledNotebook1Uri.fsPath)).not.be.undefined();
should(revealActiveDocumentInViewletSpy.calledOnce).be.true('revealActiveDocumentInViewlet should have been called');
@@ -652,7 +653,7 @@ describe('BooksTreeViewTests', function () {
it('should show error if notebook or markdown file is missing', async function (): Promise<void> {
let books: BookTreeItem[] = bookTreeViewProvider.currentBook.bookItems;
let children = await bookTreeViewProvider.currentBook.getSections({ sections: [] }, books[0].sections, rootFolderPath, books[0].book);
let children = await bookTreeViewProvider.currentBook.getSections(books[0]);
should(bookTreeViewProvider.currentBook.errorMessage).equal('Missing file : Notebook1 from '.concat(bookTreeViewProvider.currentBook.bookItems[0].title));
// rest of book should be detected correctly even with a missing file
equalBookItems(children[0], expectedNotebook2);