Minimal error renderer (#210767)

* collapse stack trace

* interaction buttons

* fixup

* place feature behind option

* styling, more cleanup

* test returned link, fix test bug

* test header link, skip header link for old IPython

* padding

* remove inline margin
This commit is contained in:
Aaron Munger 2024-04-22 08:09:11 -07:00 committed by GitHub
parent e69b6b469c
commit 674c62fade
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
10 changed files with 152 additions and 33 deletions

View file

@ -184,28 +184,35 @@ function renderError(
return disposableStore;
}
const headerMessage = err.name && err.message ? `${err.name}: ${err.message}` : err.name || err.message;
if (err.stack) {
const minimalError = ctx.settings.minimalError && !!headerMessage?.length;
outputElement.classList.add('traceback');
const stackTrace = formatStackTrace(err.stack);
const { formattedStack, errorLocation } = formatStackTrace(err.stack);
const outputScrolling = scrollingEnabled(outputInfo, ctx.settings);
const outputOptions = { linesLimit: ctx.settings.lineLimit, scrollable: outputScrolling, trustHtml, linkifyFilePaths: ctx.settings.linkifyFilePaths };
const outputScrolling = !minimalError && scrollingEnabled(outputInfo, ctx.settings);
const lineLimit = minimalError ? 1000 : ctx.settings.lineLimit;
const outputOptions = { linesLimit: lineLimit, scrollable: outputScrolling, trustHtml, linkifyFilePaths: false };
const content = createOutputContent(outputInfo.id, stackTrace ?? '', outputOptions);
const contentParent = document.createElement('div');
contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap);
const content = createOutputContent(outputInfo.id, formattedStack, outputOptions);
const stackTraceElement = document.createElement('div');
stackTraceElement.appendChild(content);
stackTraceElement.classList.toggle('word-wrap', ctx.settings.outputWordWrap);
disposableStore.push(ctx.onDidChangeSettings(e => {
contentParent.classList.toggle('word-wrap', e.outputWordWrap);
stackTraceElement.classList.toggle('word-wrap', e.outputWordWrap);
}));
contentParent.classList.toggle('scrollable', outputScrolling);
contentParent.appendChild(content);
outputElement.appendChild(contentParent);
initializeScroll(contentParent, disposableStore);
if (minimalError) {
createMinimalError(errorLocation, headerMessage, stackTraceElement, outputElement);
} else {
stackTraceElement.classList.toggle('scrollable', outputScrolling);
outputElement.appendChild(stackTraceElement);
initializeScroll(stackTraceElement, disposableStore);
}
} else {
const header = document.createElement('div');
const headerMessage = err.name && err.message ? `${err.name}: ${err.message}` : err.name || err.message;
if (headerMessage) {
header.innerText = headerMessage;
outputElement.appendChild(header);
@ -216,6 +223,54 @@ function renderError(
return disposableStore;
}
function createMinimalError(errorLocation: string | undefined, headerMessage: string, stackTrace: HTMLDivElement, outputElement: HTMLElement) {
const outputDiv = document.createElement('div');
const headerSection = document.createElement('div');
headerSection.classList.add('error-output-header');
if (errorLocation && errorLocation.indexOf('<a') === 0) {
headerSection.innerHTML = errorLocation;
}
const header = document.createElement('span');
header.innerText = headerMessage;
headerSection.appendChild(header);
outputDiv.appendChild(headerSection);
function addButton(linkElement: HTMLElement) {
const button = document.createElement('li');
button.appendChild(linkElement);
// the :hover css selector doesn't work in the webview,
// so we need to add the hover class manually
button.onmouseover = function () {
button.classList.add('hover');
};
button.onmouseout = function () {
button.classList.remove('hover');
};
return button;
}
const buttons = document.createElement('ul');
buttons.classList.add('error-output-actions');
outputDiv.appendChild(buttons);
const toggleStackLink = document.createElement('a');
toggleStackLink.innerText = 'Show Details';
toggleStackLink.href = '#!';
buttons.appendChild(addButton(toggleStackLink));
toggleStackLink.onclick = (e) => {
e.preventDefault();
const hidden = stackTrace.style.display === 'none';
stackTrace.style.display = hidden ? '' : 'none';
toggleStackLink.innerText = hidden ? 'Hide Details' : 'Show Details';
};
outputDiv.appendChild(stackTrace);
stackTrace.style.display = 'none';
outputElement.appendChild(outputDiv);
}
function getPreviousMatchingContentGroup(outputElement: HTMLElement) {
const outputContainer = outputElement.parentElement;
let match: HTMLElement | undefined = undefined;
@ -449,6 +504,35 @@ export const activate: ActivationFunction<void> = (ctx) => {
.traceback .code-underline {
text-decoration: underline;
}
#container ul.error-output-actions {
margin: 0px;
padding: 6px 0px 0px 6px;
padding-inline-start: 0px;
}
#container .error-output-actions li {
padding: 0px 4px 0px 4px;
border-radius: 5px;
height: 20px;
display: inline-flex;
cursor: pointer;
border: solid 1px var(--vscode-notebook-cellToolbarSeparator);
}
#container .error-output-actions li.hover {
background-color: var(--vscode-toolbar-hoverBackground);
}
#container .error-output-actions li:focus-within {
border-color: var(--theme-input-focus-border-color);
}
#container .error-output-actions a:focus {
outline: 0;
}
#container .error-output-actions li a {
color: var(--vscode-foreground);
text-decoration: none;
}
#container .error-output-header a {
padding-right: 12px;
}
`;
document.body.appendChild(style);

View file

@ -33,6 +33,7 @@ export interface RenderOptions {
readonly outputScrolling: boolean;
readonly outputWordWrap: boolean;
readonly linkifyFilePaths: boolean;
readonly minimalError: boolean;
}
export type IRichRenderContext = RendererContext<void> & { readonly settings: RenderOptions; readonly onDidChangeSettings: Event<RenderOptions> };

View file

@ -3,7 +3,7 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
export function formatStackTrace(stack: string) {
export function formatStackTrace(stack: string): { formattedStack: string; errorLocation?: string } {
let cleaned: string;
// Ansi colors are described here:
// https://en.wikipedia.org/wiki/ANSI_escape_code under the SGR section
@ -26,7 +26,7 @@ export function formatStackTrace(stack: string) {
return linkifyStack(cleaned);
}
return cleaned;
return { formattedStack: cleaned };
}
const formatSequence = /\u001b\[.+?m/g;
@ -49,10 +49,11 @@ type fileLocation = { kind: 'file'; path: string };
type location = cellLocation | fileLocation;
function linkifyStack(stack: string) {
function linkifyStack(stack: string): { formattedStack: string; errorLocation?: string } {
const lines = stack.split('\n');
let fileOrCell: location | undefined;
let locationLink = '';
for (const i in lines) {
@ -67,7 +68,9 @@ function linkifyStack(stack: string) {
kind: 'cell',
path: stripFormatting(original.replace(cellRegex, 'vscode-notebook-cell:?execution_count=$<executionCount>'))
};
lines[i] = original.replace(cellRegex, `$<prefix><a href=\'${fileOrCell.path}&line=$<lineNumber>\'>line $<lineNumber></a>`);
const link = original.replace(cellRegex, `<a href=\'${fileOrCell.path}&line=$<lineNumber>\'>line $<lineNumber></a>`);
lines[i] = original.replace(cellRegex, `$<prefix>${link}`);
locationLink = locationLink || link;
continue;
} else if (inputRegex.test(original)) {
@ -75,7 +78,8 @@ function linkifyStack(stack: string) {
kind: 'cell',
path: stripFormatting(original.replace(inputRegex, 'vscode-notebook-cell:?execution_count=$<executionCount>'))
};
lines[i] = original.replace(inputRegex, `Input <a href=\'${fileOrCell.path}>\'>$<cellLabel></a>$<postfix>`);
const link = original.replace(inputRegex, `<a href=\'${fileOrCell.path}\'>$<cellLabel></a>`);
lines[i] = original.replace(inputRegex, `Input ${link}$<postfix>`);
continue;
} else if (!fileOrCell || original.trim() === '') {
@ -94,5 +98,6 @@ function linkifyStack(stack: string) {
}
}
return lines.join('\n');
const errorLocation = locationLink;
return { formattedStack: lines.join('\n'), errorLocation };
}

View file

@ -16,7 +16,7 @@ suite('StackTraceHelper', () => {
'@Main c:\\src\\test\\3\\otherlanguages\\julia.ipynb: 3\n' +
'[2] top - level scope\n' +
'@c:\\src\\test\\3\\otherlanguages\\julia.ipynb: 1; ';
assert.equal(formatStackTrace(stack), stack);
assert.equal(formatStackTrace(stack).formattedStack, stack);
});
const formatSequence = /\u001b\[.+?m/g;
@ -37,10 +37,12 @@ suite('StackTraceHelper', () => {
'\u001b[1;32m----> 2\u001b[0m \u001b[38;5;28;01mraise\u001b[39;00m \u001b[38;5;167;01mException\u001b[39;00m\n\n' +
'\u001b[1;31mException\u001b[0m\n:';
const formatted = stripAsciiFormatting(formatStackTrace(stack));
assert.ok(formatted.indexOf('Cell In[3], <a href=\'vscode-notebook-cell:?execution_count=3&line=2\'>line 2</a>') > 0, 'Missing line link in ' + formatted);
assert.ok(formatted.indexOf('<a href=\'vscode-notebook-cell:?execution_count=3&line=2\'>2</a>') > 0, 'Missing frame link in ' + formatted);
assert.ok(formatted.indexOf('<a href=\'C:\\venvs\\myLib.py:2\'>2</a>') > 0, 'Missing frame link in ' + formatted);
const { formattedStack, errorLocation } = formatStackTrace(stack);
const cleanStack = stripAsciiFormatting(formattedStack);
assert.ok(cleanStack.indexOf('Cell In[3], <a href=\'vscode-notebook-cell:?execution_count=3&line=2\'>line 2</a>') > 0, 'Missing line link in ' + cleanStack);
assert.ok(cleanStack.indexOf('<a href=\'vscode-notebook-cell:?execution_count=3&line=2\'>2</a>') > 0, 'Missing frame link in ' + cleanStack);
assert.ok(cleanStack.indexOf('<a href=\'C:\\venvs\\myLib.py:2\'>2</a>') > 0, 'Missing frame link in ' + cleanStack);
assert.equal(errorLocation, '<a href=\'vscode-notebook-cell:?execution_count=3&line=2\'>line 2</a>');
});
test('IPython stack line numbers are linkified for IPython 8.3', () => {
@ -65,9 +67,10 @@ suite('StackTraceHelper', () => {
'\n' +
'\u001b[1;31mException\u001b[0m:\n';
const formatted = stripAsciiFormatting(formatStackTrace(stack));
assert.ok(formatted.indexOf('Input <a href=\'vscode-notebook-cell:?execution_count=2>\'>In [2]</a>, in <cell line: 5>') > 0, 'Missing cell link in ' + formatted);
assert.ok(formatted.indexOf('Input <a href=\'vscode-notebook-cell:?execution_count=1>\'>In [1]</a>, in myfunc()') > 0, 'Missing cell link in ' + formatted);
const { formattedStack } = formatStackTrace(stack);
const formatted = stripAsciiFormatting(formattedStack);
assert.ok(formatted.indexOf('Input <a href=\'vscode-notebook-cell:?execution_count=2\'>In [2]</a>, in <cell line: 5>') > 0, 'Missing cell link in ' + formatted);
assert.ok(formatted.indexOf('Input <a href=\'vscode-notebook-cell:?execution_count=1\'>In [1]</a>, in myfunc()') > 0, 'Missing cell link in ' + formatted);
assert.ok(formatted.indexOf('<a href=\'vscode-notebook-cell:?execution_count=2&line=5\'>5</a>') > 0, 'Missing frame link in ' + formatted);
});
@ -82,7 +85,7 @@ suite('StackTraceHelper', () => {
'\u001b[1;32m----> 2\u001b[0m \u001b[38;5;28;01mraise\u001b[39;00m \u001b[38;5;167;01mException\u001b[39;00m\n\n' +
'\u001b[1;31mException\u001b[0m\n:';
const formatted = formatStackTrace(stack);
const formatted = formatStackTrace(stack).formattedStack;
assert.ok(!/<a href=.*>\d<\/a>/.test(formatted), formatted);
});
@ -97,7 +100,7 @@ suite('StackTraceHelper', () => {
'a 1 print(\n' +
' 1a print(\n';
const formattedLines = formatStackTrace(stack).split('\n');
const formattedLines = formatStackTrace(stack).formattedStack.split('\n');
assert.ok(/<a href='vscode-notebook-cell.*>/.test(formattedLines[0]), 'line should contain a link: ' + formattedLines[0]);
formattedLines.slice(1).forEach(line => assert.ok(!/<a href=.*>/.test(line), 'line should not contain a link: ' + line));
});

View file

@ -960,6 +960,12 @@ configurationRegistry.registerConfiguration({
default: true,
tags: ['notebookOutputLayout']
},
[NotebookSetting.minimalErrorRendering]: {
description: nls.localize('notebook.minimalErrorRendering', "Control whether to render error output in a minimal style."),
type: 'boolean',
default: false,
tags: ['notebookOutputLayout']
},
[NotebookSetting.markupFontSize]: {
markdownDescription: nls.localize('notebook.markup.fontSize', "Controls the font size in pixels of rendered markup in notebooks. When set to {0}, 120% of {1} is used.", '`0`', '`#editor.fontSize#`'),
type: 'number',

View file

@ -374,6 +374,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
|| e.outputWordWrap
|| e.outputScrolling
|| e.outputLinkifyFilePaths
|| e.minimalError
) {
this._styleElement?.remove();
this._createLayoutStyles();

View file

@ -41,6 +41,7 @@ export interface NotebookDisplayOptions { // TODO @Yoyokrazy rename to a more ge
outputWordWrap: boolean;
outputLineLimit: number;
outputLinkifyFilePaths: boolean;
outputMinimalError: boolean;
fontSize: number;
outputFontSize: number;
outputFontFamily: string;
@ -103,6 +104,7 @@ export interface NotebookOptionsChangeEvent {
readonly outputWordWrap?: boolean;
readonly outputScrolling?: boolean;
readonly outputLinkifyFilePaths?: boolean;
readonly minimalError?: boolean;
}
const defaultConfigConstants = Object.freeze({
@ -206,6 +208,7 @@ export class NotebookOptions extends Disposable {
const outputWordWrap = this.configurationService.getValue<boolean>(NotebookSetting.outputWordWrap);
const outputLineLimit = this.configurationService.getValue<number>(NotebookSetting.textOutputLineLimit) ?? 30;
const linkifyFilePaths = this.configurationService.getValue<boolean>(NotebookSetting.LinkifyOutputFilePaths) ?? true;
const minimalErrors = this.configurationService.getValue<boolean>(NotebookSetting.minimalErrorRendering);
const editorTopPadding = this._computeEditorTopPadding();
@ -250,7 +253,8 @@ export class NotebookOptions extends Disposable {
outputScrolling: outputScrolling,
outputWordWrap: outputWordWrap,
outputLineLimit: outputLineLimit,
outputLinkifyFilePaths: linkifyFilePaths
outputLinkifyFilePaths: linkifyFilePaths,
outputMinimalError: minimalErrors
};
this._register(this.configurationService.onDidChangeConfiguration(e => {
@ -415,6 +419,7 @@ export class NotebookOptions extends Disposable {
const outputScrolling = e.affectsConfiguration(NotebookSetting.outputScrolling);
const outputWordWrap = e.affectsConfiguration(NotebookSetting.outputWordWrap);
const outputLinkifyFilePaths = e.affectsConfiguration(NotebookSetting.LinkifyOutputFilePaths);
const minimalError = e.affectsConfiguration(NotebookSetting.minimalErrorRendering);
if (
!cellStatusBarVisibility
@ -441,7 +446,8 @@ export class NotebookOptions extends Disposable {
&& !outputLineHeight
&& !outputScrolling
&& !outputWordWrap
&& !outputLinkifyFilePaths) {
&& !outputLinkifyFilePaths
&& !minimalError) {
return;
}
@ -548,6 +554,10 @@ export class NotebookOptions extends Disposable {
configuration.outputLinkifyFilePaths = this.configurationService.getValue<boolean>(NotebookSetting.LinkifyOutputFilePaths);
}
if (minimalError) {
configuration.outputMinimalError = this.configurationService.getValue<boolean>(NotebookSetting.minimalErrorRendering);
}
this._layoutConfiguration = Object.freeze(configuration);
// trigger event
@ -576,7 +586,8 @@ export class NotebookOptions extends Disposable {
outputLineHeight,
outputScrolling,
outputWordWrap,
outputLinkifyFilePaths: outputLinkifyFilePaths
outputLinkifyFilePaths,
minimalError
});
}
@ -790,6 +801,7 @@ export class NotebookOptions extends Disposable {
outputWordWrap: this._layoutConfiguration.outputWordWrap,
outputLineLimit: this._layoutConfiguration.outputLineLimit,
outputLinkifyFilePaths: this._layoutConfiguration.outputLinkifyFilePaths,
minimalError: this._layoutConfiguration.outputMinimalError
};
}
@ -811,7 +823,8 @@ export class NotebookOptions extends Disposable {
outputScrolling: this._layoutConfiguration.outputScrolling,
outputWordWrap: this._layoutConfiguration.outputWordWrap,
outputLineLimit: this._layoutConfiguration.outputLineLimit,
outputLinkifyFilePaths: false
outputLinkifyFilePaths: false,
minimalError: false
};
}

View file

@ -119,6 +119,7 @@ interface BacklayerWebviewOptions {
readonly outputWordWrap: boolean;
readonly outputLineLimit: number;
readonly outputLinkifyFilePaths: boolean;
readonly minimalError: boolean;
}
@ -249,6 +250,7 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
outputScrolling: this.options.outputScrolling,
outputWordWrap: this.options.outputWordWrap,
linkifyFilePaths: this.options.outputLinkifyFilePaths,
minimalError: this.options.minimalError
}
});
}
@ -287,7 +289,8 @@ export class BackLayerWebView<T extends ICommonCellInfo> extends Themable {
lineLimit: this.options.outputLineLimit,
outputScrolling: this.options.outputScrolling,
outputWordWrap: this.options.outputWordWrap,
linkifyFilePaths: this.options.outputLinkifyFilePaths
linkifyFilePaths: this.options.outputLinkifyFilePaths,
minimalError: this.options.minimalError
};
const preloadScript = preloadsScriptStr(
{

View file

@ -70,6 +70,7 @@ export interface RenderOptions {
readonly outputScrolling: boolean;
readonly outputWordWrap: boolean;
readonly linkifyFilePaths: boolean;
readonly minimalError: boolean;
}
interface PreloadContext {
@ -1911,6 +1912,7 @@ async function webviewPreloads(ctx: PreloadContext) {
get outputScrolling() { return currentRenderOptions.outputScrolling; },
get outputWordWrap() { return currentRenderOptions.outputWordWrap; },
get linkifyFilePaths() { return currentRenderOptions.linkifyFilePaths; },
get minimalError() { return currentRenderOptions.minimalError; },
},
get onDidChangeSettings() { return settingChange.event; }
};

View file

@ -937,6 +937,7 @@ export const NotebookSetting = {
outputScrolling: 'notebook.output.scrolling',
textOutputLineLimit: 'notebook.output.textLineLimit',
LinkifyOutputFilePaths: 'notebook.output.linkifyFilePaths',
minimalErrorRendering: 'notebook.output.minimalErrorRendering',
formatOnSave: 'notebook.formatOnSave.enabled',
insertFinalNewline: 'notebook.insertFinalNewline',
formatOnCellExecution: 'notebook.formatOnCellExecution',