Skip to content

Commit

Permalink
Minimal error renderer (microsoft#210767)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@amunger @sergioengineer
amunger authored and sergioengineer committed Apr 23, 2024
1 parent 2edff46 commit dd17b4c
Show file tree
Hide file tree
Showing 10 changed files with 152 additions and 33 deletions.
108 changes: 96 additions & 12 deletions extensions/notebook-renderers/src/index.ts
View file
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
View file
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
View file
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
View file
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
6 changes: 6 additions & 0 deletions src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts
View file
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
1 change: 1 addition & 0 deletions src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts
View file
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
View file
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

0 comments on commit dd17b4c

Please sign in to comment.