Fix some markdown fillInIncompleteTokens cases (#211593)

* Fix markdown patching for link target with incomplete argument

* Fix extra incomplete constructs inside link text

* Make link target patching more strict

* Patch up markdown inside lists
And fix links more

src/vs/base/browser/markdownRenderer.ts

@@ -588,34 +588,62 @@ function mergeRawTokenText(tokens: marked.Token[]): string {
 	return mergedTokenText;
 }
 
-function completeSingleLinePattern(token: marked.Tokens.ListItem | marked.Tokens.Paragraph): marked.Token | undefined {
-	for (let i = 0; i < token.tokens.length; i++) {
+function completeSingleLinePattern(token: marked.Tokens.Text | marked.Tokens.Paragraph): marked.Token | undefined {
+	if (!token.tokens) {
+		return undefined;
+	}
+
+	for (let i = token.tokens.length - 1; i >= 0; i--) {
 		const subtoken = token.tokens[i];
 		if (subtoken.type === 'text') {
 			const lines = subtoken.raw.split('\n');
 			const lastLine = lines[lines.length - 1];
 			if (lastLine.includes('`')) {
 				return completeCodespan(token);
-			} else if (lastLine.includes('**')) {
+			}
+
+			else if (lastLine.includes('**')) {
 				return completeDoublestar(token);
-			} else if (lastLine.match(/\*\w/)) {
+			}
+
+			else if (lastLine.match(/\*\w/)) {
 				return completeStar(token);
-			} else if (lastLine.match(/(^|\s)__\w/)) {
+			}
+
+			else if (lastLine.match(/(^|\s)__\w/)) {
 				return completeDoubleUnderscore(token);
-			} else if (lastLine.match(/(^|\s)_\w/)) {
+			}
+
+			else if (lastLine.match(/(^|\s)_\w/)) {
 				return completeUnderscore(token);
-			} else if (lastLine.match(/(^|\s)\[.*\]\(\w*/)) {
+			}
+
+			else if (
+				// Text with start of link target
+				hasLinkTextAndStartOfLinkTarget(lastLine) ||
+				// This token doesn't have the link text, eg if it contains other markdown constructs that are in other subtokens.
+				// But some preceding token does have an unbalanced [ at least
+				hasStartOfLinkTargetAndNoLinkText(lastLine) && token.tokens.slice(0, i).some(t => t.type === 'text' && t.raw.match(/\[[^\]]*$/))
+			) {
 				const nextTwoSubTokens = token.tokens.slice(i + 1);
-				if (nextTwoSubTokens[0]?.type === 'link' && nextTwoSubTokens[1]?.type === 'text' && nextTwoSubTokens[1].raw.match(/^ *"[^"]*$/)) {
-					// A markdown link can look like
-					// [link text](https://microsoft.com "more text")
-					// Where "more text" is a title for the link or an argument to a vscode command link
+
+				// A markdown link can look like
+				// [link text](https://microsoft.com "more text")
+				// Where "more text" is a title for the link or an argument to a vscode command link
+				if (
+					// If the link was parsed as a link, then look for a link token and a text token with a quote
+					nextTwoSubTokens[0]?.type === 'link' && nextTwoSubTokens[1]?.type === 'text' && nextTwoSubTokens[1].raw.match(/^ *"[^"]*$/) ||
+					// And if the link was not parsed as a link (eg command link), just look for a single quote in this token
+					lastLine.match(/^[^"]* +"[^"]*$/)
+				) {
+
 					return completeLinkTargetArg(token);
 				}
 				return completeLinkTarget(token);
-			} else if (hasStartOfLinkTarget(lastLine)) {
-				return completeLinkTarget(token);
-			} else if (lastLine.match(/(^|\s)\[\w/) && !token.tokens.slice(i + 1).some(t => hasStartOfLinkTarget(t.raw))) {
+			}
+
+			// Contains the start of link text, and no following tokens contain the link target
+			else if (lastLine.match(/(^|\s)\[\w*/)) {
 				return completeLinkText(token);
 			}
 		}
@@ -624,31 +652,90 @@ function completeSingleLinePattern(token: marked.Tokens.ListItem | marked.Tokens
 	return undefined;
 }
 
-function hasStartOfLinkTarget(str: string): boolean {
+function hasLinkTextAndStartOfLinkTarget(str: string): boolean {
+	return !!str.match(/(^|\s)\[.*\]\(\w*/);
+}
+
+function hasStartOfLinkTargetAndNoLinkText(str: string): boolean {
 	return !!str.match(/^[^\[]*\]\([^\)]*$/);
 }
 
-// function completeListItemPattern(token: marked.Tokens.List): marked.Tokens.List | undefined {
-// 	// Patch up this one list item
-// 	const lastItem = token.items[token.items.length - 1];
+function completeListItemPattern(list: marked.Tokens.List): marked.Tokens.List | undefined {
+	// Patch up this one list item
+	const lastListItem = list.items[list.items.length - 1];
+	const lastListSubToken = lastListItem.tokens ? lastListItem.tokens[lastListItem.tokens.length - 1] : undefined;
 
-// 	const newList = completeSingleLinePattern(lastItem);
-// 	if (!newList || newList.type !== 'list') {
-// 		// Nothing to fix, or not a pattern we were expecting
-// 		return;
-// 	}
+	/*
+	Example list token structures:
 
-// 	// Re-parse the whole list with the last item replaced
-// 	const completeList = marked.lexer(mergeRawTokenText(token.items.slice(0, token.items.length - 1)) + newList.items[0].raw);
-// 	if (completeList.length === 1 && completeList[0].type === 'list') {
-// 		return completeList[0];
-// 	}
+	list
+		list_item
+			text
+				text
+				codespan
+				link
+		list_item
+			text
+			code // Complete indented codeblock
+		list_item
+			text
+			space
+			text
+				text // Incomplete indented codeblock
+		list_item
+			text
+			list // Nested list
+				list_item
+					text
+						text
 
-// 	// Not a pattern we were expecting
-// 	return undefined;
-// }
+	Contrast with paragraph:
+	paragraph
+		text
+		codespan
+	*/
 
+	let newToken: marked.Token | undefined;
+	if (lastListSubToken?.type === 'text' && !('inRawBlock' in lastListItem)) { // Why does Tag have a type of 'text'
+		newToken = completeSingleLinePattern(lastListSubToken as marked.Tokens.Text);
+	}
+
+	if (!newToken || newToken.type !== 'paragraph') { // 'text' item inside the list item turns into paragraph
+		// Nothing to fix, or not a pattern we were expecting
+		return;
+	}
+
+	const previousListItemsText = mergeRawTokenText(list.items.slice(0, -1));
+
+	// Grabbing the `- ` off the list item because I can't find a better way to do this
+	const newListItemText = lastListItem.raw.slice(0, 2) +
+		mergeRawTokenText(lastListItem.tokens.slice(0, -1)) +
+		newToken.raw;
+
+	const newList = marked.lexer(previousListItemsText + newListItemText)[0] as marked.Tokens.List;
+	if (newList.type !== 'list') {
+		// Something went wrong
+		return;
+	}
+
+	return newList;
+}
+
+const maxIncompleteTokensFixRounds = 3;
 export function fillInIncompleteTokens(tokens: marked.TokensList): marked.TokensList {
+	for (let i = 0; i < maxIncompleteTokensFixRounds; i++) {
+		const newTokens = fillInIncompleteTokensOnce(tokens);
+		if (newTokens) {
+			tokens = newTokens;
+		} else {
+			break;
+		}
+	}
+
+	return tokens;
+}
+
+function fillInIncompleteTokensOnce(tokens: marked.TokensList): marked.TokensList | null {
 	let i: number;
 	let newTokens: marked.Token[] | undefined;
 	for (i = 0; i < tokens.length; i++) {
@@ -666,13 +753,13 @@ export function fillInIncompleteTokens(tokens: marked.TokensList): marked.Tokens
 			break;
 		}
 
-		// if (i === tokens.length - 1 && token.type === 'list') {
-		// 	const newListToken = completeListItemPattern(token);
-		// 	if (newListToken) {
-		// 		newTokens = [newListToken];
-		// 		break;
-		// 	}
-		// }
+		if (i === tokens.length - 1 && token.type === 'list') {
+			const newListToken = completeListItemPattern(token);
+			if (newListToken) {
+				newTokens = [newListToken];
+				break;
+			}
+		}
 
 		if (i === tokens.length - 1 && token.type === 'paragraph') {
 			// Only operates on a single token, because any newline that follows this should break these patterns
@@ -693,7 +780,7 @@ export function fillInIncompleteTokens(tokens: marked.TokensList): marked.Tokens
 		return newTokensList as marked.TokensList;
 	}
 
-	return tokens;
+	return null;
 }
 
 function completeCodeBlock(tokens: marked.Token[], leader: string): marked.Token[] {

src/vs/base/test/browser/markdownRenderer.test.ts

@@ -362,7 +362,9 @@ suite('MarkdownRenderer', () => {
 				const completeTableTokens = marked.lexer(completeTable);
 
 				const newTokens = fillInIncompleteTokens(tokens);
-				ignoreRaw(newTokens, completeTableTokens);
+				if (newTokens) {
+					ignoreRaw(newTokens, completeTableTokens);
+				}
 				assert.deepStrictEqual(newTokens, completeTableTokens);
 			});
 
@@ -373,7 +375,9 @@ suite('MarkdownRenderer', () => {
 
 				const newTokens = fillInIncompleteTokens(tokens);
 
-				ignoreRaw(newTokens, completeTableTokens);
+				if (newTokens) {
+					ignoreRaw(newTokens, completeTableTokens);
+				}
 				assert.deepStrictEqual(newTokens, completeTableTokens);
 			});
 
@@ -384,7 +388,9 @@ suite('MarkdownRenderer', () => {
 
 				const newTokens = fillInIncompleteTokens(tokens);
 
-				ignoreRaw(newTokens, completeTableTokens);
+				if (newTokens) {
+					ignoreRaw(newTokens, completeTableTokens);
+				}
 				assert.deepStrictEqual(newTokens, completeTableTokens);
 			});
 
@@ -592,7 +598,7 @@ const y = 2;
 				assert.deepStrictEqual(newTokens, completeTokens);
 			});
 
-			test.skip(`incomplete ${name} in list`, () => {
+			test(`incomplete ${name} in list`, () => {
 				const text = `- list item one\n- list item two and ${delimiter}text`;
 				const tokens = marked.lexer(text);
 				const newTokens = fillInIncompleteTokens(tokens);
@@ -602,6 +608,83 @@ const y = 2;
 			});
 		}
 
+		suite('list', () => {
+			test('list with complete codeblock', () => {
+				const list = `-
+	\`\`\`js
+	let x = 1;
+	\`\`\`
+- list item two
+`;
+				const tokens = marked.lexer(list);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				assert.deepStrictEqual(newTokens, tokens);
+			});
+
+			test.skip('list with incomplete codeblock', () => {
+				const incomplete = `- list item one
+
+	\`\`\`js
+	let x = 1;`
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + '\n	```');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('list with subitems', () => {
+				const list = `- hello
+	- sub item
+- text
+	newline for some reason
+`
+				const tokens = marked.lexer(list);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				assert.deepStrictEqual(newTokens, tokens);
+			});
+
+			test('list with stuff', () => {
+				const list = `- list item one \`codespan\` **bold** [link](http://microsoft.com) more text`;
+				const tokens = marked.lexer(list);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				assert.deepStrictEqual(newTokens, tokens);
+			});
+
+			test('list with incomplete link text', () => {
+				const incomplete = `- list item one
+- item two [link`
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + '](about:blank)');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('list with incomplete link target', () => {
+				const incomplete = `- list item one
+- item two [link](`
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + ')');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('list with incomplete link with other stuff', () => {
+				const incomplete = `- list item one
+- item two [\`link`
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + '\`](about:blank)');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+		});
+
 		suite('codespan', () => {
 			simpleMarkdownTestSuite('codespan', '`');
 
@@ -720,21 +803,66 @@ const y = 2;
 				assert.deepStrictEqual(newTokens, completeTokens);
 			});
 
-			test('incomplete link target with extra stuff and arg', () => {
+			test('incomplete link target with extra stuff and incomplete arg', () => {
 				const incomplete = '[before `text` after](http://microsoft.com "more text ';
 				const tokens = marked.lexer(incomplete);
 				const newTokens = fillInIncompleteTokens(tokens);
 
+				const completeTokens = marked.lexer(incomplete + '")');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('incomplete link target with incomplete arg', () => {
+				const incomplete = 'foo [text](http://microsoft.com "more text here ';
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + '")');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('incomplete link target with incomplete arg 2', () => {
+				const incomplete = '[text](command:_github.copilot.openRelativePath "arg';
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + '")');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('incomplete link target with complete arg', () => {
+				const incomplete = 'foo [text](http://microsoft.com "more text here"';
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
 				const completeTokens = marked.lexer(incomplete + ')');
 				assert.deepStrictEqual(newTokens, completeTokens);
 			});
 
-			test('incomplete link target with arg', () => {
-				const incomplete = 'foo [text](http://microsoft.com "more text here ';
+			test('link text with incomplete codespan', () => {
+				const incomplete = `text [\`codespan`;
 				const tokens = marked.lexer(incomplete);
 				const newTokens = fillInIncompleteTokens(tokens);
 
-				const completeTokens = marked.lexer(incomplete + '")');
+				const completeTokens = marked.lexer(incomplete + '`](about:blank)');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('link text with incomplete stuff', () => {
+				const incomplete = `text [more text \`codespan\` text **bold`;
+				const tokens = marked.lexer(incomplete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(incomplete + '**](about:blank)');
+				assert.deepStrictEqual(newTokens, completeTokens);
+			});
+
+			test('Looks like incomplete link target but isn\'t', () => {
+				const complete = '**bold** `codespan` text](';
+				const tokens = marked.lexer(complete);
+				const newTokens = fillInIncompleteTokens(tokens);
+
+				const completeTokens = marked.lexer(complete);
 				assert.deepStrictEqual(newTokens, completeTokens);
 			});