Remove book notebook from toc (#14704)

* Add remove notebook from book

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"
         },
         {

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",

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<void>;
+	removeNotebook(element: BookTreeItem): Promise<void>;
 	createBook(bookContentPath: string, contentFolder: string): Promise<void>;
 	recovery(): Promise<void>
 }
@@ -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<void> {
-		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(element.book.version, element.tableOfContentsPath, findSection, undefined);
-			await this.updateTOC(elementVersion, 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(element.book.version, element.tableOfContentsPath, findSection, undefined);
-				await this.updateTOC(elementVersion, 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<void> {
+		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<string> {
 		return this._modifiedDirectory;
 	}

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 { isBookItemPinned, getNotebookType } from '../common/utils';
-import { getContentPath, getTocPath } from './bookVersionHandler';
+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));

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<BookTreeIte
 		if (bookPathToUpdate) {
 			let pinStatusChanged = await this.bookPinManager.unpinNotebook(bookTreeItem);
 			if (pinStatusChanged) {
-				bookTreeItem.contextValue = 'savedNotebook';
+				bookTreeItem.contextValue = getNotebookType(bookTreeItem.book);
 			}
 		}
 	}
@@ -198,7 +198,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 			const updateBook = selectionResults.book;
 			const targetSection = pickedSection.detail !== undefined ? updateBook.findChildSection(pickedSection.detail) : undefined;
 			if (movingElement.tableOfContents.sections) {
-				if (movingElement.contextValue === 'savedNotebook') {
+				if (movingElement.contextValue === 'savedNotebook' || movingElement.contextValue === 'savedBookNotebook') {
 					let sourceBook = this.books.find(book => book.getNotebook(path.normalize(movingElement.book.contentPath)));
 					movingElement.tableOfContents.sections = sourceBook?.bookItems[0].sections;
 				}
@@ -276,6 +276,10 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 		}
 	}
 
+	async removeNotebook(bookItem: BookTreeItem): Promise<void> {
+		return this.bookTocManager.removeNotebook(bookItem);
+	}
+
 	async closeBook(book: BookTreeItem): Promise<void> {
 		// remove book from the saved books
 		let deletedBook: BookModel;

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);

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);

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);