From 784d76b88610b41a4ff499d7523e23385ee91712 Mon Sep 17 00:00:00 2001 From: Barbara Valdez <34872381+barbaravaldez@users.noreply.github.com> Date: Tue, 16 Mar 2021 15:02:29 -0700 Subject: [PATCH] Remove book notebook from toc (#14704) * Add remove notebook from book --- extensions/notebook/package.json | 16 ++++++++-- extensions/notebook/package.nls.json | 1 + .../notebook/src/book/bookTocManager.ts | 23 ++++++++------ extensions/notebook/src/book/bookTreeItem.ts | 31 ++++++++++++------- extensions/notebook/src/book/bookTreeView.ts | 10 ++++-- extensions/notebook/src/common/utils.ts | 10 ++++++ extensions/notebook/src/extension.ts | 1 + .../src/test/book/bookTocManager.test.ts | 31 +++++++++++++++++-- 8 files changed, 94 insertions(+), 29 deletions(-) diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index b4070a3a75..3c3738e138 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -239,6 +239,10 @@ "command": "notebook.command.closeNotebook", "title": "%title.closeJupyterNotebook%" }, + { + "command": "notebook.command.removeNotebook", + "title": "%title.removeJupyterNotebook%" + }, { "command": "notebook.command.moveTo", "title": "%title.moveTo%" @@ -389,6 +393,10 @@ "command": "notebook.command.closeNotebook", "when": "false" }, + { + "command": "notebook.command.removeNotebook", + "when": "false" + }, { "command": "notebook.command.moveTo", "when": "false" @@ -465,13 +473,17 @@ "command": "notebook.command.closeNotebook", "when": "view == bookTreeView && viewItem == savedNotebook" }, + { + "command": "notebook.command.removeNotebook", + "when": "view == bookTreeView && viewItem == savedBookNotebook" + }, { "command": "notebook.command.moveTo", - "when": "view == bookTreeView && viewItem == savedNotebook || view == bookTreeView && viewItem == section" + "when": "view == bookTreeView && viewItem == savedNotebook || view == bookTreeView && viewItem == savedBookNotebook || view == bookTreeView && viewItem == section" }, { "command": "notebook.command.pinNotebook", - "when": "view == bookTreeView && viewItem == savedNotebook", + "when": "view == bookTreeView && viewItem == savedNotebook || view == bookTreeView && viewItem == savedBookNotebook", "group": "inline" }, { diff --git a/extensions/notebook/package.nls.json b/extensions/notebook/package.nls.json index 6530576565..6b21ad9ad9 100644 --- a/extensions/notebook/package.nls.json +++ b/extensions/notebook/package.nls.json @@ -43,6 +43,7 @@ "title.openJupyterBook": "Open Book", "title.closeJupyterBook": "Close Book", "title.closeJupyterNotebook": "Close Notebook", + "title.removeJupyterNotebook": "Remove Notebook", "title.revealInBooksViewlet": "Reveal in Books", "title.createJupyterBook": "Create Book (Preview)", "title.openNotebookFolder": "Open Notebooks in Folder", diff --git a/extensions/notebook/src/book/bookTocManager.ts b/extensions/notebook/src/book/bookTocManager.ts index bcf00590de..39d8e05b64 100644 --- a/extensions/notebook/src/book/bookTocManager.ts +++ b/extensions/notebook/src/book/bookTocManager.ts @@ -14,6 +14,7 @@ import { BookModel } from './bookModel'; export interface IBookTocManager { updateBook(element: BookTreeItem, book: BookTreeItem, targetSection?: JupyterBookSection): Promise; + removeNotebook(element: BookTreeItem): Promise; createBook(bookContentPath: string, contentFolder: string): Promise; recovery(): Promise } @@ -388,16 +389,14 @@ export class BookTocManager implements IBookTocManager { * @param targetSection Book section that'll be modified. */ public async updateBook(element: BookTreeItem, targetBook: BookTreeItem, targetSection?: JupyterBookSection): Promise { - const targetBookVersion = targetBook.book.version as BookVersion; if (element.contextValue === 'section') { // modify the sourceBook toc and remove the section - const findSection: JupyterBookSection = { file: element.book.page.file?.replace(/\\/g, '/'), title: element.book.page.title }; + const findSection: JupyterBookSection = { file: element.book.page.file, title: element.book.page.title }; await this.addSection(element, targetBook); - const elementVersion = element.book.version as BookVersion; - await this.updateTOC(elementVersion, element.tableOfContentsPath, findSection, undefined); + await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined); if (targetSection) { // adding new section to the target book toc file - await this.updateTOC(targetBookVersion, targetBook.tableOfContentsPath, targetSection, this.newSection); + await this.updateTOC(targetBook.book.version, targetBook.tableOfContentsPath, targetSection, this.newSection); } else { //since there's not a target section, we just append the section at the end of the file @@ -408,13 +407,12 @@ export class BookTocManager implements IBookTocManager { await fs.writeFile(targetBook.tableOfContentsPath, yaml.safeDump(this.tableofContents, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); } } - else if (element.contextValue === 'savedNotebook') { + else if (element.contextValue === 'savedNotebook' || element.contextValue === 'savedBookNotebook') { // the notebook is part of a book so we need to modify its toc as well - const findSection = { file: element.book.page?.file?.replace(/\\/g, '/'), title: element.book.page?.title }; + const findSection = { file: element.book.page.file, title: element.book.page.title }; await this.addNotebook(element, targetBook); if (element.tableOfContentsPath) { - const elementVersion = element.book.version as BookVersion; - await this.updateTOC(elementVersion, element.tableOfContentsPath, findSection, undefined); + await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined); } else { // close the standalone notebook, so it doesn't throw an error when we move the notebook to new location. await vscode.commands.executeCommand('notebook.command.closeNotebook', element); @@ -426,11 +424,16 @@ export class BookTocManager implements IBookTocManager { this.tableofContents.push(this.newSection); await fs.writeFile(targetBook.tableOfContentsPath, yaml.safeDump(this.tableofContents, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); } else { - await this.updateTOC(targetBookVersion, targetBook.tableOfContentsPath, targetSection, this.newSection); + await this.updateTOC(targetBook.book.version, targetBook.tableOfContentsPath, targetSection, this.newSection); } } } + public async removeNotebook(element: BookTreeItem): Promise { + const findSection = { file: element.book.page.file, title: element.book.page.title }; + await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined); + } + public get modifiedDir(): Set { return this._modifiedDirectory; } diff --git a/extensions/notebook/src/book/bookTreeItem.ts b/extensions/notebook/src/book/bookTreeItem.ts index 0c3927f2e1..044bb8d8b6 100644 --- a/extensions/notebook/src/book/bookTreeItem.ts +++ b/extensions/notebook/src/book/bookTreeItem.ts @@ -7,14 +7,21 @@ import * as vscode from 'vscode'; import * as fs from 'fs'; import { JupyterBookSection, IJupyterBookToc } from '../contracts/content'; import * as loc from '../common/localizedConstants'; -import { isBookItemPinned } from '../common/utils'; -import { getContentPath, getTocPath } from './bookVersionHandler'; +import { isBookItemPinned, getNotebookType } from '../common/utils'; +import { BookVersion, getContentPath, getTocPath } from './bookVersionHandler'; export enum BookTreeItemType { Book = 'Book', Notebook = 'Notebook', Markdown = 'Markdown', - ExternalLink = 'ExternalLink' + ExternalLink = 'ExternalLink', + providedBook = 'providedBook', + savedBook = 'savedBook', + unsavedNotebook = 'unsavedNotebook', + savedNotebook = 'savedNotebook', + pinnedNotebook = 'pinnedNotebook', + section = 'section', + savedBookNotebook = 'savedBookNotebook' } export interface BookTreeItemFormat { @@ -26,7 +33,7 @@ export interface BookTreeItemFormat { type: BookTreeItemType; treeItemCollapsibleState: number; isUntitled: boolean; - version?: string; + version?: BookVersion; } export class BookTreeItem extends vscode.TreeItem { @@ -47,24 +54,24 @@ export class BookTreeItem extends vscode.TreeItem { this._sections = book.page; this.version = book.version; if (book.isUntitled) { - this.contextValue = 'providedBook'; + this.contextValue = BookTreeItemType.providedBook; } else { - this.contextValue = 'savedBook'; + this.contextValue = BookTreeItemType.savedBook; } } else { if (book.page && book.page.sections && book.page.sections.length > 0) { - this.contextValue = 'section'; + this.contextValue = BookTreeItemType.section; } else if (book.type === BookTreeItemType.Notebook && !book.tableOfContents.sections) { if (book.isUntitled) { - this.contextValue = 'unsavedNotebook'; + this.contextValue = BookTreeItemType.unsavedNotebook; } else { - this.contextValue = isBookItemPinned(book.contentPath) ? 'pinnedNotebook' : 'savedNotebook'; + this.contextValue = isBookItemPinned(book.contentPath) ? BookTreeItemType.pinnedNotebook : getNotebookType(book); } } else if (book.type === BookTreeItemType.ExternalLink) { this.contextValue = BookTreeItemType.ExternalLink; } else { - this.contextValue = book.type === BookTreeItemType.Notebook ? (isBookItemPinned(book.contentPath) ? 'pinnedNotebook' : 'savedNotebook') : 'section'; + this.contextValue = book.type === BookTreeItemType.Notebook ? (isBookItemPinned(book.contentPath) ? BookTreeItemType.pinnedNotebook : getNotebookType(book)) : BookTreeItemType.section; } this.setPageVariables(); this.setCommand(); @@ -77,7 +84,7 @@ export class BookTreeItem extends vscode.TreeItem { } else { // if it's a section, book or a notebook's book then we set the table of contents path. - if (this.book.type === BookTreeItemType.Book || this.contextValue === 'section' || (book.tableOfContents.sections && book.type === BookTreeItemType.Notebook)) { + if (this.book.type === BookTreeItemType.Book || this.contextValue === BookTreeItemType.section || (book.tableOfContents.sections && book.type === BookTreeItemType.Notebook)) { this._tableOfContentsPath = getTocPath(this.book.version, this.book.root); } this._rootContentPath = getContentPath(this.book.version, this.book.root, ''); @@ -93,7 +100,7 @@ export class BookTreeItem extends vscode.TreeItem { vscode.TreeItemCollapsibleState.Collapsed : vscode.TreeItemCollapsibleState.None; this._sections = this.book.page.sections || this.book.page.subsections; - this._uri = this.book.page.file ? this.book.page.file : this.book.page.url; + this._uri = this.book.page.file ? this.book.page.file?.replace(/\\/g, '/') : this.book.page.url?.replace(/\\/g, '/'); if (this.book.tableOfContents.sections) { let index = (this.book.tableOfContents.sections.indexOf(this.book.page)); diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index 014c0729b4..6d4de2391b 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 { getPinnedNotebooks, confirmReplace } from '../common/utils'; +import { getPinnedNotebooks, confirmReplace, getNotebookType } from '../common/utils'; import { IBookPinManager, BookPinManager } from './bookPinManager'; import { BookTocManager, IBookTocManager, quickPickResults } from './bookTocManager'; import { CreateBookDialog } from '../dialog/createBookDialog'; @@ -131,7 +131,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider book.getNotebook(path.normalize(movingElement.book.contentPath))); movingElement.tableOfContents.sections = sourceBook?.bookItems[0].sections; } @@ -276,6 +276,10 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { + return this.bookTocManager.removeNotebook(bookItem); + } + async closeBook(book: BookTreeItem): Promise { // remove book from the saved books let deletedBook: BookModel; diff --git a/extensions/notebook/src/common/utils.ts b/extensions/notebook/src/common/utils.ts index 428266ef52..80d5069fd5 100644 --- a/extensions/notebook/src/common/utils.ts +++ b/extensions/notebook/src/common/utils.ts @@ -13,6 +13,7 @@ import * as crypto from 'crypto'; import { notebookLanguages, notebookConfigKey, pinnedBooksConfigKey, AUTHTYPE, INTEGRATED_AUTH, KNOX_ENDPOINT_PORT, KNOX_ENDPOINT_SERVER } from './constants'; import { IPrompter, IQuestion, QuestionTypes } from '../prompts/question'; import * as loc from '../common/localizedConstants'; +import { BookTreeItemFormat, BookTreeItemType } from '../book/bookTreeItem'; const localize = nls.loadMessageBundle(); @@ -441,6 +442,15 @@ export function isBookItemPinned(notebookPath: string): boolean { return false; } +export function getNotebookType(book: BookTreeItemFormat): BookTreeItemType { + if (book.tableOfContents.sections) { + return BookTreeItemType.savedBookNotebook; + } + else { + return BookTreeItemType.savedNotebook; + } +} + export function getPinnedNotebooks(): IBookNotebook[] { let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(notebookConfigKey); let pinnedNotebooks: [] = config.get(pinnedBooksConfigKey); diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index 73fab72dbd..ae214e6249 100644 --- a/extensions/notebook/src/extension.ts +++ b/extensions/notebook/src/extension.ts @@ -47,6 +47,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi 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))); + extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.removeNotebook', (book: BookTreeItem) => bookTreeViewProvider.removeNotebook(book))); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openNotebookFolder', (folderPath?: string, urlToOpen?: string, showPreview?: boolean) => bookTreeViewProvider.openNotebookFolder(folderPath, urlToOpen, showPreview))); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.pinNotebook', async (book: any) => { await bookTreeViewProvider.pinNotebook(book); diff --git a/extensions/notebook/src/test/book/bookTocManager.test.ts b/extensions/notebook/src/test/book/bookTocManager.test.ts index d296af3161..433e54a08a 100644 --- a/extensions/notebook/src/test/book/bookTocManager.test.ts +++ b/extensions/notebook/src/test/book/bookTocManager.test.ts @@ -19,6 +19,8 @@ import { MockExtensionContext } from '../common/stubs'; import { BookTreeViewProvider } from '../../book/bookTreeView'; import { NavigationProviders } from '../../common/constants'; import * as loc from '../../common/localizedConstants'; +import { BookVersion } from '../../book/bookVersionHandler'; +import * as yaml from 'js-yaml'; export function equalTOC(actualToc: IJupyterBookSectionV2[], expectedToc: IJupyterBookSectionV2[]): boolean { @@ -125,7 +127,7 @@ describe('BookTocManagerTests', function () { let runs = [ { it: 'using the jupyter-book legacy version < 0.7.0', - version: 'v1', + version: BookVersion.v1, sourceBook: { 'rootBookFolderPath': sourceBookFolderPath, 'bookContentFolderPath': path.join(sourceBookFolderPath, 'content'), @@ -215,7 +217,7 @@ describe('BookTocManagerTests', function () { } }, { it: 'using the jupyter-book legacy version >= 0.7.0', - version: 'v2', + version: BookVersion.v2, sourceBook: { 'rootBookFolderPath': sourceBookFolderPath, 'bookContentFolderPath': sourceBookFolderPath, @@ -511,6 +513,31 @@ describe('BookTocManagerTests', function () { should(JSON.stringify(listFiles).includes('notebook5.ipynb')).be.true('Notebook 5 should be under the target book content folder'); }); + it('Remove notebook from book', async () => { + let toc: JupyterBookSection[] = yaml.safeLoad((await fs.promises.readFile(notebook.tableOfContentsPath)).toString()); + let notebookInToc = toc.some(section => { + if (section.title === 'Notebook 5' && section.file === path.join(path.sep, 'notebook5')) { + return true; + } + return false; + }); + should(notebookInToc).be.true('Verify the notebook is in toc before removing'); + + bookTocManager = new BookTocManager(); + await bookTocManager.removeNotebook(notebook); + + const listFiles = await fs.promises.readdir(run.sourceBook.bookContentFolderPath); + toc = yaml.safeLoad((await fs.promises.readFile(notebook.tableOfContentsPath)).toString()); + notebookInToc = toc.some(section => { + if (section.title === 'Notebook 5' && section.file === path.join(path.sep, 'notebook5')) { + return true; + } + return false; + }); + should(JSON.stringify(listFiles).includes('notebook5.ipynb')).be.true('Notebook 5 should be still under the content folder'); + should(notebookInToc).be.false('The notebook has been removed from toc'); + }); + it('Add duplicated notebook to book', async () => { bookTocManager = new BookTocManager(targetBookModel); await bookTocManager.updateBook(notebook, targetBook);