Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Minimal error renderer #210767

Merged
merged 9 commits into from
Apr 22, 2024
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
108 changes: 96 additions & 12 deletions extensions/notebook-renderers/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);

Expand Down
1 change: 1 addition & 0 deletions extensions/notebook-renderers/src/rendererTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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> };
Expand Down
17 changes: 11 additions & 6 deletions extensions/notebook-renderers/src/stackTraceHelper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -26,7 +26,7 @@ export function formatStackTrace(stack: string) {
return linkifyStack(cleaned);
}

return cleaned;
return { formattedStack: cleaned };
}

const formatSequence = /\u001b\[.+?m/g;
Expand All @@ -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) {

Expand All @@ -67,15 +68,18 @@ 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)) {
fileOrCell = {
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() === '') {
Expand All @@ -94,5 +98,6 @@ function linkifyStack(stack: string) {
}
}

return lines.join('\n');
const errorLocation = locationLink;
return { formattedStack: lines.join('\n'), errorLocation };
}
23 changes: 13 additions & 10 deletions extensions/notebook-renderers/src/test/stackTraceHelper.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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', () => {
Expand All @@ -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);
});

Expand All @@ -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);
});

Expand All @@ -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));
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -374,6 +374,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD
|| e.outputWordWrap
|| e.outputScrolling
|| e.outputLinkifyFilePaths
|| e.minimalError
) {
this._styleElement?.remove();
this._createLayoutStyles();
Expand Down
21 changes: 17 additions & 4 deletions src/vs/workbench/contrib/notebook/browser/notebookOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -103,6 +104,7 @@ export interface NotebookOptionsChangeEvent {
readonly outputWordWrap?: boolean;
readonly outputScrolling?: boolean;
readonly outputLinkifyFilePaths?: boolean;
readonly minimalError?: boolean;
}

const defaultConfigConstants = Object.freeze({
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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 => {
Expand Down Expand Up @@ -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
Expand All @@ -441,7 +446,8 @@ export class NotebookOptions extends Disposable {
&& !outputLineHeight
&& !outputScrolling
&& !outputWordWrap
&& !outputLinkifyFilePaths) {
&& !outputLinkifyFilePaths
&& !minimalError) {
return;
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -576,7 +586,8 @@ export class NotebookOptions extends Disposable {
outputLineHeight,
outputScrolling,
outputWordWrap,
outputLinkifyFilePaths: outputLinkifyFilePaths
outputLinkifyFilePaths,
minimalError
});
}

Expand Down Expand Up @@ -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
};
}

Expand All @@ -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
};
}

Expand Down
Loading
Loading