From fc87e9baa918d358131c3fde94c48419160e7a70 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Wed, 17 Apr 2024 11:14:25 -0700 Subject: [PATCH 1/9] collapse stack trace --- extensions/notebook-renderers/src/index.ts | 38 +++++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index ef9240df2b78f..910d81b60a8a9 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -184,7 +184,43 @@ function renderError( return disposableStore; } - if (err.stack) { + if (err.name && err.message) { + const header = document.createElement('div'); + const headerMessage = `${err.name}: ${err.message}`; + header.innerText = headerMessage; + outputElement.appendChild(header); + + if (err.stack) { + const showStackLink = document.createElement('a'); + showStackLink.innerText = 'Show stack'; + showStackLink.href = '#'; + header.appendChild(showStackLink); + + outputElement.classList.add('traceback'); + + const stackTrace = formatStackTrace(err.stack); + + const outputOptions = { linesLimit: 1000, scrollable: false, trustHtml, linkifyFilePaths: false }; + + const content = createOutputContent(outputInfo.id, stackTrace, outputOptions); + const contentParent = document.createElement('div'); + contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap); + disposableStore.push(ctx.onDidChangeSettings(e => { + contentParent.classList.toggle('word-wrap', e.outputWordWrap); + })); + + contentParent.appendChild(content); + outputElement.appendChild(contentParent); + contentParent.style.display = 'none'; + + showStackLink.onclick = (e) => { + e.preventDefault(); + const hidden = contentParent.style.display === 'none'; + contentParent.style.display = hidden ? '' : 'none'; + showStackLink.innerText = hidden ? 'Hide stack' : 'Show stack'; + }; + } + } else if (err.stack) { outputElement.classList.add('traceback'); const stackTrace = formatStackTrace(err.stack); From 8a0c93cc170edf5352d4027467027e33578cf6a5 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Thu, 18 Apr 2024 11:23:14 -0700 Subject: [PATCH 2/9] interaction buttons --- extensions/notebook-renderers/src/index.ts | 74 ++++++++++++++++--- .../src/stackTraceHelper.ts | 19 +++-- 2 files changed, 75 insertions(+), 18 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 910d81b60a8a9..eb042d1036a7a 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -191,34 +191,58 @@ function renderError( outputElement.appendChild(header); if (err.stack) { - const showStackLink = document.createElement('a'); - showStackLink.innerText = 'Show stack'; - showStackLink.href = '#'; - header.appendChild(showStackLink); outputElement.classList.add('traceback'); - const stackTrace = formatStackTrace(err.stack); + const { formattedStack, errorLocation } = formatStackTrace(err.stack); const outputOptions = { linesLimit: 1000, scrollable: false, trustHtml, linkifyFilePaths: false }; - const content = createOutputContent(outputInfo.id, stackTrace, outputOptions); + const content = createOutputContent(outputInfo.id, formattedStack, outputOptions); const contentParent = document.createElement('div'); contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap); disposableStore.push(ctx.onDidChangeSettings(e => { contentParent.classList.toggle('word-wrap', e.outputWordWrap); })); - contentParent.appendChild(content); - outputElement.appendChild(contentParent); - contentParent.style.display = 'none'; + const buttons = document.createElement('ul'); + buttons.classList.add('error-output-actions'); + const toggleStack = document.createElement('li'); + toggleStack.onmouseover = function () { + toggleStack.classList.add('hover'); + }; + toggleStack.onmouseout = function () { + toggleStack.classList.remove('hover'); + }; + + const toggleStackLink = document.createElement('a'); + toggleStackLink.innerText = 'Show Details'; + toggleStack.appendChild(toggleStackLink); + buttons.appendChild(toggleStack); + outputElement.appendChild(buttons); + + if (errorLocation) { + const goToLocation = document.createElement('li'); + goToLocation.appendChild(errorLocation); + buttons.appendChild(goToLocation); + goToLocation.onmouseover = function () { + goToLocation.classList.add('hover'); + }; + goToLocation.onmouseout = function () { + goToLocation.classList.remove('hover'); + }; + } - showStackLink.onclick = (e) => { + toggleStackLink.onclick = (e) => { e.preventDefault(); const hidden = contentParent.style.display === 'none'; contentParent.style.display = hidden ? '' : 'none'; - showStackLink.innerText = hidden ? 'Hide stack' : 'Show stack'; + toggleStackLink.innerText = hidden ? 'Hide Details' : 'Show Details'; }; + + contentParent.appendChild(content); + outputElement.appendChild(contentParent); + contentParent.style.display = 'none'; } } else if (err.stack) { outputElement.classList.add('traceback'); @@ -228,7 +252,7 @@ function renderError( const outputScrolling = scrollingEnabled(outputInfo, ctx.settings); const outputOptions = { linesLimit: ctx.settings.lineLimit, scrollable: outputScrolling, trustHtml, linkifyFilePaths: ctx.settings.linkifyFilePaths }; - const content = createOutputContent(outputInfo.id, stackTrace ?? '', outputOptions); + const content = createOutputContent(outputInfo.id, stackTrace.formattedStack ?? '', outputOptions); const contentParent = document.createElement('div'); contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap); disposableStore.push(ctx.onDidChangeSettings(e => { @@ -485,6 +509,32 @@ export const activate: ActivationFunction = (ctx) => { .traceback .code-underline { text-decoration: underline; } + #container .error-output-actions li{ + padding: 0px 5px 0px 2px; + border-radius: 5px; + height: 22px; + display: inline-flex; + align-items: center; + margin-right: 8px; + line-height: 22px; + background: none; + cursor: pointer; + border-thickness: 1px; + border-color: var(--vscode-toolbar-hoverBackground); + } + #container .error-output-actions li.hover{ + background-color: var(--vscode-toolbar-hoverBackground); + cursor: pointer; + } + #container .error-output-actions li a{ + color: var(--vscode-foreground); + display:block; + text-decoration: none; + background-size: 16px; + padding: 0px 5px 0px 2px; + border-radius: 5px; + background-color: unset; + } `; document.body.appendChild(style); diff --git a/extensions/notebook-renderers/src/stackTraceHelper.ts b/extensions/notebook-renderers/src/stackTraceHelper.ts index 943877ad59d0c..8385d4f631213 100644 --- a/extensions/notebook-renderers/src/stackTraceHelper.ts +++ b/extensions/notebook-renderers/src/stackTraceHelper.ts @@ -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?: HTMLElement } { 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,12 @@ type fileLocation = { kind: 'file'; path: string }; type location = cellLocation | fileLocation; -function linkifyStack(stack: string) { +function linkifyStack(stack: string): { formattedStack: string; linkElement?: HTMLElement } { + const parser = new DOMParser(); const lines = stack.split('\n'); let fileOrCell: location | undefined; + let locationLink = ''; for (const i in lines) { @@ -67,7 +69,9 @@ function linkifyStack(stack: string) { kind: 'cell', path: stripFormatting(original.replace(cellRegex, 'vscode-notebook-cell:?execution_count=$')) }; - lines[i] = original.replace(cellRegex, `$\'>line $`); + const link = original.replace(cellRegex, `\'>line $`); + lines[i] = original.replace(cellRegex, `$${link}`); + locationLink = locationLink || link; continue; } else if (inputRegex.test(original)) { @@ -75,7 +79,9 @@ function linkifyStack(stack: string) { kind: 'cell', path: stripFormatting(original.replace(inputRegex, 'vscode-notebook-cell:?execution_count=$')) }; - lines[i] = original.replace(inputRegex, `Input \'>$$`); + const link = original.replace(inputRegex, `\'>$`); + lines[i] = original.replace(inputRegex, `Input ${link}$`); + locationLink = locationLink || link; continue; } else if (!fileOrCell || original.trim() === '') { @@ -94,5 +100,6 @@ function linkifyStack(stack: string) { } } - return lines.join('\n'); + const linkElement = parser.parseFromString(locationLink, 'text/html').body.firstChild as HTMLElement; + return { formattedStack: lines.join('\n'), linkElement }; } From ef4f9f79f25837922c8330cb53c4a353757d7b32 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Thu, 18 Apr 2024 11:28:27 -0700 Subject: [PATCH 3/9] fixup --- extensions/notebook-renderers/src/index.ts | 3 ++- extensions/notebook-renderers/src/stackTraceHelper.ts | 6 +++--- .../src/test/stackTraceHelper.test.ts | 10 +++++----- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index eb042d1036a7a..cfdfd3ea29473 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -519,7 +519,8 @@ export const activate: ActivationFunction = (ctx) => { line-height: 22px; background: none; cursor: pointer; - border-thickness: 1px; + border-width: 1px; + border-style: solid; border-color: var(--vscode-toolbar-hoverBackground); } #container .error-output-actions li.hover{ diff --git a/extensions/notebook-renderers/src/stackTraceHelper.ts b/extensions/notebook-renderers/src/stackTraceHelper.ts index 8385d4f631213..e35fb2b15be58 100644 --- a/extensions/notebook-renderers/src/stackTraceHelper.ts +++ b/extensions/notebook-renderers/src/stackTraceHelper.ts @@ -49,7 +49,7 @@ type fileLocation = { kind: 'file'; path: string }; type location = cellLocation | fileLocation; -function linkifyStack(stack: string): { formattedStack: string; linkElement?: HTMLElement } { +function linkifyStack(stack: string): { formattedStack: string; errorLocation?: HTMLElement } { const parser = new DOMParser(); const lines = stack.split('\n'); @@ -100,6 +100,6 @@ function linkifyStack(stack: string): { formattedStack: string; linkElement?: HT } } - const linkElement = parser.parseFromString(locationLink, 'text/html').body.firstChild as HTMLElement; - return { formattedStack: lines.join('\n'), linkElement }; + const errorLocation = parser.parseFromString(locationLink, 'text/html').body.firstChild as HTMLElement; + return { formattedStack: lines.join('\n'), errorLocation }; } diff --git a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts index c4e9917908606..71165fe120d57 100644 --- a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts +++ b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts @@ -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,7 +37,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 = stripAsciiFormatting(formatStackTrace(stack)); + const formatted = stripAsciiFormatting(formatStackTrace(stack).formattedStack); assert.ok(formatted.indexOf('Cell In[3], line 2') > 0, 'Missing line link in ' + formatted); assert.ok(formatted.indexOf('2') > 0, 'Missing frame link in ' + formatted); assert.ok(formatted.indexOf('2') > 0, 'Missing frame link in ' + formatted); @@ -65,7 +65,7 @@ suite('StackTraceHelper', () => { '\n' + '\u001b[1;31mException\u001b[0m:\n'; - const formatted = stripAsciiFormatting(formatStackTrace(stack)); + const formatted = stripAsciiFormatting(formatStackTrace(stack).formattedStack); assert.ok(formatted.indexOf('Input \'>In [2], in ') > 0, 'Missing cell link in ' + formatted); assert.ok(formatted.indexOf('Input \'>In [1], in myfunc()') > 0, 'Missing cell link in ' + formatted); assert.ok(formatted.indexOf('5') > 0, 'Missing frame link in ' + formatted); @@ -82,7 +82,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(!/\d<\/a>/.test(formatted), formatted); }); @@ -97,7 +97,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(/ assert.ok(!//.test(line), 'line should not contain a link: ' + line)); }); From 20cdd823b8dbd312c6b04912a3fd54bf3296284e Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Thu, 18 Apr 2024 11:52:39 -0700 Subject: [PATCH 4/9] place feature behind option --- extensions/notebook-renderers/src/index.ts | 2 +- .../notebook-renderers/src/rendererTypes.ts | 1 + .../notebook/browser/notebook.contribution.ts | 6 ++++++ .../notebook/browser/notebookEditorWidget.ts | 1 + .../notebook/browser/notebookOptions.ts | 21 +++++++++++++++---- .../view/renderers/backLayerWebView.ts | 5 ++++- .../browser/view/renderers/webviewPreloads.ts | 2 ++ .../contrib/notebook/common/notebookCommon.ts | 1 + 8 files changed, 33 insertions(+), 6 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index cfdfd3ea29473..255d04f008500 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -184,7 +184,7 @@ function renderError( return disposableStore; } - if (err.name && err.message) { + if (err.name && err.message && ctx.settings.minimalError) { const header = document.createElement('div'); const headerMessage = `${err.name}: ${err.message}`; header.innerText = headerMessage; diff --git a/extensions/notebook-renderers/src/rendererTypes.ts b/extensions/notebook-renderers/src/rendererTypes.ts index e1fc869a301ec..bab7a34af9f7e 100644 --- a/extensions/notebook-renderers/src/rendererTypes.ts +++ b/extensions/notebook-renderers/src/rendererTypes.ts @@ -33,6 +33,7 @@ export interface RenderOptions { readonly outputScrolling: boolean; readonly outputWordWrap: boolean; readonly linkifyFilePaths: boolean; + readonly minimalError: boolean; } export type IRichRenderContext = RendererContext & { readonly settings: RenderOptions; readonly onDidChangeSettings: Event }; diff --git a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts index 650e0bf50740a..e27f6630ef8e5 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebook.contribution.ts @@ -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', diff --git a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts index e3d523ed3bb86..ab543edd1ea2c 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookEditorWidget.ts @@ -374,6 +374,7 @@ export class NotebookEditorWidget extends Disposable implements INotebookEditorD || e.outputWordWrap || e.outputScrolling || e.outputLinkifyFilePaths + || e.minimalError ) { this._styleElement?.remove(); this._createLayoutStyles(); diff --git a/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts b/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts index 6679cb3356390..6cc994b7ff6a0 100644 --- a/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts +++ b/src/vs/workbench/contrib/notebook/browser/notebookOptions.ts @@ -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(NotebookSetting.outputWordWrap); const outputLineLimit = this.configurationService.getValue(NotebookSetting.textOutputLineLimit) ?? 30; const linkifyFilePaths = this.configurationService.getValue(NotebookSetting.LinkifyOutputFilePaths) ?? true; + const minimalErrors = this.configurationService.getValue(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(NotebookSetting.LinkifyOutputFilePaths); } + if (minimalError) { + configuration.outputMinimalError = this.configurationService.getValue(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 }; } diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts index 6a6e150c4a093..cf7965b0752f2 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -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 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 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( { diff --git a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts index acbf8be3152af..c350ab72ff48a 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/webviewPreloads.ts @@ -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; } }; diff --git a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts index 9f56fd024d37a..edcff89fb9f35 100644 --- a/src/vs/workbench/contrib/notebook/common/notebookCommon.ts +++ b/src/vs/workbench/contrib/notebook/common/notebookCommon.ts @@ -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', From 8146dd1c59f77c94fe7565cb7185946fc8fd01a9 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 19 Apr 2024 09:40:46 -0700 Subject: [PATCH 5/9] styling, more cleanup --- extensions/notebook-renderers/src/index.ts | 163 ++++++++++----------- 1 file changed, 78 insertions(+), 85 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 255d04f008500..4108d4d21d025 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -184,88 +184,35 @@ function renderError( return disposableStore; } - if (err.name && err.message && ctx.settings.minimalError) { - const header = document.createElement('div'); - const headerMessage = `${err.name}: ${err.message}`; - header.innerText = headerMessage; - outputElement.appendChild(header); - - if (err.stack) { - - outputElement.classList.add('traceback'); - - const { formattedStack, errorLocation } = formatStackTrace(err.stack); - - const outputOptions = { linesLimit: 1000, scrollable: false, trustHtml, linkifyFilePaths: false }; - - const content = createOutputContent(outputInfo.id, formattedStack, outputOptions); - const contentParent = document.createElement('div'); - contentParent.classList.toggle('word-wrap', ctx.settings.outputWordWrap); - disposableStore.push(ctx.onDidChangeSettings(e => { - contentParent.classList.toggle('word-wrap', e.outputWordWrap); - })); - - const buttons = document.createElement('ul'); - buttons.classList.add('error-output-actions'); - const toggleStack = document.createElement('li'); - toggleStack.onmouseover = function () { - toggleStack.classList.add('hover'); - }; - toggleStack.onmouseout = function () { - toggleStack.classList.remove('hover'); - }; - - const toggleStackLink = document.createElement('a'); - toggleStackLink.innerText = 'Show Details'; - toggleStack.appendChild(toggleStackLink); - buttons.appendChild(toggleStack); - outputElement.appendChild(buttons); - - if (errorLocation) { - const goToLocation = document.createElement('li'); - goToLocation.appendChild(errorLocation); - buttons.appendChild(goToLocation); - goToLocation.onmouseover = function () { - goToLocation.classList.add('hover'); - }; - goToLocation.onmouseout = function () { - goToLocation.classList.remove('hover'); - }; - } - - toggleStackLink.onclick = (e) => { - e.preventDefault(); - const hidden = contentParent.style.display === 'none'; - contentParent.style.display = hidden ? '' : 'none'; - toggleStackLink.innerText = hidden ? 'Hide Details' : 'Show Details'; - }; + const headerMessage = err.name && err.message ? `${err.name}: ${err.message}` : err.name || err.message; - contentParent.appendChild(content); - outputElement.appendChild(contentParent); - contentParent.style.display = 'none'; - } - } else if (err.stack) { + 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.formattedStack ?? '', 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); @@ -276,6 +223,54 @@ function renderError( return disposableStore; } +function createMinimalError(errorLocation: HTMLElement | undefined, headerMessage: string, stackTrace: HTMLDivElement, outputElement: HTMLElement) { + const outputDiv = document.createElement('div'); + const headerSection = document.createElement('div'); + + if (errorLocation) { + errorLocation.style.paddingRight = '12px'; + headerSection.appendChild(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; @@ -509,32 +504,30 @@ export const activate: ActivationFunction = (ctx) => { .traceback .code-underline { text-decoration: underline; } + #container .error-output-actions ul{ + padding: 6px 0px 0px 6px; + } #container .error-output-actions li{ padding: 0px 5px 0px 2px; border-radius: 5px; height: 22px; display: inline-flex; - align-items: center; - margin-right: 8px; - line-height: 22px; - background: none; cursor: pointer; - border-width: 1px; - border-style: solid; - border-color: var(--vscode-toolbar-hoverBackground); + border: solid 1px var(--vscode-notebook-cellToolbarSeparator); } #container .error-output-actions li.hover{ background-color: var(--vscode-toolbar-hoverBackground); - cursor: pointer; + } + #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); - display:block; - text-decoration: none; - background-size: 16px; padding: 0px 5px 0px 2px; - border-radius: 5px; - background-color: unset; + text-decoration: none; } `; document.body.appendChild(style); From 6ea2ca4429c2cd490aad910342c9922ea30659fc Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 19 Apr 2024 10:47:24 -0700 Subject: [PATCH 6/9] test returned link, fix test bug --- extensions/notebook-renderers/src/index.ts | 22 +++++++++++-------- .../src/stackTraceHelper.ts | 9 ++++---- .../src/test/stackTraceHelper.test.ts | 18 +++++++++------ 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 4108d4d21d025..1b34e46e067a6 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -223,13 +223,13 @@ function renderError( return disposableStore; } -function createMinimalError(errorLocation: HTMLElement | undefined, headerMessage: string, stackTrace: HTMLDivElement, outputElement: HTMLElement) { +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.style.paddingRight = '12px'; - headerSection.appendChild(errorLocation); + if (errorLocation && errorLocation.indexOf(' = (ctx) => { .traceback .code-underline { text-decoration: underline; } - #container .error-output-actions ul{ + #container ul.error-output-actions { + margin: 0px; padding: 6px 0px 0px 6px; } - #container .error-output-actions li{ + #container .error-output-actions li { padding: 0px 5px 0px 2px; border-radius: 5px; height: 22px; @@ -515,20 +516,23 @@ export const activate: ActivationFunction = (ctx) => { cursor: pointer; border: solid 1px var(--vscode-notebook-cellToolbarSeparator); } - #container .error-output-actions li.hover{ + #container .error-output-actions li.hover { background-color: var(--vscode-toolbar-hoverBackground); } - #container .error-output-actions li:focus-within{ + #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{ + #container .error-output-actions li a { color: var(--vscode-foreground); padding: 0px 5px 0px 2px; text-decoration: none; } + #container .error-output-header a { + padding-right: 12px; + } `; document.body.appendChild(style); diff --git a/extensions/notebook-renderers/src/stackTraceHelper.ts b/extensions/notebook-renderers/src/stackTraceHelper.ts index e35fb2b15be58..30394256d72bd 100644 --- a/extensions/notebook-renderers/src/stackTraceHelper.ts +++ b/extensions/notebook-renderers/src/stackTraceHelper.ts @@ -3,7 +3,7 @@ * Licensed under the MIT License. See License.txt in the project root for license information. *--------------------------------------------------------------------------------------------*/ -export function formatStackTrace(stack: string): { formattedStack: string; errorLocation?: HTMLElement } { +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 @@ -49,8 +49,7 @@ type fileLocation = { kind: 'file'; path: string }; type location = cellLocation | fileLocation; -function linkifyStack(stack: string): { formattedStack: string; errorLocation?: HTMLElement } { - const parser = new DOMParser(); +function linkifyStack(stack: string): { formattedStack: string; errorLocation?: string } { const lines = stack.split('\n'); let fileOrCell: location | undefined; @@ -79,7 +78,7 @@ function linkifyStack(stack: string): { formattedStack: string; errorLocation?: kind: 'cell', path: stripFormatting(original.replace(inputRegex, 'vscode-notebook-cell:?execution_count=$')) }; - const link = original.replace(inputRegex, `\'>$`); + const link = original.replace(inputRegex, `$`); lines[i] = original.replace(inputRegex, `Input ${link}$`); locationLink = locationLink || link; @@ -100,6 +99,6 @@ function linkifyStack(stack: string): { formattedStack: string; errorLocation?: } } - const errorLocation = parser.parseFromString(locationLink, 'text/html').body.firstChild as HTMLElement; + const errorLocation = locationLink; return { formattedStack: lines.join('\n'), errorLocation }; } diff --git a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts index 71165fe120d57..6e318cdc569e0 100644 --- a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts +++ b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts @@ -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).formattedStack); - assert.ok(formatted.indexOf('Cell In[3], line 2') > 0, 'Missing line link in ' + formatted); - assert.ok(formatted.indexOf('2') > 0, 'Missing frame link in ' + formatted); - assert.ok(formatted.indexOf('2') > 0, 'Missing frame link in ' + formatted); + const { formattedStack, errorLocation } = formatStackTrace(stack); + const cleanStack = stripAsciiFormatting(formattedStack); + assert.ok(cleanStack.indexOf('Cell In[3], line 2') > 0, 'Missing line link in ' + cleanStack); + assert.ok(cleanStack.indexOf('2') > 0, 'Missing frame link in ' + cleanStack); + assert.ok(cleanStack.indexOf('2') > 0, 'Missing frame link in ' + cleanStack); + assert.equal(errorLocation, 'line 2'); }); test('IPython stack line numbers are linkified for IPython 8.3', () => { @@ -65,10 +67,12 @@ suite('StackTraceHelper', () => { '\n' + '\u001b[1;31mException\u001b[0m:\n'; - const formatted = stripAsciiFormatting(formatStackTrace(stack).formattedStack); - assert.ok(formatted.indexOf('Input \'>In [2], in ') > 0, 'Missing cell link in ' + formatted); - assert.ok(formatted.indexOf('Input \'>In [1], in myfunc()') > 0, 'Missing cell link in ' + formatted); + const { formattedStack, errorLocation } = formatStackTrace(stack); + const formatted = stripAsciiFormatting(formattedStack); + assert.ok(formatted.indexOf('Input In [2], in ') > 0, 'Missing cell link in ' + formatted); + assert.ok(formatted.indexOf('Input In [1], in myfunc()') > 0, 'Missing cell link in ' + formatted); assert.ok(formatted.indexOf('5') > 0, 'Missing frame link in ' + formatted); + assert.equal(errorLocation, 'In [2]'); }); test('IPython stack trace lines without associated location are not linkified', () => { From 35669024eb72936f825f03b1bdbd77faf96f7422 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 19 Apr 2024 11:29:00 -0700 Subject: [PATCH 7/9] test header link, skip header link for old IPython --- extensions/notebook-renderers/src/stackTraceHelper.ts | 1 - .../notebook-renderers/src/test/stackTraceHelper.test.ts | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/extensions/notebook-renderers/src/stackTraceHelper.ts b/extensions/notebook-renderers/src/stackTraceHelper.ts index 30394256d72bd..ecf0eddb40ebc 100644 --- a/extensions/notebook-renderers/src/stackTraceHelper.ts +++ b/extensions/notebook-renderers/src/stackTraceHelper.ts @@ -80,7 +80,6 @@ function linkifyStack(stack: string): { formattedStack: string; errorLocation?: }; const link = original.replace(inputRegex, `$`); lines[i] = original.replace(inputRegex, `Input ${link}$`); - locationLink = locationLink || link; continue; } else if (!fileOrCell || original.trim() === '') { diff --git a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts index 6e318cdc569e0..54ec15b428caa 100644 --- a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts +++ b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts @@ -67,12 +67,11 @@ suite('StackTraceHelper', () => { '\n' + '\u001b[1;31mException\u001b[0m:\n'; - const { formattedStack, errorLocation } = formatStackTrace(stack); + const { formattedStack } = formatStackTrace(stack); const formatted = stripAsciiFormatting(formattedStack); assert.ok(formatted.indexOf('Input In [2], in ') > 0, 'Missing cell link in ' + formatted); assert.ok(formatted.indexOf('Input In [1], in myfunc()') > 0, 'Missing cell link in ' + formatted); assert.ok(formatted.indexOf('5') > 0, 'Missing frame link in ' + formatted); - assert.equal(errorLocation, 'In [2]'); }); test('IPython stack trace lines without associated location are not linkified', () => { From adab9492e6c5a534351b2d2741bbc2fece4c5056 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 19 Apr 2024 15:26:37 -0700 Subject: [PATCH 8/9] padding --- extensions/notebook-renderers/src/index.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 1b34e46e067a6..0fcc758cf2f45 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -509,9 +509,9 @@ export const activate: ActivationFunction = (ctx) => { padding: 6px 0px 0px 6px; } #container .error-output-actions li { - padding: 0px 5px 0px 2px; + padding: 0px 4px 0px 4px; border-radius: 5px; - height: 22px; + height: 20px; display: inline-flex; cursor: pointer; border: solid 1px var(--vscode-notebook-cellToolbarSeparator); @@ -527,7 +527,6 @@ export const activate: ActivationFunction = (ctx) => { } #container .error-output-actions li a { color: var(--vscode-foreground); - padding: 0px 5px 0px 2px; text-decoration: none; } #container .error-output-header a { From e10fbf4e4a43be031f81ec4c1f73ffadef500166 Mon Sep 17 00:00:00 2001 From: Aaron Munger Date: Fri, 19 Apr 2024 15:45:54 -0700 Subject: [PATCH 9/9] remove inline margin --- extensions/notebook-renderers/src/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/extensions/notebook-renderers/src/index.ts b/extensions/notebook-renderers/src/index.ts index 0fcc758cf2f45..66b6b2cb28799 100644 --- a/extensions/notebook-renderers/src/index.ts +++ b/extensions/notebook-renderers/src/index.ts @@ -507,6 +507,7 @@ export const activate: ActivationFunction = (ctx) => { #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;