Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 46 additions & 10 deletions src/vs/base/common/htmlParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,18 @@ const voidElements: Record<string, boolean> = {
'wbr': true
};

/**
* Quick lookup table for table elements.
*/
const tableElements: Record<string, boolean> = {
'table': true,
'thead': true,
'tbody': true,
'tfoot': true,
'tr': true,
'colgroup': true
};

Comment on lines +87 to +98
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about using a set. E.g.

const tableElements = new Set([
	'table',
	'thead',
	'tbody',
	'tfoot',
	'tr',
	'colgroup'
]);

Then you can do:

tableElements.has(node.name)

Probably actually has worse performance because of how javascript handles short objects with string keys but maybe is more clear?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NVM I see that it's copying the pattern from above. No need to change!

/**
* Checks if a node is nested inside a <pre> element.
* In <pre> elements, whitespace must be preserved per HTML spec.
Expand All @@ -107,6 +119,19 @@ function isInsidePreElement(node: HtmlNode | undefined): boolean {
return false;
}

/**
* Checks if a node is a table elemen
*
* @param node The node to check (typically parent of a text node)
* @returns true if node is a table element
*/
function isTableElement(node: HtmlNode | undefined): boolean {
if (!node || !node.name) {
return false;
}
return tableElements[node.name] === true;
}

/**
* Parses a single HTML tag.
*
Expand Down Expand Up @@ -321,14 +346,22 @@ export function parseHtml(html: string): Array<HtmlNode> {
nextChar &&
nextChar !== '<'
) {
// This is a text node; add it as a child node
if (current.children === undefined) {
current.children = [];
const textContent = html.slice(start, html.indexOf('<', start));
const isWhitespace = whitespaceRE.test(textContent);
// Check if we are in a table element context with whitespace-only text node
const whitespaceInTable = isWhitespace && isTableElement(current);

// Don't add whitespace-only text nodes if they are inside table elements
if (!whitespaceInTable) {
// This is a text node; add it as a child node
if (current.children === undefined) {
current.children = [];
}
current.children.push({
type: 'text',
content: decode(textContent),
});
}
current.children.push({
type: 'text',
content: decode(html.slice(start, html.indexOf('<', start))),
});
}

// if we're at root, push new base node
Expand Down Expand Up @@ -383,11 +416,14 @@ export function parseHtml(html: string): Array<HtmlNode> {
content = ' ';
}

// Don't add whitespace-only text nodes if they would be trailing text nodes
// or if they would be leading whitespace-only text nodes:
// Check if we are in a table element context with whitespace-only text node
const whitespaceInTable = whitespaceRE.test(content) && isTableElement(current);

// Don't add whitespace-only text nodes if they would be: trailing text nodes
// leading whitespace-only text nodes, or inside table elements:
// * end > -1 indicates this is not a trailing text node
// * leading node is when level is -1 and parent has length 0
if ((end > -1 && level + parent.length >= 0) || content !== ' ') {
if (!whitespaceInTable && ((end > -1 && level + parent.length >= 0) || content !== ' ')) {
parent.push({
type: 'text',
parent: current,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,10 @@ function markedHighlight(options: marked.MarkedOptions & {
code({ text, lang, escaped }: marked.Tokens.Code) {
const classAttr = lang ? ` class="language-${escape(lang)}"` : '';
text = text.replace(/\n$/, '');
return `<pre><code${classAttr}>${escaped ? text : escape(text)}\n</code></pre>`;
// Note: We intentionally omit the trailing \n that marked-highlight includes.
// The \n is preserved by <pre>'s default white-space:pre behavior
// this causes visible whitespace at the bottom of code blocks in Positron notebooks.
return `<pre><code${classAttr}>${escaped ? text : escape(text)}</code></pre>`;
},
},
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@
table {
border-collapse: collapse;
border-spacing: 0;
margin-bottom: 0.7em;
}

table th,
Expand All @@ -125,21 +126,35 @@
}

blockquote {
margin: 0 7px 0 5px;
margin: 0 7px 0 0;
padding: 0 16px 0 10px;
border-left-width: 5px;
border-left-style: solid;
border-left-color: var(--vscode-textBlockQuote-border);
background-color: var(--vscode-textBlockQuote-background);
}

/* inline code styling */
code {
font-size: 1em;
font-family: var(--vscode-repl-font-family);
background-color: var(--vscode-textPreformat-background);
padding: 1px 3px;
border-radius: 4px;
}

/** code block styling */
pre {
background-color: var(--vscode-textCodeBlock-background);
padding: 16px;
border-radius: 4px;
}

pre code {
line-height: 1.357em;
white-space: pre-wrap;
padding: 0;
background-color: transparent; /* override inline code style */
}

li p {
Expand Down
Loading