ckaczor/azuredatastudio
1
0
Fork 0
mirror of https://github.com/ckaczor/azuredatastudio.git synced 2026-02-16 18:46:40 -05:00
Code Issues Packages Projects Releases Wiki Activity

[Editing Books] - Refactor buildToc method (#14532)

Browse Source 
* refactor buildToc method to only modify the section found and return a boolean

* fix tests

* Address pr comments
This commit is contained in:
Barbara Valdez
2021-03-05 11:41:31 -08:00
committed by GitHub
parent b82942a030
commit a17a4a585e
3 changed files with 71 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

123
extensions/notebook/src/book/bookTocManager.ts
View File

@@ -30,22 +30,17 @@ export function hasSections(node: JupyterBookSection): boolean {
} }
export class BookTocManager implements IBookTocManager { export class BookTocManager implements IBookTocManager {
public tableofContents: JupyterBookSection[]; public tableofContents: JupyterBookSection[] = [];
public newSection: JupyterBookSection; public newSection: JupyterBookSection = {};
private _movedFiles: Map<string, string>; public movedFiles: Map<string, string> = new Map<string, string>();
private _modifiedDirectory: Set<string>; private _modifiedDirectory: Set<string> = new Set<string>();
private _tocFiles: Map<string, JupyterBookSection[]>; public tocFiles: Map<string, string> = new Map<string, string>();
private sourceBookContentPath: string; private sourceBookContentPath: string;
private targetBookContentPath: string; private targetBookContentPath: string;
private _sourceBook: BookModel; private _sourceBook: BookModel;
constructor(targetBook?: BookModel, sourceBook?: BookModel) { constructor(targetBook?: BookModel, sourceBook?: BookModel) {
this._sourceBook = sourceBook; this._sourceBook = sourceBook;
this.newSection = {};
this.tableofContents = [];
this._movedFiles = new Map<string, string>();
this._modifiedDirectory = new Set<string>();
this._tocFiles = new Map<string, JupyterBookSection[]>();
this.sourceBookContentPath = sourceBook?.bookItems[0].rootContentPath; this.sourceBookContentPath = sourceBook?.bookItems[0].rootContentPath;
this.targetBookContentPath = targetBook?.bookItems[0].rootContentPath; this.targetBookContentPath = targetBook?.bookItems[0].rootContentPath;
} }
@@ -132,7 +127,7 @@ export class BookTocManager implements IBookTocManager {
counter++; counter++;
} }
await fs.move(src, path.join(newFileName.concat(' - ', counter.toString())).concat(path.parse(dest).ext), { overwrite: true }); 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()))); vscode.window.showInformationMessage(loc.duplicateFileError(path.parse(dest).base, src, newFileName.concat(' - ', counter.toString())));
return 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. * Rewrite the original table of contents of the book, in case of error as well.
*/ */
async recovery(): Promise<void> { async recovery(): Promise<void> {
this._movedFiles.forEach(async (value, key) => { for (const [key, value] of this.movedFiles.entries()) {
await fs.move(value, key); await fs.move(value, key);
}); }
this._tocFiles.forEach(async (value, key) => { for (const [key, value] of this.tocFiles.entries()) {
await fs.writeFile(key, yaml.safeDump(value, { lineWidth: Infinity, noRefs: true, skipInvalid: true })); const yamlFile = await yaml.safeLoad(value);
}); await fs.writeFile(key, yaml.safeDump(yamlFile, { lineWidth: Infinity, noRefs: true, skipInvalid: true }));
}
} }
async cleanUp(directory: string): Promise<void> { async cleanUp(directory: string): Promise<void> {
@@ -165,7 +161,7 @@ export class BookTocManager implements IBookTocManager {
let fileStat = await fs.stat(filePath); let fileStat = await fs.stat(filePath);
if (fileStat.isFile) { if (fileStat.isFile) {
//check if the file is in the moved files //check if the file is in the moved files
let newPath = this._movedFiles.get(filePath); let newPath = this.movedFiles.get(filePath);
if (newPath) { if (newPath) {
let exists = await fs.pathExists(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 // 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. * @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<void> { async updateTOC(version: BookVersion, tocPath: string, findSection: JupyterBookSection, addSection?: JupyterBookSection): Promise<void> {
const toc = yaml.safeLoad((await fs.readFile(tocPath, 'utf8'))); const tocFile = await fs.readFile(tocPath, 'utf8');
this._tocFiles.set(tocPath, toc); this.tableofContents = yaml.safeLoad(tocFile);
let newToc = new Array<JupyterBookSection>(toc.length); if (!this.tocFiles.has(tocPath)) {
for (const [index, section] of toc.entries()) { this.tocFiles.set(tocPath, tocFile);
let newSection = this.buildTOC(version, section, findSection, addSection); }
if (newSection) { const isModified = this.modifyToc(version, this.tableofContents, findSection, addSection);
newToc[index] = newSection; 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. * Modify the table of contents read from file to add or remove a 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. *
* 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 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 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. * @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 { private modifyToc(version: BookVersion, toc: JupyterBookSection[], findSection: JupyterBookSection, addSection: JupyterBookSection): boolean {
// condition to find the section to be modified 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 (section.title === findSection.title && (section.file && section.file === findSection.file || section.url && section.url === findSection.file)) {
if (addSection) { if (addSection) {
//if addSection is not undefined, then we added to the table of contents. //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]; section.sections !== undefined && section.sections.length > 0 ? section.sections.push(addSection) : section.sections = [addSection];
return section; toc[index] = section;
} } else {
// if addSection is undefined then we remove the whole section from the table of contents. // if addSection is undefined then we remove the whole section from the table of contents.
return addSection; toc[index] = addSection;
} else { }
let newSection = convertTo(version, section); return true;
if (section.sections && section.sections.length > 0) { } else if (hasSections(section)) {
newSection.sections = [] as JupyterBookSection[]; const found = this.modifyToc(version, section.sections, findSection, addSection);
for (let s of section.sections) { if (found) {
const child = this.buildTOC(version, s, findSection, addSection); // we only want to return true if it finds the section and modifies the toc
if (child) { return true;
newSection.sections.push(convertTo(version, child));
}
} }
} }
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; let fileName = undefined;
try { try {
await fs.move(path.join(this.sourceBookContentPath, elem.file).concat('.ipynb'), path.join(this.targetBookContentPath, elem.file).concat('.ipynb'), { overwrite: false }); 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) { } catch (error) {
if (error.code === 'EEXIST') { if (error.code === 'EEXIST') {
fileName = await this.renameFile(path.join(this.sourceBookContentPath, elem.file).concat('.ipynb'), path.join(this.targetBookContentPath, elem.file).concat('.ipynb')); 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 { try {
await fs.move(path.join(this.sourceBookContentPath, elem.file).concat('.md'), path.join(this.targetBookContentPath, elem.file).concat('.md'), { overwrite: false }); 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) { } catch (error) {
if (error.code === 'EEXIST') { if (error.code === 'EEXIST') {
fileName = await this.renameFile(path.join(this.sourceBookContentPath, elem.file).concat('.md'), path.join(this.targetBookContentPath, elem.file).concat('.md')); 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; let fileName = undefined;
try { try {
await fs.move(section.book.contentPath, path.join(this.targetBookContentPath, moveFile).concat(path.parse(uri).ext), { overwrite: false }); 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) { } catch (error) {
if (error.code === 'EEXIST') { if (error.code === 'EEXIST') {
fileName = await this.renameFile(section.book.contentPath, path.join(this.targetBookContentPath, moveFile).concat(path.parse(uri).ext)); 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<string, string> {
return this._movedFiles;
}
public get originalToc(): Map<string, JupyterBookSection[]> {
return this._tocFiles;
}
public get modifiedDir(): Set<string> { public get modifiedDir(): Set<string> {
return this._modifiedDirectory; return this._modifiedDirectory;
} }
public set movedFiles(files: Map<string, string>) {
this._movedFiles = files;
}
public set originalToc(files: Map<string, JupyterBookSection[]>) {
this._tocFiles = files;
}
public set modifiedDir(files: Set<string>) { public set modifiedDir(files: Set<string>) {
this._modifiedDirectory = files; this._modifiedDirectory = files;
} }

2
extensions/notebook/src/common/localizedConstants.ts
View File

@@ -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 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 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 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 // Remote Book dialog constants
export const url = localize('url', "URL"); 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); } export function msgCreateBookWarningMsg(file: string): string { return localize('msgCreateBookWarningMsg', "Error while trying to access: {0}", file); }

37
extensions/notebook/src/test/book/bookTocManager.test.ts
View File

@@ -20,6 +20,7 @@ import { BookTreeViewProvider } from '../../book/bookTreeView';
import { NavigationProviders } from '../../common/constants'; import { NavigationProviders } from '../../common/constants';
import * as loc from '../../common/localizedConstants'; import * as loc from '../../common/localizedConstants';
export function equalTOC(actualToc: IJupyterBookSectionV2[], expectedToc: IJupyterBookSectionV2[]): boolean { export function equalTOC(actualToc: IJupyterBookSectionV2[], expectedToc: IJupyterBookSectionV2[]): boolean {
for (let [i, section] of actualToc.entries()) { for (let [i, section] of actualToc.entries()) {
if (section.title !== expectedToc[i].title || section.file !== expectedToc[i].file) { if (section.title !== expectedToc[i].title || section.file !== expectedToc[i].file) {
@@ -95,7 +96,7 @@ describe('BookTocManagerTests', function () {
}]; }];
await bookTocManager.createBook(bookFolderPath, root2FolderPath); await bookTocManager.createBook(bookFolderPath, root2FolderPath);
should(equalTOC(bookTocManager.tableofContents[1].sections, expectedSection)).be.true; 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 () => { it('should ignore invalid file extensions', async () => {
@@ -338,7 +339,11 @@ describe('BookTocManagerTests', function () {
treeItemCollapsibleState: undefined, treeItemCollapsibleState: undefined,
type: BookTreeItemType.Markdown, type: BookTreeItemType.Markdown,
version: run.version, 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 // section B is from source book
@@ -353,7 +358,11 @@ describe('BookTocManagerTests', function () {
treeItemCollapsibleState: undefined, treeItemCollapsibleState: undefined,
type: BookTreeItemType.Markdown, type: BookTreeItemType.Markdown,
version: run.version, 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 // notebook5 is from source book
@@ -374,12 +383,8 @@ describe('BookTocManagerTests', function () {
type: BookTreeItemType.Notebook, type: BookTreeItemType.Notebook,
version: run.version, version: run.version,
page: { 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, type: BookTreeItemType.Notebook,
version: run.version, version: run.version,
page: { 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; sectionA.tableOfContentsPath = run.sourceBook.tocPath;
sectionB.tableOfContentsPath = run.sourceBook.tocPath; sectionB.tableOfContentsPath = run.sourceBook.tocPath;
notebook.tableOfContentsPath = run.sourceBook.tocPath; notebook.tableOfContentsPath = run.sourceBook.tocPath;
duplicatedNotebook.tableOfContentsPath = run.sourceBook.tocPath; duplicatedNotebook.tableOfContentsPath = undefined;
sectionA.sections = run.sectionA.sectionFormat; sectionA.sections = run.sectionA.sectionFormat;
sectionB.sections = run.sectionB.sectionFormat; sectionB.sections = run.sectionB.sectionFormat;
@@ -468,9 +469,9 @@ describe('BookTocManagerTests', function () {
} }
// target book // 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 // 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(); const mockExtensionContext = new MockExtensionContext();