diff --git a/extensions/notebook/src/book/bookTocManager.ts b/extensions/notebook/src/book/bookTocManager.ts index fecca7bb696e..bcf00590def9 100644 --- a/extensions/notebook/src/book/bookTocManager.ts +++ b/extensions/notebook/src/book/bookTocManager.ts @@ -30,22 +30,17 @@ export function hasSections(node: JupyterBookSection): boolean { } export class BookTocManager implements IBookTocManager { - public tableofContents: JupyterBookSection[]; - public newSection: JupyterBookSection; - private _movedFiles: Map; - private _modifiedDirectory: Set; - private _tocFiles: Map; + public tableofContents: JupyterBookSection[] = []; + public newSection: JupyterBookSection = {}; + public movedFiles: Map = new Map(); + private _modifiedDirectory: Set = new Set(); + public tocFiles: Map = new Map(); private sourceBookContentPath: string; private targetBookContentPath: string; private _sourceBook: BookModel; constructor(targetBook?: BookModel, sourceBook?: BookModel) { this._sourceBook = sourceBook; - this.newSection = {}; - this.tableofContents = []; - this._movedFiles = new Map(); - this._modifiedDirectory = new Set(); - this._tocFiles = new Map(); this.sourceBookContentPath = sourceBook?.bookItems[0].rootContentPath; this.targetBookContentPath = targetBook?.bookItems[0].rootContentPath; } @@ -132,7 +127,7 @@ export class BookTocManager implements IBookTocManager { counter++; } await fs.move(src, path.join(newFileName.concat(' - ', counter.toString())).concat(path.parse(dest).ext), { overwrite: true }); - this._movedFiles.set(src, path.join(newFileName.concat(' - ', counter.toString())).concat(path.parse(dest).ext)); + this.movedFiles.set(src, path.join(newFileName.concat(' - ', counter.toString())).concat(path.parse(dest).ext)); vscode.window.showInformationMessage(loc.duplicateFileError(path.parse(dest).base, src, newFileName.concat(' - ', counter.toString()))); return newFileName.concat(' - ', counter.toString()); } @@ -145,13 +140,14 @@ export class BookTocManager implements IBookTocManager { * Rewrite the original table of contents of the book, in case of error as well. */ async recovery(): Promise { - this._movedFiles.forEach(async (value, key) => { + for (const [key, value] of this.movedFiles.entries()) { await fs.move(value, key); - }); + } - this._tocFiles.forEach(async (value, key) => { - await fs.writeFile(key, yaml.safeDump(value, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); - }); + for (const [key, value] of this.tocFiles.entries()) { + const yamlFile = await yaml.safeLoad(value); + await fs.writeFile(key, yaml.safeDump(yamlFile, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); + } } async cleanUp(directory: string): Promise { @@ -165,7 +161,7 @@ export class BookTocManager implements IBookTocManager { let fileStat = await fs.stat(filePath); if (fileStat.isFile) { //check if the file is in the moved files - let newPath = this._movedFiles.get(filePath); + let newPath = this.movedFiles.get(filePath); if (newPath) { let exists = await fs.pathExists(newPath); // if the file exists in the new path and if the the new path and old path are different @@ -190,54 +186,51 @@ export class BookTocManager implements IBookTocManager { * @param addSection The section that'll be added to the target section. If it's undefined then the target section (findSection) is removed from the table of contents. */ async updateTOC(version: BookVersion, tocPath: string, findSection: JupyterBookSection, addSection?: JupyterBookSection): Promise { - const toc = yaml.safeLoad((await fs.readFile(tocPath, 'utf8'))); - this._tocFiles.set(tocPath, toc); - let newToc = new Array(toc.length); - for (const [index, section] of toc.entries()) { - let newSection = this.buildTOC(version, section, findSection, addSection); - if (newSection) { - newToc[index] = newSection; - } + const tocFile = await fs.readFile(tocPath, 'utf8'); + this.tableofContents = yaml.safeLoad(tocFile); + if (!this.tocFiles.has(tocPath)) { + this.tocFiles.set(tocPath, tocFile); + } + const isModified = this.modifyToc(version, this.tableofContents, findSection, addSection); + if (isModified) { + await fs.writeFile(tocPath, yaml.safeDump(this.tableofContents, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); + } else { + throw (new Error(loc.sectionNotFound(findSection.title, tocPath))); } - await fs.writeFile(tocPath, yaml.safeDump(newToc, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); - this.tableofContents = newToc; } /** - * Creates a new table of contents structure containing the added section. This method is only called when we move a section to another section. - * Since the sections can be arranged in a tree structure we need to look for the section that will be modified in a recursively. + * Modify the table of contents read from file to add or remove a section. + * + * Returns true if the table of contents is modified successfully. If it does not find the section that will be modified then that means that the toc and book tree view are not showing the same information, + * in that case return false. + * * @param version Version of the book - * @param section The current section that we are iterating + * @param toc The table of contents that will be modified * @param findSection The section that will be modified. * @param addSection The section that'll be added to the target section. If it's undefined then the target section (findSection) is removed from the table of contents. */ - private buildTOC(version: BookVersion, section: JupyterBookSection, findSection: JupyterBookSection, addSection: JupyterBookSection): JupyterBookSection { - // condition to find the section to be modified - if (section.title === findSection.title && (section.file && section.file === findSection.file || section.url && section.url === findSection.file)) { - if (addSection) { - //if addSection is not undefined, then we added to the table of contents. - section.sections !== undefined && section.sections.length > 0 ? section.sections.push(addSection) : section.sections = [addSection]; - return section; - } - // if addSection is undefined then we remove the whole section from the table of contents. - return addSection; - } else { - let newSection = convertTo(version, section); - if (section.sections && section.sections.length > 0) { - newSection.sections = [] as JupyterBookSection[]; - for (let s of section.sections) { - const child = this.buildTOC(version, s, findSection, addSection); - if (child) { - newSection.sections.push(convertTo(version, child)); - } + private modifyToc(version: BookVersion, toc: JupyterBookSection[], findSection: JupyterBookSection, addSection: JupyterBookSection): boolean { + for (const [index, section] of toc.entries()) { + if (section.title === findSection.title && (section.file && section.file === findSection.file || section.url && section.url === findSection.file)) { + if (addSection) { + //if addSection is not undefined, then we added to the table of contents. + section.sections !== undefined && section.sections.length > 0 ? section.sections.push(addSection) : section.sections = [addSection]; + toc[index] = section; + } else { + // if addSection is undefined then we remove the whole section from the table of contents. + toc[index] = addSection; + } + return true; + } else if (hasSections(section)) { + const found = this.modifyToc(version, section.sections, findSection, addSection); + if (found) { + // we only want to return true if it finds the section and modifies the toc + return true; } } - if (newSection.sections?.length === 0) { - // if sections is an empty array then assign it to undefined, so it's converted into a markdown file. - newSection.sections = undefined; - } - return newSection; } + return false; } /** @@ -278,7 +271,7 @@ export class BookTocManager implements IBookTocManager { let fileName = undefined; try { await fs.move(path.join(this.sourceBookContentPath, elem.file).concat('.ipynb'), path.join(this.targetBookContentPath, elem.file).concat('.ipynb'), { overwrite: false }); - this._movedFiles.set(path.join(this.sourceBookContentPath, elem.file).concat('.ipynb'), path.join(this.targetBookContentPath, elem.file).concat('.ipynb')); + this.movedFiles.set(path.join(this.sourceBookContentPath, elem.file).concat('.ipynb'), path.join(this.targetBookContentPath, elem.file).concat('.ipynb')); } catch (error) { if (error.code === 'EEXIST') { fileName = await this.renameFile(path.join(this.sourceBookContentPath, elem.file).concat('.ipynb'), path.join(this.targetBookContentPath, elem.file).concat('.ipynb')); @@ -289,7 +282,7 @@ export class BookTocManager implements IBookTocManager { } try { await fs.move(path.join(this.sourceBookContentPath, elem.file).concat('.md'), path.join(this.targetBookContentPath, elem.file).concat('.md'), { overwrite: false }); - this._movedFiles.set(path.join(this.sourceBookContentPath, elem.file).concat('.md'), path.join(this.targetBookContentPath, elem.file).concat('.md')); + this.movedFiles.set(path.join(this.sourceBookContentPath, elem.file).concat('.md'), path.join(this.targetBookContentPath, elem.file).concat('.md')); } catch (error) { if (error.code === 'EEXIST') { fileName = await this.renameFile(path.join(this.sourceBookContentPath, elem.file).concat('.md'), path.join(this.targetBookContentPath, elem.file).concat('.md')); @@ -319,7 +312,7 @@ export class BookTocManager implements IBookTocManager { let fileName = undefined; try { await fs.move(section.book.contentPath, path.join(this.targetBookContentPath, moveFile).concat(path.parse(uri).ext), { overwrite: false }); - this._movedFiles.set(section.book.contentPath, path.join(this.targetBookContentPath, moveFile).concat(path.parse(uri).ext)); + this.movedFiles.set(section.book.contentPath, path.join(this.targetBookContentPath, moveFile).concat(path.parse(uri).ext)); } catch (error) { if (error.code === 'EEXIST') { fileName = await this.renameFile(section.book.contentPath, path.join(this.targetBookContentPath, moveFile).concat(path.parse(uri).ext)); @@ -438,26 +431,10 @@ export class BookTocManager implements IBookTocManager { } } - public get movedFiles(): Map { - return this._movedFiles; - } - - public get originalToc(): Map { - return this._tocFiles; - } - public get modifiedDir(): Set { return this._modifiedDirectory; } - public set movedFiles(files: Map) { - this._movedFiles = files; - } - - public set originalToc(files: Map) { - this._tocFiles = files; - } - public set modifiedDir(files: Set) { this._modifiedDirectory = files; } diff --git a/extensions/notebook/src/common/localizedConstants.ts b/extensions/notebook/src/common/localizedConstants.ts index c569675fa134..625b82e993d1 100644 --- a/extensions/notebook/src/common/localizedConstants.ts +++ b/extensions/notebook/src/common/localizedConstants.ts @@ -49,6 +49,7 @@ export function closeBookError(resource: string, error: string): string { return export function duplicateFileError(title: string, path: string, newPath: string): string { return localize('duplicateFileError', "File {0} already exists in the destination folder {1} \n The file has been renamed to {2} to prevent data loss.", title, path, newPath); } export function editBookError(path: string, error: string): string { return localize('editBookError', "Error while editing book {0}: {1}", path, error); } export function selectBookError(error: string): string { return localize('selectBookError', "Error while selecting a book or a section to edit: {0}", error); } +export function sectionNotFound(section: string, tocPath: string): string { return localize('sectionNotFound', "Failed to find section {0} in {1}.", section, tocPath); } // Remote Book dialog constants export const url = localize('url', "URL"); @@ -97,3 +98,4 @@ export const msgSaveFolderError = localize('msgSaveFolderError', "Save location export function msgCreateBookWarningMsg(file: string): string { return localize('msgCreateBookWarningMsg', "Error while trying to access: {0}", file); } + diff --git a/extensions/notebook/src/test/book/bookTocManager.test.ts b/extensions/notebook/src/test/book/bookTocManager.test.ts index 33fed374e2d1..d296af31613a 100644 --- a/extensions/notebook/src/test/book/bookTocManager.test.ts +++ b/extensions/notebook/src/test/book/bookTocManager.test.ts @@ -20,6 +20,7 @@ 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) { @@ -95,7 +96,7 @@ describe('BookTocManagerTests', function () { }]; await bookTocManager.createBook(bookFolderPath, root2FolderPath); should(equalTOC(bookTocManager.tableofContents[1].sections, expectedSection)).be.true; - should(bookTocManager.tableofContents[1].file).be.equal(path.join(path.sep, subfolder, 'index')); + should((bookTocManager.tableofContents[1] as IJupyterBookSectionV2).file).be.equal(path.join(path.sep, subfolder, 'index')); }); it('should ignore invalid file extensions', async () => { @@ -338,7 +339,11 @@ describe('BookTocManagerTests', function () { treeItemCollapsibleState: undefined, type: BookTreeItemType.Markdown, version: run.version, - page: run.sectionA.sectionFormat + page: { + title: run.sectionA.sectionName, + file: path.join(path.sep, 'sectionA', 'readme'), + sections: run.sectionA.sectionFormat + } }; // section B is from source book @@ -353,7 +358,11 @@ describe('BookTocManagerTests', function () { treeItemCollapsibleState: undefined, type: BookTreeItemType.Markdown, version: run.version, - page: run.sectionB.sectionFormat + page: { + title: run.sectionB.sectionName, + file: path.join(path.sep, 'sectionB', 'readme'), + sections: run.sectionB.sectionFormat + } }; // notebook5 is from source book @@ -374,12 +383,8 @@ describe('BookTocManagerTests', function () { type: BookTreeItemType.Notebook, version: run.version, page: { - sections: [ - { - 'title': 'Notebook 5', - 'file': path.join(path.sep, 'notebook5') - } - ] + 'title': 'Notebook 5', + 'file': path.join(path.sep, 'notebook5') } }; @@ -400,12 +405,8 @@ describe('BookTocManagerTests', function () { type: BookTreeItemType.Notebook, version: run.version, page: { - sections: [ - { - 'title': 'Notebook 5', - 'file': path.join(path.sep, 'notebook5') - } - ] + 'title': 'Notebook 5', + 'file': path.join(path.sep, 'notebook5') } }; @@ -432,7 +433,7 @@ describe('BookTocManagerTests', function () { sectionA.tableOfContentsPath = run.sourceBook.tocPath; sectionB.tableOfContentsPath = run.sourceBook.tocPath; notebook.tableOfContentsPath = run.sourceBook.tocPath; - duplicatedNotebook.tableOfContentsPath = run.sourceBook.tocPath; + duplicatedNotebook.tableOfContentsPath = undefined; sectionA.sections = run.sectionA.sectionFormat; sectionB.sections = run.sectionB.sectionFormat; @@ -468,9 +469,9 @@ describe('BookTocManagerTests', function () { } // target book - await fs.writeFile(run.targetBook.tocPath, '- title: Welcome\n file: /readme\n- title: Section C\n file: /sectionC/readme\n sections:\n - title: Notebook6\n file: /sectionC/notebook6'); + await fs.writeFile(run.targetBook.tocPath, '- title: Welcome\n file: /readme\n- title: Section C\n file: /sectionC/readme\n sections:\n - title: Notebook 6\n file: /sectionC/notebook6'); // source book - await fs.writeFile(run.sourceBook.tocPath, '- title: Notebook 5\n file: /notebook5\n- title: Section A\n file: /sectionA/readme\n sections:\n - title: Notebook1\n file: /sectionA/notebook1\n - title: Notebook2\n file: /sectionA/notebook2'); + await fs.writeFile(run.sourceBook.tocPath, '- title: Notebook 5\n file: /notebook5\n- title: Section A\n file: /sectionA/readme\n sections:\n - title: Notebook1\n file: /sectionA/notebook1\n - title: Notebook2\n file: /sectionA/notebook2\n- title: Section B\n file: /sectionB/readme\n sections:\n - title: Notebook3\n file: /sectionB/notebook3\n - title: Notebook4\n file: /sectionB/notebook4'); const mockExtensionContext = new MockExtensionContext();