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

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<JupyterBookSection[]> {
-		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<JupyterBookSection[]> {
+		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<void> {
-		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<void> {
-		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.

extensions/notebook/src/book/bookTreeView.ts

@@ -159,15 +159,15 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 			const updateBook = this.books.find(book => 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<BookTreeIte
 					}
 				}
 			}
-			return { quickPickSection: pickedSection, book: updateBook };
+			return pickedSection ? { quickPickSection: pickedSection, book: updateBook } : undefined;
 		}
 		return undefined;
 	}
 
 	async editBook(movingElement: BookTreeItem): Promise<void> {
 		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') {

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

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<string | undefined> {
@@ -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;

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) => {

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 () => {