From bce484e4e4ab4315e5135f4fe1881791ed77ea46 Mon Sep 17 00:00:00 2001 From: Rob Lourens Date: Sun, 28 Apr 2024 22:31:52 -0700 Subject: [PATCH] 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 | 165 +++++++++++++----- .../test/browser/markdownRenderer.test.ts | 144 ++++++++++++++- 2 files changed, 262 insertions(+), 47 deletions(-) diff --git a/src/vs/base/browser/markdownRenderer.ts b/src/vs/base/browser/markdownRenderer.ts index fe822fe1a61..76c1afb4e39 100644 --- a/src/vs/base/browser/markdownRenderer.ts +++ b/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[] { diff --git a/src/vs/base/test/browser/markdownRenderer.test.ts b/src/vs/base/test/browser/markdownRenderer.test.ts index 289b3974df8..010c0d672af 100644 --- a/src/vs/base/test/browser/markdownRenderer.test.ts +++ b/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); });