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

Jump to bottom

Remove transform output calls #110855

Merged
merged 4 commits into from
Nov 19, 2020
Merged

Remove transform output calls #110855

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5dd81b4
move orderedMimeTypes and pickedMimeTypeIndex out of the domain model…
jrieken Nov 18, 2020
7ec1a68
remove transformXYZOutput
jrieken Nov 18, 2020
ca20be5
simplify ITransformedDisplayOutputDto type
jrieken Nov 18, 2020
11644ab
Merge branch 'master' into joh/pureoutput
jrieken Nov 18, 2020
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: 0 additions & 4 deletions src/vs/workbench/api/browser/mainThreadNotebook.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,6 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
if (!textModel) {
return false;
}
this._notebookService.transformEditsOutputs(textModel, cellEdits);
return textModel.applyEdits(modelVersionId, cellEdits, true, undefined, () => undefined, undefined);
}

Expand Down Expand Up @@ -479,8 +478,6 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
const edits: ICellEditOperation[] = [
{ editType: CellEditType.Replace, index: 0, count: mainthreadTextModel.cells.length, cells: data.cells }
];

this._notebookService.transformEditsOutputs(mainthreadTextModel, edits);
await new Promise(resolve => {
DOM.scheduleAtNextAnimationFrame(() => {
const ret = mainthreadTextModel!.applyEdits(mainthreadTextModel!.versionId, edits, true, undefined, () => undefined, undefined);
Expand Down Expand Up @@ -606,7 +603,6 @@ export class MainThreadNotebooks extends Disposable implements MainThreadNoteboo
return;
}

this._notebookService.transformSpliceOutputs(textModel, splices);
const cell = textModel.cells.find(cell => cell.handle === cellHandle);

if (!cell) {
Expand Down
3 changes: 0 additions & 3 deletions src/vs/workbench/contrib/bulkEdit/browser/bulkCellEdits.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import { IProgress } from 'vs/platform/progress/common/progress';
import { UndoRedoGroup } from 'vs/platform/undoRedo/common/undoRedo';
import { ICellEditOperation } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { INotebookEditorModelResolverService } from 'vs/workbench/contrib/notebook/common/notebookEditorModelResolverService';
import { INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';

export class ResourceNotebookCellEdit extends ResourceEdit {

Expand All @@ -32,7 +31,6 @@ export class BulkCellEdits {
private _undoRedoGroup: UndoRedoGroup,
private readonly _progress: IProgress<void>,
private readonly _edits: ResourceNotebookCellEdit[],
@INotebookService private readonly _notebookService: INotebookService,
@INotebookEditorModelResolverService private readonly _notebookModelService: INotebookEditorModelResolverService,
) { }

Expand All @@ -52,7 +50,6 @@ export class BulkCellEdits {

// apply edits
const edits = group.map(entry => entry.cellEdit);
this._notebookService.transformEditsOutputs(ref.object.notebook, edits);
ref.object.notebook.applyEdits(ref.object.notebook.versionId, edits, true, undefined, () => undefined, this._undoRedoGroup);
ref.dispose();

Expand Down
76 changes: 6 additions & 70 deletions src/vs/workbench/contrib/notebook/browser/notebookServiceImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import { NotebookKernelProviderAssociationRegistry, NotebookViewTypesExtensionRe
import { CellViewModel } from 'vs/workbench/contrib/notebook/browser/viewModel/notebookViewModel';
import { NotebookCellTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookCellTextModel';
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
import { ACCESSIBLE_NOTEBOOK_DISPLAY_ORDER, BUILTIN_RENDERER_ID, CellEditType, CellKind, CellOutputKind, DisplayOrderKey, ICellEditOperation, IDisplayOutput, INotebookDecorationRenderOptions, INotebookKernelInfo2, INotebookKernelProvider, INotebookRendererInfo, INotebookTextModel, IOrderedMimeType, ITransformedDisplayOutputDto, mimeTypeSupportedByCore, NotebookCellOutputsSplice, notebookDocumentFilterMatch, NotebookEditorPriority, NOTEBOOK_DISPLAY_ORDER, sortMimeTypes } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { ACCESSIBLE_NOTEBOOK_DISPLAY_ORDER, BUILTIN_RENDERER_ID, CellKind, CellOutputKind, DisplayOrderKey, IDisplayOutput, INotebookDecorationRenderOptions, INotebookKernelInfo2, INotebookKernelProvider, INotebookRendererInfo, INotebookTextModel, IOrderedMimeType, ITransformedDisplayOutputDto, mimeTypeSupportedByCore, notebookDocumentFilterMatch, NotebookEditorPriority, NOTEBOOK_DISPLAY_ORDER, sortMimeTypes } from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { NotebookOutputRendererInfo } from 'vs/workbench/contrib/notebook/common/notebookOutputRenderer';
import { NotebookEditorDescriptor, NotebookProviderInfo } from 'vs/workbench/contrib/notebook/common/notebookProvider';
import { IMainNotebookController, INotebookService } from 'vs/workbench/contrib/notebook/common/notebookService';
Expand Down Expand Up @@ -718,8 +718,6 @@ export class NotebookService extends Disposable implements INotebookService, ICu

this._models.set(uri, modelData);
this._onDidAddNotebookDocument.fire(notebookModel);
// after the document is added to the store and sent to ext host, we transform the ouputs
await this.transformTextModelOutputs(notebookModel);

return modelData.model;
}
Expand All @@ -732,68 +730,12 @@ export class NotebookService extends Disposable implements INotebookService, ICu
return Iterable.map(this._models.values(), data => data.model);
}

private async transformTextModelOutputs(textModel: NotebookTextModel) {
for (let i = 0; i < textModel.cells.length; i++) {
const cell = textModel.cells[i];

cell.outputs.forEach((output) => {
if (output.outputKind === CellOutputKind.Rich) {
// TODO@rebornix no string[] casting
const ret = this._transformMimeTypes(output, output.outputId, textModel.metadata.displayOrder as string[] || []);
const orderedMimeTypes = ret.orderedMimeTypes!;
const pickedMimeTypeIndex = ret.pickedMimeTypeIndex!;
output.pickedMimeTypeIndex = pickedMimeTypeIndex;
output.orderedMimeTypes = orderedMimeTypes;
}
});
}
}

transformEditsOutputs(textModel: NotebookTextModel, edits: ICellEditOperation[]) {
edits.forEach((edit) => {
if (edit.editType === CellEditType.Replace) {
edit.cells.forEach((cell) => {
const outputs = cell.outputs;
outputs.map((output) => {
if (output.outputKind === CellOutputKind.Rich) {
const ret = this._transformMimeTypes(output, output.outputId, textModel.metadata.displayOrder as string[] || []);
const orderedMimeTypes = ret.orderedMimeTypes!;
const pickedMimeTypeIndex = ret.pickedMimeTypeIndex!;
output.pickedMimeTypeIndex = pickedMimeTypeIndex;
output.orderedMimeTypes = orderedMimeTypes;
}
});
});
} else if (edit.editType === CellEditType.Output) {
edit.outputs.map((output) => {
if (output.outputKind === CellOutputKind.Rich) {
const ret = this._transformMimeTypes(output, output.outputId, textModel.metadata.displayOrder as string[] || []);
const orderedMimeTypes = ret.orderedMimeTypes!;
const pickedMimeTypeIndex = ret.pickedMimeTypeIndex!;
output.pickedMimeTypeIndex = pickedMimeTypeIndex;
output.orderedMimeTypes = orderedMimeTypes;
}
});
}
});
getMimeTypeInfo(textModel: NotebookTextModel, output: ITransformedDisplayOutputDto): readonly IOrderedMimeType[] {
// TODO@rebornix no string[] casting
return this._getOrderedMimeTypes(output, textModel.metadata.displayOrder as string[] ?? []);
}

transformSpliceOutputs(textModel: NotebookTextModel, splices: NotebookCellOutputsSplice[]) {
splices.forEach((splice) => {
const outputs = splice[2];
outputs.map((output) => {
if (output.outputKind === CellOutputKind.Rich) {
const ret = this._transformMimeTypes(output, output.outputId, textModel.metadata.displayOrder as string[] || []);
const orderedMimeTypes = ret.orderedMimeTypes!;
const pickedMimeTypeIndex = ret.pickedMimeTypeIndex!;
output.pickedMimeTypeIndex = pickedMimeTypeIndex;
output.orderedMimeTypes = orderedMimeTypes;
}
});
});
}

private _transformMimeTypes(output: IDisplayOutput, outputId: string, documentDisplayOrder: string[]): ITransformedDisplayOutputDto {
private _getOrderedMimeTypes(output: IDisplayOutput, documentDisplayOrder: string[]): IOrderedMimeType[] {
const mimeTypes = Object.keys(output.data);
const coreDisplayOrder = this._displayOrder;
const sorted = sortMimeTypes(mimeTypes, coreDisplayOrder?.userOrder || [], documentDisplayOrder, coreDisplayOrder?.defaultOrder || []);
Expand Down Expand Up @@ -832,13 +774,7 @@ export class NotebookService extends Disposable implements INotebookService, ICu
}
});

return {
outputKind: output.outputKind,
outputId,
data: output.data,
orderedMimeTypes: orderMimeTypes,
pickedMimeTypeIndex: 0
};
return orderMimeTypes;
}

private _findBestMatchedRenderer(mimeType: string): readonly NotebookOutputRendererInfo[] {
Expand Down
26 changes: 19 additions & 7 deletions src/vs/workbench/contrib/notebook/browser/view/renderers/codeCell.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ interface IMimeTypeRenderer extends IQuickPickItem {
}

class OutputElement extends Disposable {

// this isn't super proper but I couldn't find a view-model equivalent for output
// and it seems as of today we use the domain model - and pragamtically enrich it
// with UX properties
private pickedMimeTypes = new WeakMap<ITransformedDisplayOutputDto, number>();

readonly resizeListener = new DisposableStore();
domNode!: HTMLElement;
renderResult?: IRenderOutput;
Expand All @@ -60,12 +66,15 @@ class OutputElement extends Disposable {
if (this.output.outputKind === CellOutputKind.Rich) {
const transformedDisplayOutput = this.output as ITransformedDisplayOutputDto;

if (transformedDisplayOutput.orderedMimeTypes!.length > 1) {
const mimeTypes = this.notebookService.getMimeTypeInfo(this.notebookEditor.textModel!, this.output);
const pick = this.pickedMimeTypes.get(this.output) ?? 0;

if (mimeTypes.length > 1) {
outputItemDiv.style.position = 'relative';
const mimeTypePicker = DOM.$('.multi-mimetype-output');
mimeTypePicker.classList.add('codicon', 'codicon-code');
mimeTypePicker.tabIndex = 0;
mimeTypePicker.title = nls.localize('mimeTypePicker', "Choose a different output mimetype, available mimetypes: {0}", transformedDisplayOutput.orderedMimeTypes!.map(mimeType => mimeType.mimeType).join(', '));
mimeTypePicker.title = nls.localize('mimeTypePicker', "Choose a different output mimetype, available mimetypes: {0}", mimeTypes.map(mimeType => mimeType.mimeType).join(', '));
outputItemDiv.appendChild(mimeTypePicker);
this.resizeListener.add(DOM.addStandardDisposableListener(mimeTypePicker, 'mousedown', async e => {
if (e.leftButton) {
Expand All @@ -85,7 +94,7 @@ class OutputElement extends Disposable {
})));

}
const pickedMimeTypeRenderer = this.output.orderedMimeTypes![this.output.pickedMimeTypeIndex!];
const pickedMimeTypeRenderer = mimeTypes[pick];

const innerContainer = DOM.$('.output-inner-container');
DOM.append(outputItemDiv, innerContainer);
Expand Down Expand Up @@ -167,8 +176,12 @@ class OutputElement extends Disposable {
}

async pickActiveMimeTypeRenderer(output: ITransformedDisplayOutputDto) {
const currIndex = output.pickedMimeTypeIndex;
const items = output.orderedMimeTypes!.map((mimeType, index): IMimeTypeRenderer => ({

const mimeTypes = this.notebookService.getMimeTypeInfo(this.notebookEditor.textModel!, output);
const currIndex = this.pickedMimeTypes.get(output) ?? 0;

// const currIndex = output.pickedMimeTypeIndex;
const items = mimeTypes.map((mimeType, index): IMimeTypeRenderer => ({
label: mimeType.mimeType,
id: mimeType.mimeType,
index: index,
Expand Down Expand Up @@ -205,7 +218,7 @@ class OutputElement extends Disposable {
this.notebookEditor.removeInset(output);
}

output.pickedMimeTypeIndex = pick;
this.pickedMimeTypes.set(output, pick);
this.render(index, nextElement as HTMLElement);
this.relayoutCell();
}
Expand Down Expand Up @@ -769,4 +782,3 @@ export class CodeCell extends Disposable {
super.dispose();
}
}

15 changes: 1 addition & 14 deletions src/vs/workbench/contrib/notebook/common/notebookCommon.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -184,27 +184,14 @@ export interface IOrderedMimeType {
rendererId: string;
}

export interface ITransformedDisplayOutputDto {
outputKind: CellOutputKind.Rich;
export interface ITransformedDisplayOutputDto extends IDisplayOutput {
outputId: string;
data: { [key: string]: unknown; }
metadata?: NotebookCellOutputMetadata;

orderedMimeTypes?: IOrderedMimeType[];
pickedMimeTypeIndex?: number;
}

export function isTransformedDisplayOutput(thing: unknown): thing is ITransformedDisplayOutputDto {
return (thing as ITransformedDisplayOutputDto).outputKind === CellOutputKind.Rich && !!(thing as ITransformedDisplayOutputDto).outputId;
}

export interface IGenericOutput {
outputKind: CellOutputKind;
pickedMimeType?: string;
pickedRenderer?: number;
transformedOutput?: { [key: string]: IDisplayOutput };
}


export const addIdToOutput = (output: IRawOutput, id = UUID.generateUuid()): IProcessedOutput => output.outputKind === CellOutputKind.Rich
? ({ ...output, outputId: id }) : output;
Expand Down
6 changes: 3 additions & 3 deletions src/vs/workbench/contrib/notebook/common/notebookService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { NotebookExtensionDescription } from 'vs/workbench/api/common/extHost.pr
import { Event } from 'vs/base/common/event';
import {
INotebookTextModel, INotebookRendererInfo,
IEditor, ICellEditOperation, NotebookCellOutputsSplice, INotebookKernelProvider, INotebookKernelInfo2, TransientMetadata, NotebookDataDto, TransientOptions, INotebookDecorationRenderOptions, INotebookExclusiveDocumentFilter
IEditor, INotebookKernelProvider, INotebookKernelInfo2, TransientMetadata, NotebookDataDto, TransientOptions, INotebookDecorationRenderOptions, INotebookExclusiveDocumentFilter, IOrderedMimeType, ITransformedDisplayOutputDto
} from 'vs/workbench/contrib/notebook/common/notebookCommon';
import { NotebookTextModel } from 'vs/workbench/contrib/notebook/common/model/notebookTextModel';
import { CancellationToken } from 'vs/base/common/cancellation';
Expand Down Expand Up @@ -49,8 +49,8 @@ export interface INotebookService {
onDidChangeNotebookActiveKernel: Event<{ uri: URI, providerHandle: number | undefined, kernelId: string | undefined }>;
registerNotebookController(viewType: string, extensionData: NotebookExtensionDescription, controller: IMainNotebookController): IDisposable;

transformEditsOutputs(textModel: NotebookTextModel, edits: ICellEditOperation[]): void;
transformSpliceOutputs(textModel: NotebookTextModel, splices: NotebookCellOutputsSplice[]): void;
getMimeTypeInfo(textModel: NotebookTextModel, output: ITransformedDisplayOutputDto): readonly IOrderedMimeType[];

registerNotebookKernelProvider(provider: INotebookKernelProvider): IDisposable;
getContributedNotebookKernels(viewType: string, resource: URI, token: CancellationToken): Promise<INotebookKernelInfo2[]>;
getContributedNotebookOutputRenderers(id: string): NotebookOutputRendererInfo | undefined;
Expand Down