From 3f0ca8b71417c2e9594d9b6e61869ef12cf4dc76 Mon Sep 17 00:00:00 2001 From: Barbara Valdez <34872381+barbaravaldez@users.noreply.github.com> Date: Tue, 2 Mar 2021 21:23:28 -0800 Subject: [PATCH] Create book dialog improvements (#14429) * add improvements TODO on creating book experience * fix create book to support a more complex folder structure * replace \\ to a forward slash on windows * address pr comments * fix tests * use the posix version of path.sep --- .../notebook/src/book/bookTocManager.ts | 124 +++++++++++------- extensions/notebook/src/book/bookTreeView.ts | 14 +- .../notebook/src/common/localizedConstants.ts | 1 + .../notebook/src/dialog/createBookDialog.ts | 4 +- extensions/notebook/src/extension.ts | 11 +- .../src/test/book/bookTocManager.test.ts | 8 +- 6 files changed, 89 insertions(+), 73 deletions(-) diff --git a/extensions/notebook/src/book/bookTocManager.ts b/extensions/notebook/src/book/bookTocManager.ts index 7f4b15dc6f..fecca7bb69 100644 --- a/extensions/notebook/src/book/bookTocManager.ts +++ b/extensions/notebook/src/book/bookTocManager.ts @@ -24,7 +24,6 @@ export interface quickPickResults { } const allowedFileExtensions: string[] = ['.md', '.ipynb']; -const initMarkdown: string[] = ['index.md', 'introduction.md', 'intro.md', 'readme.md']; export function hasSections(node: JupyterBookSection): boolean { return node.sections !== undefined && node.sections.length > 0; @@ -51,49 +50,72 @@ export class BookTocManager implements IBookTocManager { this.targetBookContentPath = targetBook?.bookItems[0].rootContentPath; } - async getAllFiles(toc: JupyterBookSection[], directory: string, filesInDir: string[], rootDirectory: string): Promise { - await Promise.all(filesInDir.map(async file => { - let isDirectory = (await fs.promises.stat(path.join(directory, file))).isDirectory(); - if (isDirectory) { - let files = await fs.promises.readdir(path.join(directory, file)); - let initFile: string = ''; - //Add files named as readme or index within the directory as the first file of the section. - files.some((f, index) => { - if (initMarkdown.includes(f)) { - initFile = path.parse(f).name; - files.splice(index, 1); - } - }); - let jupyterSection: JupyterBookSection = { - title: file, - file: path.join(file, initFile), - expand_sections: true, - numbered: false, - sections: [] - }; - toc.push(jupyterSection); - await this.getAllFiles(toc, path.join(directory, file), files, rootDirectory); - } else if (allowedFileExtensions.includes(path.extname(file))) { - // if the file is in the book root we don't include the directory. - const filePath = directory === rootDirectory ? path.parse(file).name : path.join(path.basename(directory), path.parse(file).name); - const addFile: JupyterBookSection = { - title: path.parse(file).name, - file: filePath - }; - //find if the directory (section) of the file exists else just add the file at the end of the table of contents - let indexToc = toc.findIndex(parent => parent.title === path.basename(directory)); - //if there is not init markdown file then add the first notebook or markdown file that is found - if (indexToc !== -1) { - if (toc[indexToc].file === '') { - toc[indexToc].file = addFile.file; - } else { - toc[indexToc].sections.push(addFile); - } - } else { - toc.push(addFile); + /** + * Files in the table of contents are ordered alpha-numerically, if there's a file named + * 'index.md' then it's treated as the first file. + * @param files The files in the directory + */ + getInitFile(files: string[]): path.ParsedPath | undefined { + let initFile = undefined; + // + const initFileIndex = files.findIndex(f => f === 'index.md'); + + // If it doesnt find a file named as 'index.md' then use the first file we find. + if (initFileIndex !== -1) { + initFile = path.parse(files[initFileIndex]); + } else { + files.some((f) => { + const parsedPath = path.parse(f); + if (allowedFileExtensions.includes(parsedPath.ext)) { + initFile = parsedPath; + return true; + } + return false; + }); + } + return initFile; + } + + /** + * Creates the table of contents of a book by reading the folder structure + * that the user provides. + * @param contents The contents of directory. + * @param directory The current directory. + * @param rootDirectory The root path of the book. + * Returns an array of Jupyter Sections. + */ + async createTocFromDir(contents: string[], directory: string, rootDirectory: string): Promise { + let toc: JupyterBookSection[] = []; + for (const content of contents) { + try { + const contentStat = (await fs.promises.stat(path.join(directory, content))); + const parsedFile = path.parse(content); + if (contentStat.isFile() && allowedFileExtensions.includes(parsedFile.ext)) { + let filePath = directory === rootDirectory ? path.posix.join(path.posix.sep, parsedFile.name) : path.posix.join(path.posix.sep, path.relative(rootDirectory, directory), parsedFile.name); + const section: JupyterBookSection = { + title: parsedFile.name, + file: filePath + }; + toc.push(section); + } else if (contentStat.isDirectory()) { + let files = await fs.promises.readdir(path.join(directory, content)); + let initFile = this.getInitFile(files); + let filePath = directory === rootDirectory ? path.posix.join(path.posix.sep, parsedFile.name, initFile.name) : path.posix.join(path.posix.sep, path.relative(rootDirectory, directory), parsedFile.name, initFile.name); + let section: JupyterBookSection = {}; + section = { + title: parsedFile.name, + file: filePath, + expand_sections: true, + numbered: false, + sections: await this.createTocFromDir(files, path.join(directory, content), rootDirectory) + }; + toc.push(section); } } - })); + catch (error) { + vscode.window.showWarningMessage(loc.msgCreateBookWarningMsg(content)); + } + } return toc; } @@ -226,17 +248,19 @@ export class BookTocManager implements IBookTocManager { * If it's undefined then a blank notebook is attached to the book. */ public async createBook(bookContentPath: string, contentFolder?: string): Promise { - let filesinDir: string[]; await fs.promises.mkdir(bookContentPath, { recursive: true }); if (contentFolder) { await fs.copy(contentFolder, bookContentPath); - filesinDir = await fs.readdir(bookContentPath); - this.tableofContents = await this.getAllFiles([], bookContentPath, filesinDir, bookContentPath); } else { await fs.writeFile(path.join(bookContentPath, 'README.md'), ''); - filesinDir = ['readme.md']; - this.tableofContents = await this.getAllFiles([], bookContentPath, filesinDir, bookContentPath); } + let contents = await fs.promises.readdir(bookContentPath); + const initFile = this.getInitFile(contents); + if (initFile) { + contents.splice(contents.indexOf(initFile.base), 1); + contents.unshift(initFile.base); + } + this.tableofContents = await this.createTocFromDir(contents, bookContentPath, bookContentPath); await fs.writeFile(path.join(bookContentPath, '_config.yml'), yaml.safeDump({ title: path.basename(bookContentPath) })); await fs.writeFile(path.join(bookContentPath, '_toc.yml'), yaml.safeDump(this.tableofContents, { lineWidth: Infinity })); await vscode.commands.executeCommand('notebook.command.openNotebookFolder', bookContentPath, undefined, true); @@ -290,7 +314,7 @@ export class BookTocManager implements IBookTocManager { * @param book The target book. */ async addSection(section: BookTreeItem, book: BookTreeItem): Promise { - const uri = path.sep.concat(path.relative(section.rootContentPath, section.book.contentPath)); + const uri = path.posix.join(path.posix.sep, path.relative(section.rootContentPath, section.book.contentPath)); let moveFile = path.join(path.parse(uri).dir, path.parse(uri).name); let fileName = undefined; try { @@ -313,7 +337,7 @@ export class BookTocManager implements IBookTocManager { } } this.newSection.title = section.title; - this.newSection.file = path.join(path.parse(uri).dir, fileName)?.replace(/\\/g, '/'); + this.newSection.file = path.posix.join(path.parse(uri).dir, fileName); if (section.sections) { const files = section.sections as JupyterBookSection[]; const movedSections = await this.traverseSections(files); @@ -356,7 +380,7 @@ export class BookTocManager implements IBookTocManager { } } fileName = fileName === undefined ? notebookPath.name : path.parse(fileName).name; - this.newSection.file = path.sep.concat(fileName).replace(/\\/g, '/'); + this.newSection.file = path.posix.join(path.posix.sep, fileName); this.newSection.title = notebook.book.title; if (book.version === BookVersion.v1) { // here we only convert if is v1 because we are already using the v2 notation for every book that we read. diff --git a/extensions/notebook/src/book/bookTreeView.ts b/extensions/notebook/src/book/bookTreeView.ts index 9f19677d17..014c0729b4 100644 --- a/extensions/notebook/src/book/bookTreeView.ts +++ b/extensions/notebook/src/book/bookTreeView.ts @@ -159,15 +159,15 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider book.bookPath === pickedBook.detail).bookItems[0]; if (updateBook) { let bookSections = updateBook.sections; - while (bookSections?.length > 0) { + while (bookSections) { bookOptions = [{ label: loc.labelAddToLevel, detail: pickedSection ? pickedSection.detail : '' }]; bookSections.forEach(section => { if (section.sections) { bookOptions.push({ label: section.title ? section.title : section.file, detail: section.file }); } }); - bookSections = []; - if (bookOptions.length > 1) { + bookSections = undefined; + if (bookOptions.length >= 1) { pickedSection = await vscode.window.showQuickPick(bookOptions, { canPickMany: false, placeHolder: loc.labelBookSection @@ -186,16 +186,16 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider { const selectionResults = await this.getSelectionQuickPick(movingElement); - const pickedSection = selectionResults.quickPickSection; - const updateBook = selectionResults.book; - if (pickedSection && updateBook) { + if (selectionResults) { + const pickedSection = selectionResults.quickPickSection; + const updateBook = selectionResults.book; const targetSection = pickedSection.detail !== undefined ? updateBook.findChildSection(pickedSection.detail) : undefined; if (movingElement.tableOfContents.sections) { if (movingElement.contextValue === 'savedNotebook') { diff --git a/extensions/notebook/src/common/localizedConstants.ts b/extensions/notebook/src/common/localizedConstants.ts index 94823b4e88..c569675fa1 100644 --- a/extensions/notebook/src/common/localizedConstants.ts +++ b/extensions/notebook/src/common/localizedConstants.ts @@ -94,5 +94,6 @@ export const saveLocation = localize('saveLocation', "Save location"); export const contentFolder = localize('contentFolder', "Content folder (Optional)"); export const msgContentFolderError = localize('msgContentFolderError', "Content folder path does not exist"); export const msgSaveFolderError = localize('msgSaveFolderError', "Save location path does not exist"); +export function msgCreateBookWarningMsg(file: string): string { return localize('msgCreateBookWarningMsg', "Error while trying to access: {0}", file); } diff --git a/extensions/notebook/src/dialog/createBookDialog.ts b/extensions/notebook/src/dialog/createBookDialog.ts index fc56b59c6e..9a1ec8237e 100644 --- a/extensions/notebook/src/dialog/createBookDialog.ts +++ b/extensions/notebook/src/dialog/createBookDialog.ts @@ -28,7 +28,7 @@ export class CreateBookDialog { } protected createHorizontalContainer(view: azdata.ModelView, items: azdata.Component[]): azdata.FlexContainer { - return view.modelBuilder.flexContainer().withItems(items, { CSSStyles: { 'margin-right': '10px', 'margin-bottom': '10px' } }).withLayout({ flexFlow: 'row' }).component(); + return view.modelBuilder.flexContainer().withItems(items, { CSSStyles: { 'margin-right': '5px', 'margin-bottom': '10px' } }).withLayout({ flexFlow: 'row', alignItems: 'center' }).component(); } public async selectFolder(): Promise { @@ -140,7 +140,7 @@ export class CreateBookDialog { }, ], title: '' - }]).withLayout({ width: '100%' }).component(); + }]).component(); await this.view.initializeModel(this.formModel); }); this.dialog.okButton.label = loc.create; diff --git a/extensions/notebook/src/extension.ts b/extensions/notebook/src/extension.ts index c787376227..73fab72dbd 100644 --- a/extensions/notebook/src/extension.ts +++ b/extensions/notebook/src/extension.ts @@ -6,7 +6,6 @@ import * as vscode from 'vscode'; import * as azdata from 'azdata'; import * as nls from 'vscode-nls'; -import * as path from 'path'; import { JupyterController } from './jupyter/jupyterController'; import { AppContext } from './common/appContext'; @@ -31,7 +30,6 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi IconPathHelper.setExtensionContext(extensionContext); const appContext = new AppContext(extensionContext); - const createBookPath: string = path.posix.join(extensionContext.extensionPath, 'resources', 'notebooks', 'JupyterBooksCreate.ipynb'); /** * ***** IMPORTANT ***** * If changes are made to bookTreeView.openBook, please ensure backwards compatibility with its current state. @@ -60,14 +58,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi })); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.createBook', async () => { - let untitledFileName: vscode.Uri = vscode.Uri.parse(`untitled:${createBookPath}`); - await vscode.workspace.openTextDocument(createBookPath).then((document) => { - azdata.nb.showNotebookDocument(untitledFileName, { - connectionProfile: null, - initialContent: document.getText(), - initialDirtyState: false - }); - }); + await bookTreeViewProvider.createBook(); })); extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.moveTo', async (book: BookTreeItem) => { diff --git a/extensions/notebook/src/test/book/bookTocManager.test.ts b/extensions/notebook/src/test/book/bookTocManager.test.ts index f8a7555b0a..33fed374e2 100644 --- a/extensions/notebook/src/test/book/bookTocManager.test.ts +++ b/extensions/notebook/src/test/book/bookTocManager.test.ts @@ -58,7 +58,7 @@ describe('BookTocManagerTests', function () { rootFolderPath = path.join(os.tmpdir(), `BookTestData_${uuid.v4()}`); bookFolderPath = path.join(os.tmpdir(), `BookTestData_${uuid.v4()}`); root2FolderPath = path.join(os.tmpdir(), `BookTestData_${uuid.v4()}`); - notebooks = ['notebook1.ipynb', 'notebook2.ipynb', 'notebook3.ipynb', 'index.md', 'readme.md']; + notebooks = ['notebook1.ipynb', 'notebook2.ipynb', 'notebook3.ipynb', 'index.md']; await fs.mkdir(rootFolderPath); await fs.writeFile(path.join(rootFolderPath, notebooks[0]), ''); @@ -71,7 +71,7 @@ describe('BookTocManagerTests', function () { await fs.writeFile(path.join(root2FolderPath, notebooks[0]), ''); await fs.writeFile(path.join(root2FolderPath, subfolder, notebooks[1]), ''); await fs.writeFile(path.join(root2FolderPath, subfolder, notebooks[2]), ''); - await fs.writeFile(path.join(root2FolderPath, subfolder, notebooks[4]), ''); + await fs.writeFile(path.join(root2FolderPath, subfolder, notebooks[3]), ''); await fs.writeFile(path.join(root2FolderPath, notebooks[3]), ''); }); @@ -94,8 +94,8 @@ describe('BookTocManagerTests', function () { file: path.join(subfolder, 'notebook3') }]; await bookTocManager.createBook(bookFolderPath, root2FolderPath); - should(equalTOC(bookTocManager.tableofContents[2].sections, expectedSection)).be.true; - should((bookTocManager.tableofContents[2] as IJupyterBookSectionV2).file).be.equal(path.join(subfolder, 'readme')); + should(equalTOC(bookTocManager.tableofContents[1].sections, expectedSection)).be.true; + should(bookTocManager.tableofContents[1].file).be.equal(path.join(path.sep, subfolder, 'index')); }); it('should ignore invalid file extensions', async () => {