From 3d5ff25d139f2000827206720472e474fc47d02b Mon Sep 17 00:00:00 2001 From: Barbara Valdez <34872381+barbaravaldez@users.noreply.github.com> Date: Wed, 24 Feb 2021 14:24:50 -0800 Subject: [PATCH] Fix debounce issue when book toc is updated (#14392) * pass function to debounce * remove debounce decorator and move watch methods to bookmodel * Move the vscode tree change data event to book model * address pr comments * fix book tests --- extensions/notebook/src/book/bookModel.ts | 40 +++++++++++++----- extensions/notebook/src/book/bookTreeView.ts | 42 ++++--------------- .../notebook/src/test/book/book.test.ts | 4 +- .../src/test/book/bookTocManager.test.ts | 5 +-- 4 files changed, 41 insertions(+), 50 deletions(-) diff --git a/extensions/notebook/src/book/bookModel.ts b/extensions/notebook/src/book/bookModel.ts index bb7bfc0a85..88966b9f87 100644 --- a/extensions/notebook/src/book/bookModel.ts +++ b/extensions/notebook/src/book/bookModel.ts @@ -13,7 +13,7 @@ import * as fs from 'fs-extra'; import * as loc from '../common/localizedConstants'; import { IJupyterBookToc, JupyterBookSection } from '../contracts/content'; import { convertFrom, getContentPath, BookVersion } from './bookVersionHandler'; - +import { debounce } from '../common/utils'; const fsPromises = fileServices.promises; const content = 'content'; @@ -32,43 +32,62 @@ export class BookModel { public readonly openAsUntitled: boolean, public readonly isNotebook: boolean, private _extensionContext: vscode.ExtensionContext, + private _onDidChangeTreeData: vscode.EventEmitter, public readonly notebookRootPath?: string) { this._bookItems = []; } + public unwatchTOC(): void { + fs.unwatchFile(this.tableOfContentsPath); + } + + public watchTOC(): void { + fs.watchFile(this.tableOfContentsPath, async (curr, prev) => { + if (curr.mtime > prev.mtime) { + this.reinitializeContents(); + } + }); + } + + @debounce(1500) + public async reinitializeContents(): Promise { + await this.initializeContents(); + this._onDidChangeTreeData.fire(undefined); + } + public async initializeContents(): Promise { this._bookItems = []; this._allNotebooks = new Map(); if (this.isNotebook) { this.readNotebook(); } else { - await this.readBookStructure(this.bookPath); + await this.readBookStructure(); await this.loadTableOfContentFiles(); await this.readBooks(); } } - public async readBookStructure(folderPath: string): Promise { + public async readBookStructure(): Promise { // check book structure to determine version let isOlderVersion: boolean; - this._configPath = path.posix.join(folderPath, '_config.yml'); + this._configPath = path.posix.join(this.bookPath, '_config.yml'); try { - isOlderVersion = (await fs.stat(path.posix.join(folderPath, '_data'))).isDirectory() && (await fs.stat(path.posix.join(folderPath, content))).isDirectory(); + isOlderVersion = (await fs.stat(path.posix.join(this.bookPath, '_data'))).isDirectory() && (await fs.stat(path.posix.join(this.bookPath, content))).isDirectory(); } catch { isOlderVersion = false; } if (isOlderVersion) { - let isTocFile = (await fs.stat(path.posix.join(folderPath, '_data', 'toc.yml'))).isFile(); + let isTocFile = (await fs.stat(path.posix.join(this.bookPath, '_data', 'toc.yml'))).isFile(); if (isTocFile) { - this._tableOfContentsPath = path.posix.join(folderPath, '_data', 'toc.yml'); + this._tableOfContentsPath = path.posix.join(this.bookPath, '_data', 'toc.yml'); } this._bookVersion = BookVersion.v1; - this._contentFolderPath = path.posix.join(folderPath, content, ''); + this._contentFolderPath = path.posix.join(this.bookPath, content, ''); this._rootPath = path.dirname(path.dirname(this._tableOfContentsPath)); } else { - this._contentFolderPath = folderPath; - this._tableOfContentsPath = path.posix.join(folderPath, '_toc.yml'); + this._contentFolderPath = this.bookPath; + this._tableOfContentsPath = path.posix.join(this.bookPath, '_toc.yml'); this._rootPath = path.dirname(this._tableOfContentsPath); this._bookVersion = BookVersion.v2; } @@ -89,6 +108,7 @@ export class BookModel { if (await fs.pathExists(this._tableOfContentsPath)) { vscode.commands.executeCommand('setContext', 'bookOpened', true); + this.watchTOC(); } else { this._errorMessage = loc.missingTocError; throw new Error(loc.missingTocError); diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index 6b97e12c15..9f19677d17 100644 --- a/extensions/notebook/src/book/bookTreeView.ts +++ b/extensions/notebook/src/book/bookTreeView.ts @@ -16,7 +16,7 @@ import { Deferred } from '../common/promise'; import { IBookTrustManager, BookTrustManager } from './bookTrustManager'; import * as loc from '../common/localizedConstants'; import * as glob from 'fast-glob'; -import { debounce, getPinnedNotebooks, confirmReplace } from '../common/utils'; +import { getPinnedNotebooks, confirmReplace } from '../common/utils'; import { IBookPinManager, BookPinManager } from './bookPinManager'; import { BookTocManager, IBookTocManager, quickPickResults } from './bookTocManager'; import { CreateBookDialog } from '../dialog/createBookDialog'; @@ -92,14 +92,6 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - if (curr.mtime > prev.mtime) { - await this.initializeBookContents(book); - } - }); - } - trustBook(bookTreeItem?: BookTreeItem): void { let bookPathToTrust: string = bookTreeItem ? bookTreeItem.root : this.currentBook?.bookPath; if (bookPathToTrust) { @@ -216,7 +208,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - if (curr.mtime > prev.mtime) { - let book = this.books.find(book => book.bookPath === bookPath); - if (book) { - await this.initializeBookContents(book); - } - } - }); - } } catch (e) { // if there is an error remove book from context const index = this.books.findIndex(book => book.bookPath === bookPath); @@ -297,13 +276,6 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - await book.initializeContents().then(() => { - this._onDidChangeTreeData.fire(undefined); - }); - } - async closeBook(book: BookTreeItem): Promise { // remove book from the saved books let deletedBook: BookModel; @@ -324,7 +296,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { if (!this.books.find(x => x.bookPath === bookPath)) { - const book: BookModel = new BookModel(bookPath, this._openAsUntitled, isNotebook, this._extensionContext, notebookBookRoot); + const book: BookModel = new BookModel(bookPath, this._openAsUntitled, isNotebook, this._extensionContext, this._onDidChangeTreeData, notebookBookRoot); await book.initializeContents(); this.books.push(book); if (!this.currentBook) { diff --git a/extensions/notebook/src/test/book/book.test.ts b/extensions/notebook/src/test/book/book.test.ts index 2dec3b011a..4a5271048d 100644 --- a/extensions/notebook/src/test/book/book.test.ts +++ b/extensions/notebook/src/test/book/book.test.ts @@ -485,14 +485,14 @@ describe('BooksTreeViewTests', function () { if (run.it === 'v1') { it('should ignore toc.yml files not in _data folder', async () => { - await bookTreeViewProvider.currentBook.readBookStructure(rootFolderPath); + await bookTreeViewProvider.currentBook.readBookStructure(); await bookTreeViewProvider.currentBook.loadTableOfContentFiles(); let path = bookTreeViewProvider.currentBook.tableOfContentsPath; should(vscode.Uri.file(path).fsPath).equal(vscode.Uri.file(run.folderPaths.tableOfContentsFile).fsPath); }); } else if (run.it === 'v2') { it('should ignore toc.yml files not under the root book folder', async () => { - await bookTreeViewProvider.currentBook.readBookStructure(rootFolderPath); + await bookTreeViewProvider.currentBook.readBookStructure(); await bookTreeViewProvider.currentBook.loadTableOfContentFiles(); let path = bookTreeViewProvider.currentBook.tableOfContentsPath; should(vscode.Uri.file(path).fsPath).equal(vscode.Uri.file(run.folderPaths.tableOfContentsFile).fsPath); diff --git a/extensions/notebook/src/test/book/bookTocManager.test.ts b/extensions/notebook/src/test/book/bookTocManager.test.ts index 4b8b14eb12..f8a7555b0a 100644 --- a/extensions/notebook/src/test/book/bookTocManager.test.ts +++ b/extensions/notebook/src/test/book/bookTocManager.test.ts @@ -20,7 +20,6 @@ import { BookTreeViewProvider } from '../../book/bookTreeView'; import { NavigationProviders } from '../../common/constants'; import * as loc from '../../common/localizedConstants'; - export function equalTOC(actualToc: IJupyterBookSectionV2[], expectedToc: IJupyterBookSectionV2[]): boolean { for (let [i, section] of actualToc.entries()) { if (section.title !== expectedToc[i].title || section.file !== expectedToc[i].file) { @@ -475,8 +474,8 @@ describe('BookTocManagerTests', function () { const mockExtensionContext = new MockExtensionContext(); - sourceBookModel = new BookModel(run.sourceBook.rootBookFolderPath, false, false, mockExtensionContext); - targetBookModel = new BookModel(run.targetBook.rootBookFolderPath, false, false, mockExtensionContext); + sourceBookModel = new BookModel(run.sourceBook.rootBookFolderPath, false, false, mockExtensionContext, undefined); + targetBookModel = new BookModel(run.targetBook.rootBookFolderPath, false, false, mockExtensionContext, undefined); // create book model mock objects sinon.stub(sourceBookModel, 'bookItems').value([sectionA]); sinon.stub(targetBookModel, 'bookItems').value([targetBook]);