From a9ee16c1351edfaec823519eec2dde2b3cc6ebd7 Mon Sep 17 00:00:00 2001 From: aamunger Date: Tue, 17 Oct 2023 11:10:03 -0700 Subject: [PATCH] handle new URI format from webview --- .../src/stackTraceHelper.ts | 22 +++++---- .../src/test/stackTraceHelper.test.ts | 4 +- .../view/renderers/backLayerWebView.ts | 45 ++++++++++++++++--- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/extensions/notebook-renderers/src/stackTraceHelper.ts b/extensions/notebook-renderers/src/stackTraceHelper.ts index dbab8718f94..e221a779c62 100644 --- a/extensions/notebook-renderers/src/stackTraceHelper.ts +++ b/extensions/notebook-renderers/src/stackTraceHelper.ts @@ -44,10 +44,15 @@ function stripFormatting(text: string) { return text.replace(formatSequence, ''); } +type cellLocation = { kind: 'cell'; path: string }; +type fileLocation = { kind: 'file'; path: string }; + +type location = cellLocation | fileLocation; + function linkifyStack(stack: string) { const lines = stack.split('\n'); - let fileOrCell: string | undefined; + let fileOrCell: location | undefined; for (const i in lines) { @@ -55,20 +60,20 @@ function linkifyStack(stack: string) { console.log(`linkify ${original}`); // REMOVE if (fileRegex.test(original)) { const fileMatch = lines[i].match(fileRegex); - fileOrCell = stripFormatting(fileMatch![1]); + fileOrCell = { kind: 'file', path: stripFormatting(fileMatch![1]) }; console.log(`matched file ${fileOrCell}`); // REMOVE continue; } else if (cellRegex.test(original)) { lines[i] = original.replace(cellRegex, (_s, cellLabel, executionCount, suffix) => { - fileOrCell = `vscode-notebook-cell:?execution=${stripFormatting(executionCount)}`; - return `${stripFormatting(cellLabel)}${suffix}`; + fileOrCell = { kind: 'cell', path: `vscode-notebook-cell:?execution=${stripFormatting(executionCount)}` }; + return `${stripFormatting(cellLabel)}${suffix}`; }); console.log(`matched cell ${fileOrCell}`); // REMOVE continue; } else if (inputRegex.test(original)) { lines[i] = original.replace(inputRegex, (_s, cellLabel, executionCount, suffix) => { - fileOrCell = `vscode-notebook-cell:?execution=${stripFormatting(executionCount)}`; - return `${stripFormatting(cellLabel)}${suffix}`; + fileOrCell = { kind: 'cell', path: `vscode-notebook-cell:?execution=${stripFormatting(executionCount)}` }; + return `${stripFormatting(cellLabel)}${suffix}`; }); console.log(`matched cell ${fileOrCell}`); // REMOVE continue; @@ -77,9 +82,10 @@ function linkifyStack(stack: string) { fileOrCell = undefined; continue; } else if (lineNumberRegex.test(original)) { - lines[i] = original.replace(lineNumberRegex, (_s, prefix, num, suffix) => { - return `${prefix}${num}${suffix}`; + return fileOrCell?.kind === 'file' ? + `${prefix}${num}${suffix}` : + `${prefix}${num}${suffix}`; }); console.log(`matched line ${lines[i]}`); // REMOVE continue; diff --git a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts index 59404c78633..2b9cd73c95a 100644 --- a/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts +++ b/extensions/notebook-renderers/src/test/stackTraceHelper.test.ts @@ -33,7 +33,7 @@ suite('StackTraceHelper', () => { const formatted = formatStackTrace(stack); assert.ok(formatted.indexOf('Cell In[3]') > 0, formatted); - assert.ok(formatted.indexOf('2') > 0, formatted); + assert.ok(formatted.indexOf('2') > 0, formatted); assert.ok(formatted.indexOf('2') > 0, formatted); }); @@ -47,7 +47,7 @@ suite('StackTraceHelper', () => { const formatted = formatStackTrace(stack); assert.ok(formatted.indexOf('Input In [2]') > 0, formatted); - assert.ok(formatted.indexOf('2') > 0, formatted); + assert.ok(formatted.indexOf('2') > 0, formatted); }); test('IPython stack trace lines without associated location are not linkified', () => { 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 65ece00b65c..1cb11aaee93 100644 --- a/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts +++ b/src/vs/workbench/contrib/notebook/browser/view/renderers/backLayerWebView.ts @@ -907,18 +907,54 @@ export class BackLayerWebView extends Themable { } private _handleNotebookCellResource(uri: URI) { - const lineMatch = /\?line=(\d+)$/.exec(uri.fragment); + const lineMatch = /(?:^|&)line=([^&]+)/.exec(uri.query); + let editorOptions: ITextEditorOptions | undefined = undefined; if (lineMatch) { const parsedLineNumber = parseInt(lineMatch[1], 10); + if (!isNaN(parsedLineNumber)) { + const lineNumber = parsedLineNumber; + + editorOptions = { + selection: { startLineNumber: lineNumber, startColumn: 1 } + }; + } + } + + const executionMatch = /(?:^|&)execution=([^&]+)/.exec(uri.query); + const notebookResource = uri.path.length > 0 ? uri : this.documentUri; + if (executionMatch) { + const executionCount = parseInt(executionMatch[1], 10); + if (!isNaN(executionCount)) { + const notebookModel = this.notebookService.getNotebookTextModel(notebookResource); + const cell = notebookModel?.cells.find(cell => { + return cell.internalMetadata.executionOrder === executionCount; + }); + if (cell?.uri) { + this.openerService.open(cell.uri, { + fromUserGesture: true, + fromWorkspace: true, + editorOptions: editorOptions + }); + return; + } + } + } + + // URLs built by the jupyter extension put the line query param in the fragment + // They also have the cell fragment pre-calculated + const fragmentLineMatch = /\?line=(\d+)$/.exec(uri.fragment); + if (fragmentLineMatch) { + const parsedLineNumber = parseInt(fragmentLineMatch[1], 10); if (!isNaN(parsedLineNumber)) { const lineNumber = parsedLineNumber + 1; - const fragment = uri.fragment.substring(0, lineMatch.index); + const fragment = uri.fragment.substring(0, fragmentLineMatch.index); // open the uri with selection const editorOptions: ITextEditorOptions = { selection: { startLineNumber: lineNumber, startColumn: 1, endLineNumber: lineNumber, endColumn: 1 } }; - this.openerService.open(uri.with({ fragment }), { + + this.openerService.open(notebookResource.with({ fragment }), { fromUserGesture: true, fromWorkspace: true, editorOptions: editorOptions @@ -927,8 +963,7 @@ export class BackLayerWebView extends Themable { } } - this.openerService.open(uri, { fromUserGesture: true, fromWorkspace: true }); - return uri; + this.openerService.open(notebookResource, { fromUserGesture: true, fromWorkspace: true }); } private _handleResourceOpening(href: string) {