From 624c07947c6b45a1fc94795d7e3549f666502bcf Mon Sep 17 00:00:00 2001
From: Chris LaFreniere <40371649+chlafreniere@users.noreply.github.com>
Date: Wed, 12 May 2021 16:28:23 -0700
Subject: [PATCH] Notebooks: Fix Table Generation into Pure Markdown When No
 thead Exists (#15423)

* works without alignment

* Alignment working

* Add comment

* Remove outdated comment
---
 .../notebook/browser/htmlMarkdownConverter.ts | 15 ++++++
 .../notebook/browser/turndownPluginGfm.ts     | 48 ++++++++++---------
 .../browser/htmlMarkdownConverter.test.ts     | 32 +++++++++++++
 3 files changed, 72 insertions(+), 23 deletions(-)

diff --git a/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts b/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts
index 4f1a116335..84817196b2 100644
--- a/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts
+++ b/src/sql/workbench/contrib/notebook/browser/htmlMarkdownConverter.ts
@@ -266,6 +266,15 @@ export class HTMLMarkdownConverter {
 				return delimiter + leadingSpace + content + trailingSpace + delimiter;
 			}
 		});
+
+		this.turndownService.addRule('p', {
+			filter: 'p',
+			replacement: function (content, node) {
+				// If inside of a table cell, extra newlines would break table rendering
+				return isInsideTable(node) ? content : '\n\n' + content + '\n\n';
+			}
+		});
+
 		this.turndownService.escape = escapeMarkdown;
 	}
 }
@@ -281,10 +290,16 @@ function blankReplacement(content, node) {
 	// When outdenting a nested list, an empty list will still remain. Need to handle this case.
 	if (node.nodeName === 'UL' || node.nodeName === 'OL') {
 		return '\n';
+	} else if (isInsideTable(node)) {
+		return '  ';
 	}
 	return node.isBlock ? '\n\n' : '';
 }
 
+function isInsideTable(node): boolean {
+	return node.parentNode?.nodeName === 'TH' || node.parentNode?.nodeName === 'TD';
+}
+
 export function findPathRelativeToContent(notebookFolder: string, contentPath: URI | undefined): string {
 	if (notebookFolder) {
 		if (contentPath?.scheme === 'file') {
diff --git a/src/sql/workbench/contrib/notebook/browser/turndownPluginGfm.ts b/src/sql/workbench/contrib/notebook/browser/turndownPluginGfm.ts
index f4eef89652..62f705e59d 100644
--- a/src/sql/workbench/contrib/notebook/browser/turndownPluginGfm.ts
+++ b/src/sql/workbench/contrib/notebook/browser/turndownPluginGfm.ts
@@ -68,36 +68,23 @@ rules['tableCell'] = {
 rules['tableRow'] = {
 	filter: 'tr',
 	replacement: function (content, node) {
-		let borderCells = '';
-		let alignMap = { left: ':--', right: '--:', center: ':-:' };
-
-		if (isHeadingRow(node)) {
-			for (let i = 0; i < node.childNodes.length; i++) {
-				let border = '---';
-				let align = (
-					node.childNodes[i].getAttribute('align') || ''
-				).toLowerCase();
-
-				if (align) {
-					border = alignMap[align] || border;
-				}
-
-				borderCells += cell(border, node.childNodes[i]);
-			}
-		}
+		const borderCells = isHeadingRow(node) ? constructBorderCells(node) : '';
 		return '\n' + content + (borderCells ? '\n' + borderCells : '');
 	}
 };
 
 rules['table'] = {
-	// Only convert tables with a heading row.
-	// Tables with no heading row are kept using `keep` (see below).
 	filter: function (node) {
-		return node.nodeName === 'TABLE' && isHeadingRow(node.rows[0]);
+		return node.nodeName === 'TABLE';
 	},
 	replacement: function (content, node) {
 		// Ensure there are no blank lines
 		content = content.replace('\n\n', '\n');
+		// if the headings are empty, add border line and headings to keep table format
+		if (!isHeadingRow(node.rows[0])) {
+			let emptyHeader = '\n\n|' + '  |'.repeat(node.rows[0].childNodes.length) + '\n';
+			return emptyHeader + constructBorderCells(node.rows[0]) + content + '\n\n';
+		}
 		return '\n\n' + content + '\n\n';
 	}
 };
@@ -148,10 +135,25 @@ function cell(content, node) {
 	return prefix + content + ' |';
 }
 
+function constructBorderCells(node): string {
+	const alignMap = { left: ':--', right: '--:', center: ':-:' };
+	let borderCells = '';
+	for (let i = 0; i < node.childNodes.length; i++) {
+		let border = '---';
+		let align = (
+			node.childNodes[i].getAttribute('align') || ''
+		).toLowerCase();
+
+		if (align) {
+			border = alignMap[align] || border;
+		}
+
+		borderCells += cell(border, node.childNodes[i]);
+	}
+	return borderCells;
+}
+
 export function tables(turndownService) {
-	turndownService.keep(function (node) {
-		return node.nodeName === 'TABLE' && !isHeadingRow(node.rows[0]);
-	});
 	for (let key in rules) {
 		turndownService.addRule(key, rules[key]);
 	}
diff --git a/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts b/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts
index 1a10cf0341..076a790c95 100644
--- a/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts
+++ b/src/sql/workbench/contrib/notebook/test/browser/htmlMarkdownConverter.test.ts
@@ -228,6 +228,38 @@ suite('HTML Markdown Converter', function (): void {
 		assert.equal(htmlMarkdownConverter.convert(htmlString), `| Test | Test | Test |\n| --- | --- | --- |\n| test | test | test |\n| test | test | test |\n| test | test | test |\n| test | test | test |`, 'Table with no thead failed');
 	});
 
+	test('Should transform table with only tbody - typical Office scenario', () => {
+		htmlString = '<table><tbody><tr>\n<td>test1</td>\n<td>test2</td>\n<td>test3</td>\n</tr></tbody></table>\n';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `|  |  |  |\n| --- | --- | --- |\n| test1 | test2 | test3 |`, 'One row test with only tbody failed');
+		htmlString = '<table><tbody><tr>\n<td>test1</td>\n</tr></tbody></table>\n';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `|  |\n| --- |\n| test1 |`, 'One row one cell test with only tbody failed');
+		htmlString = '<table><tbody><tr>\n<td>test1</td>\n<td>test2</td>\n<td>test3</td>\n</tr>\n<tr>\n<td>test4</td>\n<td>test5</td>\n<td>test6</td>\n</tr>\n<tr>\n<td>test7</td>\n<td>test8</td>\n<td>test9</td>\n</tr>\n<tr>\n<td>test10</td>\n<td>test11</td>\n<td>test12</td>\n</tr>\n</tbody></table>\n';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `|  |  |  |\n| --- | --- | --- |\n| test1 | test2 | test3 |\n| test4 | test5 | test6 |\n| test7 | test8 | test9 |\n| test10 | test11 | test12 |`, 'Table with no thead failed');
+	});
+
+	test('Should transform table with paragraph cell correctly', () => {
+		htmlString = '<table><thead><tr><th>Test</th><th>Test2</th></tr></thead><tbody><tr><td><p>testP</p></td><td>test</td></tr></tbody></table>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| Test | Test2 |\n| --- | --- |\n| testP | test |`, 'Table with simple nested paragraph failed');
+		htmlString = '<table><thead><tr><th><p>Test</p></th><th><p>Test2</p></th></tr></thead><tbody><tr><td><p>testP</p></td><td><p>test</p></td></tr></tbody></table>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| Test | Test2 |\n| --- | --- |\n| testP | test |`, 'Table with every element with nested paragraph failed');
+	});
+
+	test('Should keep highlight and link tags in tables', () => {
+		htmlString = '<table><thead><tr><th><mark>Test</mark></th><th>Test2</th></tr></thead><tbody><tr><td><p>testP</p></td><td>test</td></tr></tbody></table>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| <mark>Test</mark> | Test2 |\n| --- | --- |\n| testP | test |`, 'Table with simple nested paragraph failed');
+		htmlString = '<table><thead><tr><th><p><a href="https://www.microsoft.com/">Test</a></p></th><th><p>Test2</p></th></tr></thead><tbody><tr><td><p>testP</p></td><td><p>test</p></td></tr></tbody></table>';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| [Test](https://www.microsoft.com/) | Test2 |\n| --- | --- |\n| testP | test |`, 'Table with link in cell failed');
+	});
+
+	test('Should transform table with column alignment', () => {
+		htmlString = '<table>\n<thead>\n<tr>\n<th align=right>Test</th>\n<th>Test</th>\n<th>Test</th>\n</tr>\n</thead>\n<tbody><tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n</tbody></table>\n';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| Test | Test | Test |\n| --: | --- | --- |\n| test | test | test |\n| test | test | test |\n| test | test | test |\n| test | test | test |`, 'Table with right align column header failed');
+		htmlString = '<table>\n<thead>\n<tr>\n<th align=left>Test</th>\n<th>Test</th>\n<th>Test</th>\n</tr>\n</thead>\n<tbody><tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n</tbody></table>\n';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| Test | Test | Test |\n| :-- | --- | --- |\n| test | test | test |\n| test | test | test |\n| test | test | test |\n| test | test | test |`, 'Table with left align column header failed');
+		htmlString = '<table>\n<thead>\n<tr>\n<th align=center>Test</th>\n<th>Test</th>\n<th>Test</th>\n</tr>\n</thead>\n<tbody><tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n<tr>\n<td>test</td>\n<td>test</td>\n<td>test</td>\n</tr>\n</tbody></table>\n';
+		assert.equal(htmlMarkdownConverter.convert(htmlString), `| Test | Test | Test |\n| :-: | --- | --- |\n| test | test | test |\n| test | test | test |\n| test | test | test |\n| test | test | test |`, 'Table with center align column header failed');
+	});
+
 	test('Should transform <b> and <strong> tags', () => {
 		htmlString = '<b>test string</b>';
 		assert.equal(htmlMarkdownConverter.convert(htmlString), '**test string**', 'Basic bold test failed');