diff --git a/extensions/notebook/package.json b/extensions/notebook/package.json index fd01c8981e..53908b9ac3 100644 --- a/extensions/notebook/package.json +++ b/extensions/notebook/package.json @@ -470,24 +470,24 @@ }, { "command": "notebook.command.closeBook", - "when": "view == bookTreeView && viewItem == savedBook" + "when": "view == bookTreeView && viewItem == savedBook && listMultiSelection == false" }, { "command": "notebook.command.closeNotebook", - "when": "view == bookTreeView && viewItem == savedNotebook" + "when": "view == bookTreeView && viewItem == savedNotebook && listMultiSelection == false" }, { "command": "notebook.command.removeNotebook", - "when": "view == bookTreeView && viewItem == savedBookNotebook" + "when": "view == bookTreeView && viewItem == savedBookNotebook && listMultiSelection == false" }, { "command": "notebook.command.addNotebook", - "when": "view == bookTreeView && viewItem == section || view == bookTreeView && viewItem == savedBook", + "when": "view == bookTreeView && viewItem == section && listMultiSelection == false || view == bookTreeView && viewItem == savedBook && listMultiSelection == false", "group": "newFile@1" }, { "command": "notebook.command.addMarkdown", - "when": "view == bookTreeView && viewItem == section || view == bookTreeView && viewItem == savedBook", + "when": "view == bookTreeView && viewItem == section && listMultiSelection == false || view == bookTreeView && viewItem == savedBook && listMultiSelection == false", "group": "newFile@1" }, { diff --git a/extensions/notebook/src/book/bookTocManager.ts b/extensions/notebook/src/book/bookTocManager.ts index cb80ab845b..d1b4bc73b6 100644 --- a/extensions/notebook/src/book/bookTocManager.ts +++ b/extensions/notebook/src/book/bookTocManager.ts @@ -15,7 +15,7 @@ import { TocEntryPathHandler } from './tocEntryPathHandler'; import { FileExtension } from '../common/utils'; export interface IBookTocManager { - updateBook(element: BookTreeItem, book: BookTreeItem, targetSection?: JupyterBookSection): Promise; + updateBook(sources: BookTreeItem[], target: BookTreeItem, targetSection?: JupyterBookSection): Promise; removeNotebook(element: BookTreeItem): Promise; createBook(bookContentPath: string, contentFolder: string): Promise; addNewFile(pathDetails: TocEntryPathHandler, bookItem: BookTreeItem): Promise; @@ -339,7 +339,7 @@ export class BookTocManager implements IBookTocManager { this.newSection = sectionTOC; } } - this.newSection.title = section.title; + this.newSection.title = section.book.title; this.newSection.file = path.posix.join(path.parse(uri).dir, fileName); if (section.sections) { const files = section.sections as JupyterBookSection[]; @@ -366,8 +366,11 @@ export class BookTocManager implements IBookTocManager { const filePath = path.parse(file.book.contentPath); let fileName = undefined; try { - this.movedFiles.set(file.book.contentPath, path.join(rootPath, filePath.base)); - await fs.move(file.book.contentPath, path.join(rootPath, filePath.base), { overwrite: false }); + // no op if the notebook is already in the dest location + if (file.book.contentPath !== path.join(rootPath, filePath.base)) { + this.movedFiles.set(file.book.contentPath, path.join(rootPath, filePath.base)); + await fs.move(file.book.contentPath, path.join(rootPath, filePath.base), { overwrite: false }); + } } catch (error) { if (error.code === 'EEXIST') { fileName = await this.renameFile(file.book.contentPath, path.join(rootPath, filePath.base)); @@ -394,44 +397,47 @@ export class BookTocManager implements IBookTocManager { /** * Moves the element to the target book's folder and adds it to the table of contents. - * @param element Notebook, Markdown File, or section that will be added to the book. - * @param targetBook Book that will be modified. - * @param targetSection Book section that'll be modified. + * @param sources The tree items that are been moved. + * @param target Target tree item on which the sources will be added + * @param section (Optional) book section that'll be modified. Not required when using drag and drop. */ - public async updateBook(element: BookTreeItem, targetItem: BookTreeItem, targetSection?: JupyterBookSection): Promise { - try { - if (element.contextValue === 'section') { - // modify the sourceBook toc and remove the section - const findSection: JupyterBookSection = { file: element.book.page.file, title: element.book.page.title }; - await this.moveSectionFiles(element, targetItem); - // remove section from book - await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined); - // add section to book - await this.updateTOC(targetItem.book.version, targetItem.tableOfContentsPath, targetSection, this.newSection); - } - else { - // the notebook is part of a book so we need to modify its toc as well - const findSection = { file: element.book.page.file, title: element.book.page.title }; - await this.moveFile(element, targetItem); - if (element.contextValue === 'savedBookNotebook' || element.contextValue === 'Markdown') { - // remove notebook entry from book toc - 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); - } - await this.updateTOC(targetItem.book.version, targetItem.tableOfContentsPath, targetSection, this.newSection); - } - } catch (e) { - await this.recovery(); - void vscode.window.showErrorMessage(loc.editBookError(element.book.contentPath, e instanceof Error ? e.message : e)); - } finally { + public async updateBook(sources: BookTreeItem[], target: BookTreeItem, section?: JupyterBookSection): Promise { + for (let element of sources) { try { - await this._targetBook.reinitializeContents(); + const targetSection = section ? section : (target.contextValue === 'section' ? { file: target.book.page.file, title: target.book.page.title } : undefined); + if (element.contextValue === 'section') { + // modify the sourceBook toc and remove the section + const findSection: JupyterBookSection = { file: element.book.page.file, title: element.book.page.title }; + await this.moveSectionFiles(element, target); + // remove section from book + await this.updateTOC(element.book.version, element.tableOfContentsPath, findSection, undefined); + // add section to book + await this.updateTOC(target.book.version, target.tableOfContentsPath, targetSection, this.newSection); + } + else { + // the notebook is part of a book so we need to modify its toc as well + const findSection = { file: element.book.page.file, title: element.book.page.title }; + await this.moveFile(element, target); + if (element.contextValue === 'savedBookNotebook' || element.contextValue === 'Markdown') { + // remove notebook entry from book toc + 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); + } + await this.updateTOC(target.book.version, target.tableOfContentsPath, targetSection, this.newSection); + } + } catch (e) { + await this.recovery(); + void vscode.window.showErrorMessage(loc.editBookError(element.book.contentPath, e instanceof Error ? e.message : e)); } finally { - if (this._sourceBook && this._sourceBook.bookPath !== this._targetBook.bookPath) { - // refresh source book model to pick up latest changes - await this._sourceBook.reinitializeContents(); + try { + await this._targetBook.reinitializeContents(); + } finally { + if (this._sourceBook && this._sourceBook.bookPath !== this._targetBook.bookPath) { + // refresh source book model to pick up latest changes + await this._sourceBook.reinitializeContents(); + } } } } diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index 9e05013649..0e40be1654 100644 --- a/extensions/notebook/src/book/bookTreeView.ts +++ b/extensions/notebook/src/book/bookTreeView.ts @@ -159,7 +159,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { + async bookSectionQuickPick(): Promise { let bookOptions: vscode.QuickPickItem[] = []; let pickedSection: vscode.QuickPickItem; this.books.forEach(book => { @@ -172,7 +172,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider book.bookPath === pickedBook.detail).bookItems[0]; if (updateBook) { let bookSections = updateBook.sections; @@ -194,11 +194,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { + /** + * Move selected tree items (sections/notebooks) using the move option tree item entry point. + * A quick pick menu will show all the possible books and sections for the user to choose + * the target element to add the selected tree items. + * @param treeItems Elements to be moved + */ + async moveTreeItems(treeItems: BookTreeItem[]): Promise { TelemetryReporter.sendActionEvent(BookTelemetryView, NbTelemetryActions.MoveNotebook); - const selectionResults = await this.getSelectionQuickPick(movingElement); + const selectionResults = await this.bookSectionQuickPick(); if (selectionResults) { - const pickedSection = selectionResults.quickPickSection; - const updateBook = selectionResults.book; - const targetSection = pickedSection.detail !== undefined ? updateBook.findChildSection(pickedSection.detail) : undefined; - const sourceBook = this.books.find(book => book.bookPath === movingElement.book.root); - const targetBook = this.books.find(book => book.bookPath === updateBook.book.root); - this.bookTocManager = new BookTocManager(sourceBook, targetBook); - await this.bookTocManager.updateBook(movingElement, updateBook, targetSection); + let pickedSection = selectionResults.quickPickSection; + // filter target from sources + let movingElements = treeItems.filter(item => item.uri !== pickedSection.detail); + const targetBookItem = selectionResults.book; + const targetSection = pickedSection.detail !== undefined ? targetBookItem.findChildSection(pickedSection.detail) : undefined; + let sourcesByBook = this.groupTreeItemsByBookModel(movingElements); + const targetBookModel = this.books.find(book => book.bookPath === targetBookItem.book.root); + for (let [bookModel, items] of sourcesByBook) { + this.bookTocManager = new BookTocManager(bookModel, targetBookModel); + await this.bookTocManager.updateBook(items, targetBookItem, targetSection); + } } } @@ -717,11 +723,56 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { - let treeItems = JSON.parse(await sources.items.get('text/treeitems')!.asString()); - if (treeItems) { - + groupTreeItemsByBookModel(treeItems: BookTreeItem[]): Map { + const sourcesByBook = new Map(); + for (let item of treeItems) { + const book = this.books.find(book => book.bookPath === item.book.root); + if (sourcesByBook.has(book)) { + sourcesByBook.get(book).push(item); + } else { + sourcesByBook.set(book, [item]); + } } + return sourcesByBook; + } + + async onDrop(sources: vscode.TreeDataTransfer, target: BookTreeItem): Promise { + TelemetryReporter.sendActionEvent(BookTelemetryView, NbTelemetryActions.DragAndDrop); + // gets the tree items that are dragged and dropped + let treeItems = JSON.parse(await sources.items.get(this.supportedTypes[0])!.asString()) as BookTreeItem[]; + let rootItems = this.getLocalRoots(treeItems); + rootItems = rootItems.filter(item => item.resourceUri !== target.resourceUri); + if (rootItems && target) { + let sourcesByBook = this.groupTreeItemsByBookModel(rootItems); + const targetBook = this.books.find(book => book.bookPath === target.book.root); + for (let [book, items] of sourcesByBook) { + this.bookTocManager = new BookTocManager(book, targetBook); + await this.bookTocManager.updateBook(items, target); + } + } + } + + /** + * From the tree items moved find the local roots. + * We don't need to move a child element if the parent element has been selected as well, since every time that an element is moved + * we add its children. + * @param bookItems that have been dragged and dropped + * @returns an array of tree items that do not share a parent element. + */ + public getLocalRoots(bookItems: BookTreeItem[]): BookTreeItem[] { + const localRoots = []; + for (let i = 0; i < bookItems.length; i++) { + const parent = bookItems[i].book.parent; + if (parent) { + const isInList = bookItems.find(item => item.resourceUri.path === parent.resourceUri.path && parent.contextValue !== 'savedBook'); + if (isInList === undefined) { + localRoots.push(bookItems[i]); + } + } else { + localRoots.push(bookItems[i]); + } + } + return localRoots; } dispose(): void { } diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index 48016eb0f0..acc24e58f0 100644 --- a/extensions/notebook/src/extension.ts +++ b/extensions/notebook/src/extension.ts @@ -62,8 +62,9 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi await pinnedBookTreeViewProvider.removeNotebookFromPinnedView(book); })); - extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.moveTo', async (book: BookTreeItem) => { - await bookTreeViewProvider.editBook(book); + extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.moveTo', async (firstTreeItem: BookTreeItem, treeItems?: BookTreeItem[]) => { + let allTreeItems = treeItems ? [firstTreeItem, ...treeItems] : [firstTreeItem]; + await bookTreeViewProvider.moveTreeItems(allTreeItems); })); let model = new RemoteBookDialogModel(); diff --git a/extensions/notebook/src/telemetry.ts b/extensions/notebook/src/telemetry.ts index 49a7d59d3d..f72d4b0c19 100644 --- a/extensions/notebook/src/telemetry.ts +++ b/extensions/notebook/src/telemetry.ts @@ -21,5 +21,6 @@ export enum NbTelemetryActions { PinNotebook = 'NotebookPinned', OpenNotebookFromBook = 'NotebookOpenedFromBook', MoveNotebook = 'MoveNotebook', + DragAndDrop = 'DragAndDrop' } diff --git a/extensions/notebook/src/test/book/bookTocManager.test.ts b/extensions/notebook/src/test/book/bookTocManager.test.ts index 21e69ea2b4..a1108cfe27 100644 --- a/extensions/notebook/src/test/book/bookTocManager.test.ts +++ b/extensions/notebook/src/test/book/bookTocManager.test.ts @@ -4,7 +4,7 @@ *--------------------------------------------------------------------------------------------*/ import * as should from 'should'; import * as path from 'path'; -import { BookTocManager, hasSections, quickPickResults } from '../../book/bookTocManager'; +import { BookTocManager, hasSections } from '../../book/bookTocManager'; import { BookTreeItem, BookTreeItemFormat, BookTreeItemType } from '../../book/bookTreeItem'; import * as sinon from 'sinon'; import { IJupyterBookSectionV1, IJupyterBookSectionV2, JupyterBookSection } from '../../contracts/content'; @@ -18,7 +18,6 @@ import { BookModel } from '../../book/bookModel'; 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'; @@ -461,7 +460,7 @@ describe('BookTocManagerTests', function () { it('Add section to book', async () => { bookTocManager = new BookTocManager(sourceBookModel, targetBookModel); - await bookTocManager.updateBook(sectionA, targetBook, undefined); + await bookTocManager.updateBook([sectionA], targetBook, undefined); const listFiles = await fs.promises.readdir(path.join(run.targetBook.bookContentFolderPath, 'sectionA')); const listSourceFiles = await fs.promises.readdir(path.join(run.sourceBook.bookContentFolderPath)); should(JSON.stringify(listSourceFiles).includes('sectionA')).be.false('The source book files should not contain the section A files'); @@ -470,7 +469,7 @@ describe('BookTocManagerTests', function () { it('Add section to section', async () => { bookTocManager = new BookTocManager(sourceBookModel, targetBookModel); - await bookTocManager.updateBook(sectionB, sectionC, { + await bookTocManager.updateBook([sectionB], sectionC, { 'title': 'Notebook 6', 'file': path.posix.join(path.posix.sep, 'sectionC', 'notebook6') }); @@ -482,7 +481,7 @@ describe('BookTocManagerTests', function () { it('Add notebook to book', async () => { bookTocManager = new BookTocManager(undefined, targetBookModel); - await bookTocManager.updateBook(notebook, targetBook); + await bookTocManager.updateBook([notebook], targetBook); const listFiles = await fs.promises.readdir(run.targetBook.bookContentFolderPath); should(JSON.stringify(listFiles).includes('notebook5.ipynb')).be.true('Notebook 5 should be under the target book content folder'); }); @@ -514,8 +513,8 @@ describe('BookTocManagerTests', function () { it('Add duplicated notebook to book', async () => { bookTocManager = new BookTocManager(undefined, targetBookModel); - await bookTocManager.updateBook(notebook, targetBook); - await bookTocManager.updateBook(duplicatedNotebook, targetBook); + await bookTocManager.updateBook([notebook], targetBook); + await bookTocManager.updateBook([duplicatedNotebook], targetBook); const listFiles = await fs.promises.readdir(run.targetBook.bookContentFolderPath); should(JSON.stringify(listFiles).includes('notebook5 - 2.ipynb')).be.true('Should rename the notebook to notebook5 - 2.ipynb'); should(JSON.stringify(listFiles).includes('notebook5.ipynb')).be.true('Should keep notebook5.ipynb'); @@ -526,17 +525,10 @@ describe('BookTocManagerTests', function () { const recoverySpy = sinon.spy(BookTocManager.prototype, 'recovery'); sinon.stub(BookTocManager.prototype, 'updateTOC').throws(new Error('Unexpected error.')); const bookTreeViewProvider = new BookTreeViewProvider([], mockExtensionContext, false, 'bookTreeView', NavigationProviders.NotebooksNavigator); - const results: quickPickResults = { - book: targetBook, - quickPickSection: { - label: loc.labelAddToLevel, - description: undefined - } - }; bookTocManager = new BookTocManager(targetBookModel); - sinon.stub(bookTreeViewProvider, 'getSelectionQuickPick').returns(Promise.resolve(results)); + sinon.stub(bookTreeViewProvider, 'moveTreeItems').returns(Promise.resolve(bookTocManager.updateBook([notebook], targetBook))); try { - await bookTreeViewProvider.editBook(notebook); + await bookTreeViewProvider.moveTreeItems([notebook]); } catch (error) { should(recoverySpy.calledOnce).be.true('If unexpected error then recovery method is called.'); }