Skip to content

Commit

Permalink
update cell output api from value to data
Browse files Browse the repository at this point in the history
  • Loading branch information
@brettfo
brettfo committed May 26, 2021
1 parent 3e516a0 commit dcc66f1
Show file tree
Hide file tree
Showing 13 changed files with 183 additions and 161 deletions.
12 changes: 12 additions & 0 deletions NotebookTestScript.dib
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,18 @@ document.getElementById("output").innerText = `The value is ${x}.`;

#!markdown

# Execute the next cell, the output should be displayed as JSON like so:

``` json
{"Name":"Developer","Salary":42}
```

#!csharp

display(new { Name = "Developer", Salary = 42 }, "application/json");

#!markdown

# Execute the next cell and verify the following error appears:

```
Expand Down
7 changes: 3 additions & 4 deletions src/dotnet-interactive-vscode/common/interactiveClient.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -381,15 +381,14 @@ export class InteractiveClient {
}

private displayEventToCellOutput(disp: DisplayEvent): vscodeLike.NotebookCellOutput {
const encoder = new TextEncoder();
let outputItems: Array<vscodeLike.NotebookCellOutputItem> = [];
if (disp.formattedValues && disp.formattedValues.length > 0) {
for (let formatted of disp.formattedValues) {
let value: any = formatted.mimeType === 'application/json'
? JSON.parse(formatted.value)
: formatted.value;
let data = encoder.encode(formatted.value);
outputItems.push({
mime: formatted.mimeType,
value,
data,
});
}
}
Expand Down
28 changes: 15 additions & 13 deletions src/dotnet-interactive-vscode/common/interfaces/utilities.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {
NotebookCellErrorOutput,
NotebookCellTextOutput,
} from './contracts';
import * as notebook from './vscode-like';
import * as vscodeLike from './vscode-like';

export function isErrorOutput(arg: any): arg is NotebookCellErrorOutput {
return arg
Expand All @@ -25,17 +25,19 @@ export function isTextOutput(arg: any): arg is NotebookCellTextOutput {
&& typeof arg.text === 'string';
}

export function reshapeOutputValueForVsCode(mimeType: string, value: unknown): unknown {
if (mimeType === notebook.ErrorOutputMimeType &&
typeof value === 'string') {
// this looks suspiciously like an error message; make sure it's the shape that vs code expects
return {
ename: 'Error',
evalue: value,
traceback: [],
};
export function reshapeOutputValueForVsCode(value: Uint8Array | string, mime: string): Uint8Array {
if (typeof value === 'string') {
const encoder = new TextEncoder();
const dataString = mime === vscodeLike.ErrorOutputMimeType
? JSON.stringify({
ename: 'Error',
evalue: value,
traceback: [],
})
: value;
const data = encoder.encode(dataString);
return data;
} else {
return value;
}

// no change
return value;
}
2 changes: 1 addition & 1 deletion src/dotnet-interactive-vscode/common/interfaces/vscode-like.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ export const ErrorOutputMimeType = 'application/vnd.code.notebook.error';

export interface NotebookCellOutputItem {
readonly mime: string;
readonly value: Uint8Array | unknown;
readonly data: Uint8Array;
readonly metadata?: { [key: string]: any };
}

Expand Down
35 changes: 20 additions & 15 deletions src/dotnet-interactive-vscode/common/tests/unit/client.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,14 @@ describe('InteractiveClient tests', () => {
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'text/plain',
value: 'deferred output',
decodedData: 'deferred output',
}
]
},
Expand All @@ -90,7 +91,7 @@ describe('InteractiveClient tests', () => {
outputs: [
{
mime: 'text/html',
value: '2',
decodedData: '2',
}
]
}
Expand Down Expand Up @@ -156,13 +157,14 @@ describe('InteractiveClient tests', () => {
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'text/plain',
value: 'deferred output',
decodedData: 'deferred output',
}
]
},
Expand All @@ -171,7 +173,7 @@ describe('InteractiveClient tests', () => {
outputs: [
{
mime: 'text/html',
value: '2',
decodedData: '2',
}
]
}
Expand Down Expand Up @@ -252,13 +254,14 @@ describe('InteractiveClient tests', () => {
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'text/plain',
value: 'deferred output 1',
decodedData: 'deferred output 1',
}
]
},
Expand All @@ -267,7 +270,7 @@ describe('InteractiveClient tests', () => {
outputs: [
{
mime: 'text/html',
value: '2',
decodedData: '2',
}
]
},
Expand All @@ -276,7 +279,7 @@ describe('InteractiveClient tests', () => {
outputs: [
{
mime: 'text/plain',
value: 'deferred output 2',
decodedData: 'deferred output 2',
}
]
}
Expand Down Expand Up @@ -335,13 +338,14 @@ describe('InteractiveClient tests', () => {
// execute first command
let result1: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result1 = outputs, _ => { }, { token: 'token 1' });
expect(result1).to.deep.equal([
let decodedResults1 = decodeNotebookCellOutputs(result1);
expect(decodedResults1).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'text/html',
value: '1',
decodedData: '1',
}
]
}
Expand All @@ -353,13 +357,14 @@ describe('InteractiveClient tests', () => {
expect(result2).to.deep.equal([]);

// ensure first result array was updated
expect(result1).to.deep.equal([
decodedResults1 = decodeNotebookCellOutputs(result1);
expect(decodedResults1).to.deep.equal([
{
id: '2',
outputs: [
{
mime: 'text/html',
value: '2',
decodedData: '2',
}
]
}
Expand Down Expand Up @@ -478,7 +483,7 @@ describe('InteractiveClient tests', () => {
id: '1',
outputs: [{
mime: vscodeLike.ErrorOutputMimeType,
decodedValue: {
decodedData: {
name: 'Error',
message: 'Error: expected exception during submit',
},
Expand Down
27 changes: 7 additions & 20 deletions src/dotnet-interactive-vscode/common/tests/unit/misc.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { NotebookCellDisplayOutput, NotebookCellErrorOutput, NotebookCellTextOut
import { isDisplayOutput, isErrorOutput, isTextOutput, reshapeOutputValueForVsCode } from '../../interfaces/utilities';
import { isDotNetNotebookMetadata, isIpynbFile } from '../../ipynbUtilities';
import { createUri, debounce, executeSafe, getWorkingDirectoryForNotebook, isDotNetUpToDate, parse, processArguments, stringify } from '../../utilities';
import { decodeNotebookCellOutputs, decodeToString } from './utilities';

import * as vscodeLike from '../../interfaces/vscode-like';

Expand Down Expand Up @@ -207,33 +208,19 @@ describe('Miscellaneous tests', () => {

describe('vs code output value reshaping', () => {
it('error string is reshaped', () => {
const reshaped = reshapeOutputValueForVsCode(vscodeLike.ErrorOutputMimeType, 'some error message');
expect(reshaped).to.deep.equal({
const reshaped = reshapeOutputValueForVsCode('some error message', vscodeLike.ErrorOutputMimeType);
const decoded = JSON.parse(decodeToString(reshaped));
expect(decoded).to.deep.equal({
ename: 'Error',
evalue: 'some error message',
traceback: [],
});
});

it(`properly shaped output isn't reshaped`, () => {
const reshaped = reshapeOutputValueForVsCode(vscodeLike.ErrorOutputMimeType, { ename: 'Error2', evalue: 'some error message', traceback: [] });
expect(reshaped).to.deep.equal({
ename: 'Error2',
evalue: 'some error message',
traceback: [],
});
});

it('non-string error message is not reshaped', () => {
const reshaped = reshapeOutputValueForVsCode(vscodeLike.ErrorOutputMimeType, { some_deep_value: 'some error message' });
expect(reshaped).to.deep.equal({
some_deep_value: 'some error message',
});
});

it(`non-error mime type doesn't reshape output`, () => {
const reshaped = reshapeOutputValueForVsCode('text/plain', 'some error message');
expect(reshaped).to.equal('some error message');
const reshaped = reshapeOutputValueForVsCode('some error message', 'text/plain');
const decoded = decodeToString(reshaped);
expect(decoded).to.equal('some error message');
});
});

Expand Down
28 changes: 16 additions & 12 deletions src/dotnet-interactive-vscode/common/tests/unit/notebook.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
ReturnValueProducedType,
StandardOutputValueProducedType,
} from '../../interfaces/contracts';
import { createKernelTransportConfig, withFakeGlobalStorageLocation } from './utilities';
import { createKernelTransportConfig, decodeNotebookCellOutputs, withFakeGlobalStorageLocation } from './utilities';
import { createUri } from '../../utilities';
import { backupNotebook, languageToCellKind } from '../../interactiveNotebook';
import * as vscodeLike from '../../interfaces/vscode-like';
Expand Down Expand Up @@ -72,13 +72,14 @@ describe('Notebook tests', () => {
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, language, outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'text/html',
value: '2',
decodedData: '2',
}
]
}
Expand Down Expand Up @@ -159,13 +160,14 @@ Console.WriteLine(1);
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'text/plain',
value: '1\r\n',
decodedData: '1\r\n',
}
],
},
Expand All @@ -174,7 +176,7 @@ Console.WriteLine(1);
outputs: [
{
mime: 'text/plain',
value: '2\r\n',
decodedData: '2\r\n',
}
]
},
Expand All @@ -183,7 +185,7 @@ Console.WriteLine(1);
outputs: [
{
mime: 'text/plain',
value: '3\r\n',
decodedData: '3\r\n',
}
]
}
Expand Down Expand Up @@ -254,13 +256,14 @@ Console.WriteLine(1);
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '2',
outputs: [
{
mime: 'text/plain',
value: 'Installed package Newtonsoft.Json version 1.2.3.4',
decodedData: 'Installed package Newtonsoft.Json version 1.2.3.4',
}
]
},
Expand All @@ -269,7 +272,7 @@ Console.WriteLine(1);
outputs: [
{
mime: 'text/plain',
value: 'sentinel',
decodedData: 'sentinel',
}
]
},
Expand Down Expand Up @@ -319,13 +322,14 @@ Console.WriteLine(1);
const client = await clientMapper.getOrAddClient(createUri('test/path'));
let result: Array<vscodeLike.NotebookCellOutput> = [];
await client.execute(code, 'csharp', outputs => result = outputs, _ => { }, { token });
expect(result).to.deep.equal([
const decodedResults = decodeNotebookCellOutputs(result);
expect(decodedResults).to.deep.equal([
{
id: '1',
outputs: [
{
mime: 'application/json',
value: {
decodedData: {
a: 1,
b: false,
}
Expand Down
Loading

0 comments on commit dcc66f1

Please sign in to comment.