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.

Sign up for GitHub

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

Use text encoder decoder and not buffer #15102

Merged
merged 3 commits into from
Feb 1, 2024
Merged

Use text encoder decoder and not buffer #15102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
d7290b4
Use TextEncoder/TextDecoder and not Buffer
DonJayamanne Feb 1, 2024
207412e
oops
DonJayamanne Feb 1, 2024
241b78b
Fix tests
DonJayamanne Feb 1, 2024
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
4 changes: 2 additions & 2 deletions src/kernels/execution/cellExecutionMessageHandler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ export class CellExecutionMessageHandler implements IDisposable {
return true;
}
if (mime === WIDGET_MIMETYPE) {
const data: WidgetData = JSON.parse(Buffer.from(outputItem.data).toString());
const data: WidgetData = JSON.parse(new TextDecoder().decode(outputItem.data));
// Jupyter Output widgets cannot be rendered properly by the widget manager,
// We need to render that.
if (typeof data.model_id === 'string' && this.commIdsMappedToWidgetOutputModels.has(data.model_id)) {
Expand Down Expand Up @@ -763,7 +763,7 @@ export class CellExecutionMessageHandler implements IDisposable {
return false;
}
try {
const value = JSON.parse(Buffer.from(outputItem.data).toString()) as { model_id?: string };
const value = JSON.parse(new TextDecoder().decode(outputItem.data)) as { model_id?: string };
return value.model_id === expectedModelId;
} catch (ex) {
traceWarning(`Failed to deserialize the widget data`, ex);
Expand Down
128 changes: 100 additions & 28 deletions src/kernels/execution/cellExecutionMessageHandler.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
// Update the display data again.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayDataWorld, 1, (producer) => {
return [
Expand All @@ -429,7 +432,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'World'
);
});

test('Execute cell and add Display output (even if Cell DOM has not yet been updated) ', async () => {
Expand Down Expand Up @@ -471,7 +477,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
// Update the display data again.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayDataWorld, 1, (producer) => {
return [
Expand All @@ -482,7 +491,10 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'World'
);
});
test('Updates to two separate display updates in the same cell output', async () => {
const notebook = createNotebook([
Expand Down Expand Up @@ -520,8 +532,14 @@ suite(`Cell Execution Message Handler`, () => {
assert.strictEqual((output1.transient as any).display_id, display_id);
const output2 = translateCellDisplayOutput(notebook.cellAt(0).outputs[1]);
assert.strictEqual((output2.transient as any).display_id, display_id2);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'World'
);

// Update the first display data.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayData1ILike, 2, (producer) => {
Expand All @@ -534,8 +552,14 @@ suite(`Cell Execution Message Handler`, () => {
];
});
// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1ILike, 2, outputsFromILikeUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'World'
);

// Update the second display data.
await executeCellWithOutput(notebook.cellAt(2), codeToUpdateDisplayData1Pizza, 3, (producer) => {
Expand All @@ -548,8 +572,14 @@ suite(`Cell Execution Message Handler`, () => {
];
});
// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1Pizza, 3, outputsFromPizzaUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);
});

test('Updates to two separate display updates in the same cell output (update second display update)', async () => {
Expand Down Expand Up @@ -589,8 +619,14 @@ suite(`Cell Execution Message Handler`, () => {
assert.strictEqual((output1.transient as any).display_id, display_id);
const output2 = translateCellDisplayOutput(notebook.cellAt(0).outputs[1]);
assert.strictEqual((output2.transient as any).display_id, display_id2);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'World');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'World'
);

// Update the second display data.
await executeCellWithOutput(notebook.cellAt(1), codeToUpdateDisplayData1Pizza, 2, (producer) => {
Expand All @@ -604,8 +640,14 @@ suite(`Cell Execution Message Handler`, () => {
});

// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1Pizza, 2, outputsFromPizzaUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);

// Update the first display data.
await executeCellWithOutput(notebook.cellAt(2), codeToUpdateDisplayData1ILike, 3, (producer) => {
Expand All @@ -618,8 +660,14 @@ suite(`Cell Execution Message Handler`, () => {
];
});
// await executeAndUpdateDisplayData(notebook, codeToUpdateDisplayData1ILike, 3, outputsFromILikeUpdate);
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);
});

test('Updates to two separate display updates in the same cell output (even if Cell DOM has not yet been updated)', async () => {
Expand Down Expand Up @@ -667,8 +715,14 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Hello');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Hello'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);

// Update the first display data.
await executeCellWithOutput(notebook.cellAt(2), codeToUpdateDisplayData1ILike, 3, (producer) => {
Expand All @@ -680,8 +734,14 @@ suite(`Cell Execution Message Handler`, () => {
})
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'I Like');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'Pizza');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'I Like'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(),
'Pizza'
);
});

test('Updates display updates in the same cell output within the same execution (even if Cell DOM has not yet been updated) (Issue 12755, 13105, 13163)', async () => {
Expand Down Expand Up @@ -741,11 +801,23 @@ suite(`Cell Execution Message Handler`, () => {
producer.stream({ name: 'stdout', text: 'Pizza\n' })
];
});
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[0].items[0].data).toString(), 'Touch me not\n');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'C');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[2].items[0].data).toString(), 'Hello\n');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[2].items[1].data).toString(), 'World\n');
assert.strictEqual(Buffer.from(notebook.cellAt(0).outputs[2].items[2].data).toString(), 'Pizza\n');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[0].items[0].data).toString(),
'Touch me not\n'
);
assert.strictEqual(new TextDecoder().decode(notebook.cellAt(0).outputs[1].items[0].data).toString(), 'C');
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[2].items[0].data).toString(),
'Hello\n'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[2].items[1].data).toString(),
'World\n'
);
assert.strictEqual(
new TextDecoder().decode(notebook.cellAt(0).outputs[2].items[2].data).toString(),
'Pizza\n'
);
});
});

Expand Down Expand Up @@ -792,7 +864,7 @@ suite(`Cell Execution Message Handler`, () => {
for (let index = 0; index < cell.outputs[0].items.length; index++) {
const item = cell.outputs[0].items[index];
assert.strictEqual(item.mime, 'application/vnd.code.notebook.stdout');
assert.strictEqual(Buffer.from(item.data).toString(), `${index}\n`);
assert.strictEqual(new TextDecoder().decode(item.data).toString(), `${index}\n`);
}

// Now assume we closed VS Code, and then opened it again.
Expand Down Expand Up @@ -828,9 +900,9 @@ suite(`Cell Execution Message Handler`, () => {
const item = cell.outputs[0].items[index];
assert.strictEqual(item.mime, 'application/vnd.code.notebook.stdout');
if (index >= 50) {
assert.strictEqual(Buffer.from(item.data).toString(), `${25 + index}\n`);
assert.strictEqual(new TextDecoder().decode(item.data).toString(), `${25 + index}\n`);
} else {
assert.strictEqual(Buffer.from(item.data).toString(), `${index}\n`);
assert.strictEqual(new TextDecoder().decode(item.data).toString(), `${index}\n`);
}
}
}
Expand Down
24 changes: 5 additions & 19 deletions src/kernels/execution/helpers.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ import {
import { StopWatch } from '../../platform/common/utils/stopWatch';
import { getExtensionSpecifcStack } from '../../platform/errors/errors';
import { getVersion } from '../../platform/interpreter/helpers';
import { base64ToUint8Array, uint8ArrayToBase64 } from '../../platform/common/utils/string';

export enum CellOutputMimeTypes {
error = 'application/vnd.code.notebook.error',
Expand Down Expand Up @@ -112,7 +113,7 @@ const orderOfMimeTypes = [
function isEmptyVendoredMimeType(outputItem: NotebookCellOutputItem) {
if (outputItem.mime.startsWith('application/vnd.')) {
try {
return Buffer.from(outputItem.data).toString().length === 0;
return new TextDecoder().decode(outputItem.data).length === 0;
} catch {}
}
return false;
Expand Down Expand Up @@ -372,7 +373,7 @@ export function translateCellErrorOutput(output: NotebookCellOutput): nbformat.I
};
}
const originalError: undefined | nbformat.IError = output.metadata?.originalError;
const value: Error = JSON.parse(Buffer.from(firstItem.data as Uint8Array).toString('utf8'));
const value: Error = JSON.parse(new TextDecoder().decode(firstItem.data));
return {
output_type: 'error',
ename: value.name,
Expand Down Expand Up @@ -400,17 +401,7 @@ function convertOutputMimeToJupyterOutput(mime: string, value: Uint8Array) {
} else if (mime.startsWith('image/') && mime !== 'image/svg+xml') {
// Images in Jupyter are stored in base64 encoded format.
// VS Code expects bytes when rendering images.
if (typeof Buffer !== 'undefined' && typeof Buffer.from === 'function') {
return Buffer.from(value).toString('base64');
} else {
// https://developer.mozilla.org/en-US/docs/Glossary/Base64#solution_1_%E2%80%93_escaping_the_string_before_encoding_it
const stringValue = textDecoder.decode(value);
return btoa(
encodeURIComponent(stringValue).replace(/%([0-9A-F]{2})/g, function (_match, p1) {
return String.fromCharCode(Number.parseInt('0x' + p1));
})
);
}
return uint8ArrayToBase64(value);
} else if (
mime.toLowerCase().startsWith('application/vnd.holoviews_load.v') &&
mime.toLowerCase().endsWith('+json')
Expand Down Expand Up @@ -450,12 +441,7 @@ function convertJupyterOutputToBuffer(mime: string, value: unknown): NotebookCel
} else if (mime.startsWith('image/') && typeof value === 'string' && mime !== 'image/svg+xml') {
// Images in Jupyter are stored in base64 encoded format.
// VS Code expects bytes when rendering images.
if (typeof Buffer !== 'undefined' && typeof Buffer.from === 'function') {
return new NotebookCellOutputItem(Buffer.from(value, 'base64'), mime);
} else {
const data = Uint8Array.from(atob(value), (c) => c.charCodeAt(0));
return new NotebookCellOutputItem(data, mime);
}
return new NotebookCellOutputItem(base64ToUint8Array(value), mime);
} else if (typeof value === 'object' && value !== null && !Array.isArray(value)) {
return NotebookCellOutputItem.text(JSON.stringify(value), mime);
} else {
Expand Down
12 changes: 6 additions & 6 deletions src/kernels/kernelCrashMonitor.unit.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@ suite('Kernel Crash Monitor', () => {
const execution = controller.createNotebookCellExecution(cell);
execution.start();

const expectedErrorMessage = Buffer.from(
const expectedErrorMessage = new TextDecoder().decode(
createOutputWithErrorMessageForDisplay(DataScience.kernelCrashedDueToCodeInCurrentOrPreviousCell)?.items[0]!
.data!
).toString();
);

when(kernel.status).thenReturn('dead');
onKernelStatusChanged.fire({ status: 'dead', kernel: instance(kernel) });
Expand All @@ -127,7 +127,7 @@ suite('Kernel Crash Monitor', () => {
assert.strictEqual(cell.outputs.length, 1);
assert.strictEqual(cell.outputs[0].items.length, 1);
const outputItem = cell.outputs[0].items[0];
assert.include(Buffer.from(outputItem.data).toString(), expectedErrorMessage);
assert.include(new TextDecoder().decode(outputItem.data), expectedErrorMessage);
});
test('Error message displayed and Cell output updated with error message (jupyter kernel)', async () => {
when(kernelSession.kind).thenReturn('localJupyter');
Expand All @@ -139,10 +139,10 @@ suite('Kernel Crash Monitor', () => {
const execution = controller.createNotebookCellExecution(cell);
execution.start();

const expectedErrorMessage = Buffer.from(
const expectedErrorMessage = new TextDecoder().decode(
createOutputWithErrorMessageForDisplay(DataScience.kernelCrashedDueToCodeInCurrentOrPreviousCell)?.items[0]!
.data!
).toString();
);

when(kernel.status).thenReturn('autorestarting');
when(kernelSession.status).thenReturn('autorestarting');
Expand All @@ -160,6 +160,6 @@ suite('Kernel Crash Monitor', () => {
assert.strictEqual(cell.outputs.length, 1);
assert.strictEqual(cell.outputs[0].items.length, 1);
const outputItem = cell.outputs[0].items[0];
assert.include(Buffer.from(outputItem.data).toString(), expectedErrorMessage);
assert.include(new TextDecoder().decode(outputItem.data), expectedErrorMessage);
});
});
2 changes: 1 addition & 1 deletion src/notebooks/export/exportBase.web.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ export class ExportBase implements IExportBase {
});
const bytes = this.b64toBlob(content.content, 'application/pdf');
const buffer = await bytes.arrayBuffer();
await this.fs.writeFile(target!, Buffer.from(buffer));
await this.fs.writeFile(target!, new Uint8Array(buffer));
} else {
const content = await contentsManager.get(tempTarget, {
type: 'file',
Expand Down
Loading
Loading