Compress streams in notebook outputs #160946
Conversation
| @@ -271,17 +270,21 @@ type JupyterOutput = | |||
|
|
|||
| function convertStreamOutput(output: NotebookCellOutput): JupyterOutput { | |||
| const outputs: string[] = []; | |||
| const compressedStream = output.items.length ? new TextDecoder().decode(compressOutputItemStreams(output.items[0].mime, output.items)) : ''; | |||
| // Ensure each line is a separate entry in an array (ending with \n). | |||
There was a problem hiding this comment.
Reverted code from previous PR
| * Given a stream of individual stdout outputs, this function will return the compressed lines, escaping some of the common terminal escape codes. | ||
| * E.g. some terminal escape codes would result in the previous line getting cleared, such if we had 3 lines and | ||
| * last line contained such a code, then the result string would be just the first two lines. | ||
| */ |
There was a problem hiding this comment.
Moved into a common area
| @@ -110,6 +110,14 @@ export class ExtHostCell { | |||
| output.items.length = 0; | |||
| } | |||
| output.items.push(...newItems); | |||
| if (output.items.every(item => notebookCommon.isTextStreamMime(item.mime))) { | |||
There was a problem hiding this comment.
TODO: This needs to be via a proposed API, have a specific method such as appendStreamOutputItem or the like
| * E.g. some terminal escape codes would result in the previous line getting cleared, such if we had 3 lines and | ||
| * last line contained such a code, then the result string would be just the first two lines. | ||
| */ | ||
| export function compressOutputItemStreams(mimeType: string, outputs: IOutputItemDto[]) { |
There was a problem hiding this comment.
Moved to a common area so the code can be shared
src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts
Outdated
Show resolved
Hide resolved
DonJayamanne
force-pushed
the
donjayamanne/nbOutputHandling
branch
from
7090b07
to
1448d42
Compare
DonJayamanne
force-pushed
the
donjayamanne/nbOutputHandling
branch
2 times, most recently
from
7649cd1
to
2803c8f
Compare
DonJayamanne
force-pushed
the
donjayamanne/nbOutputHandling
branch
from
fe55b73
to
5ca48e5
Compare
DonJayamanne
force-pushed
the
donjayamanne/nbOutputHandling
branch
from
5ca48e5
to
ba3371b
Compare
| if (output.items.length > 1 && output.items.every(item => notebookCommon.isTextStreamMime(item.mime))) { | ||
| // Look for the mimes in the items, and keep track of their order. | ||
| // Merge the streams into one output item, per mime type. | ||
| const mimeOutputs = new Map<string, Uint8Array[]>(); |
There was a problem hiding this comment.
Might be useful to share code between here and notebookCellTextModel?
There was a problem hiding this comment.
@roblourens where would i place such a common (shared) block of code.
I need to share code between the two files
- src/vs/workbench/api/common/extHostNotebookDocument.ts
- src/vs/workbench/contrib/notebook/common/model/notebookCellTextModel.ts
I couldn't find a way to share a file that ends up importing NotebookCellOutputItem.
Tried to add such a function in the following locations and got eslint errors about imports not being allowed
- src/vs/workbench/common/notebookCommon.ts
- src/vs/workbench/contrib/notebook/common/notebookCommon.ts
Hgere's the error I get
There was a problem hiding this comment.
Yeah, you're already calling into notebookCommon and that makes sense as a place to keep it, but you would have to do it in a way that doesn't directly import 'vscode', eg work with a different interface. Whatever you wanna do though.
There was a problem hiding this comment.
Oops, missed this, will work on that in debt.
DonJayamanne
force-pushed
the
donjayamanne/nbOutputHandling
branch
from
ee83af1
to
f9d02ea
Compare
This reverts commit 4230c22.
DonJayamanne
force-pushed
the
donjayamanne/nbOutputHandling
branch
from
f9d02ea
to
42522fd
Compare
Fixes #161023