Cell attachment cleanup tool improvement in diff editor (#161132)

* Move attachment out of custom metadata, prep for attachment clean up in diff editor

* recover attachments from dirty notebook document

* Allow metadata to be restored when content changed/reverted in nb diff editor
This commit is contained in:
Peng Lyu 2022-09-16 14:12:53 -07:00 committed by GitHub
parent 2d7655cb16
commit 6e8bc02be4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 166 additions and 47 deletions

View file

@ -22,7 +22,7 @@ export async function activate(ctx: RendererContext<void>) {
md.renderer.rules.image = (tokens: MarkdownItToken[], idx: number, options, env, self) => {
const token = tokens[idx];
const src = token.attrGet('src');
const attachments: Record<string, Record<string, string>> = env.outputItem.metadata.custom?.attachments;
const attachments: Record<string, Record<string, string>> = env.outputItem.metadata.attachments;
if (attachments && src) {
const imageAttachment = attachments[src.replace('attachment:', '')];
if (imageAttachment) {

View file

@ -9,7 +9,8 @@
"vscode": "^1.57.0"
},
"enabledApiProposals": [
"documentPaste"
"documentPaste",
"diffContentOptions"
],
"activationEvents": [
"*"

View file

@ -149,21 +149,29 @@ function convertJupyterOutputToBuffer(mime: string, value: unknown): NotebookCel
}
}
function getNotebookCellMetadata(cell: nbformat.IBaseCell): CellMetadata {
function getNotebookCellMetadata(cell: nbformat.IBaseCell): {
[key: string]: any;
} {
const cellMetadata: { [key: string]: any } = {};
// We put this only for VSC to display in diff view.
// Else we don't use this.
const propertiesToClone: (keyof CellMetadata)[] = ['metadata', 'attachments'];
const custom: CellMetadata = {};
propertiesToClone.forEach((propertyToClone) => {
if (cell[propertyToClone]) {
custom[propertyToClone] = JSON.parse(JSON.stringify(cell[propertyToClone]));
}
});
if (cell['metadata']) {
custom['metadata'] = JSON.parse(JSON.stringify(cell['metadata']));
}
if ('id' in cell && typeof cell.id === 'string') {
custom.id = cell.id;
}
return custom;
cellMetadata.custom = custom;
if (cell['attachments']) {
cellMetadata.attachments = JSON.parse(JSON.stringify(cell['attachments']));
}
return cellMetadata;
}
function getOutputMetadata(output: nbformat.IOutput): CellOutputMetadata {
// Add on transient data if we have any. This should be removed by our save functions elsewhere.
const metadata: CellOutputMetadata = {
@ -284,7 +292,7 @@ export function jupyterCellOutputToCellOutput(output: nbformat.IOutput): Noteboo
function createNotebookCellDataFromRawCell(cell: nbformat.IRawCell): NotebookCellData {
const cellData = new NotebookCellData(NotebookCellKind.Code, concatMultilineString(cell.source), 'raw');
cellData.outputs = [];
cellData.metadata = { custom: getNotebookCellMetadata(cell) };
cellData.metadata = getNotebookCellMetadata(cell);
return cellData;
}
function createNotebookCellDataFromMarkdownCell(cell: nbformat.IMarkdownCell): NotebookCellData {
@ -294,7 +302,7 @@ function createNotebookCellDataFromMarkdownCell(cell: nbformat.IMarkdownCell): N
'markdown'
);
cellData.outputs = [];
cellData.metadata = { custom: getNotebookCellMetadata(cell) };
cellData.metadata = getNotebookCellMetadata(cell);
return cellData;
}
function createNotebookCellDataFromCodeCell(cell: nbformat.ICodeCell, cellLanguage: string): NotebookCellData {
@ -313,7 +321,7 @@ function createNotebookCellDataFromCodeCell(cell: nbformat.ICodeCell, cellLangua
const cellData = new NotebookCellData(NotebookCellKind.Code, source, cellLanguageId);
cellData.outputs = outputs;
cellData.metadata = { custom: getNotebookCellMetadata(cell) };
cellData.metadata = getNotebookCellMetadata(cell);
cellData.executionSummary = executionSummary;
return cellData;
}

View file

@ -35,9 +35,13 @@ export function activate(context: vscode.ExtensionContext) {
transientOutputs: false,
transientCellMetadata: {
breakpointMargin: true,
custom: false
custom: false,
attachments: false
},
cellContentMetadata: {
attachments: true
}
}));
} as vscode.NotebookDocumentContentOptions));
vscode.languages.registerCodeLensProvider({ pattern: '**/*.ipynb' }, {
provideCodeLenses: (document) => {

View file

@ -156,13 +156,13 @@ export class AttachmentCleaner implements vscode.CodeActionProvider {
if (this.checkMetadataAttachmentsExistence(cell.metadata)) {
// the cell metadata contains attachments, check if any are used in the markdown source
for (const currFilename of Object.keys(cell.metadata.custom.attachments)) {
for (const currFilename of Object.keys(cell.metadata.attachments)) {
// means markdown reference is present in the metadata, rendering will work properly
// therefore, we don't need to check it in the next loop either
if (markdownAttachmentsRefedInCell.has(currFilename)) {
// attachment reference is present in the markdown source, no need to cache it
markdownAttachmentsRefedInCell.get(currFilename)!.valid = true;
markdownAttachmentsInUse[currFilename] = cell.metadata.custom.attachments[currFilename];
markdownAttachmentsInUse[currFilename] = cell.metadata.attachments[currFilename];
} else {
// attachment reference is not present in the markdown source, cache it
this.saveAttachmentToCache(notebookUri, cellFragment, currFilename, cell.metadata);
@ -187,9 +187,9 @@ export class AttachmentCleaner implements vscode.CodeActionProvider {
}
}
if (!objectEquals(markdownAttachmentsInUse, cell.metadata.custom.attachments)) {
if (!objectEquals(markdownAttachmentsInUse, cell.metadata.attachments)) {
const updateMetadata: { [key: string]: any } = deepClone(cell.metadata);
updateMetadata.custom.attachments = markdownAttachmentsInUse;
updateMetadata.attachments = markdownAttachmentsInUse;
const metadataEdit = vscode.NotebookEdit.updateCellMetadata(cell.index, updateMetadata);
const workspaceEdit = new vscode.WorkspaceEdit();
workspaceEdit.set(e.notebook.uri, [metadataEdit]);
@ -229,7 +229,7 @@ export class AttachmentCleaner implements vscode.CodeActionProvider {
const markdownAttachments = this.getAttachmentNames(document);
if (this.checkMetadataAttachmentsExistence(activeCell.metadata)) {
for (const [currFilename, attachment] of markdownAttachments) {
if (!activeCell.metadata.custom.attachments[currFilename]) {
if (!activeCell.metadata.attachments[currFilename]) {
// no attachment reference in the metadata
diagnostics.push({ name: currFilename, ranges: attachment.ranges });
}
@ -287,7 +287,7 @@ export class AttachmentCleaner implements vscode.CodeActionProvider {
* @returns
*/
private getMetadataAttachment(metadata: { [key: string]: any }, currFilename: string): { [key: string]: any } {
return metadata.custom.attachments[currFilename];
return metadata.attachments[currFilename];
}
/**
@ -296,7 +296,7 @@ export class AttachmentCleaner implements vscode.CodeActionProvider {
* @returns boolean representing the presence of any attachments
*/
private checkMetadataAttachmentsExistence(metadata: { [key: string]: any }): boolean {
return !!(metadata.custom?.attachments);
return !!(metadata.attachments);
}
/**
@ -311,8 +311,8 @@ export class AttachmentCleaner implements vscode.CodeActionProvider {
const cellCache = documentCache.get(cellFragment) ?? new Map<string, IAttachmentData>();
documentCache.set(cellFragment, cellCache);
for (const currFilename of Object.keys(metadata.custom.attachments)) {
cellCache.set(currFilename, metadata.custom.attachments[currFilename]);
for (const currFilename of Object.keys(metadata.attachments)) {
cellCache.set(currFilename, metadata.attachments[currFilename]);
}
}

View file

@ -51,7 +51,7 @@ class CopyPasteEditProvider implements vscode.DocumentPasteEditProvider {
// create updated metadata for cell (prep for WorkspaceEdit)
const b64string = encodeBase64(fileDataAsUint8);
const startingAttachments = currentCell.metadata.custom?.attachments;
const startingAttachments = currentCell.metadata.attachments;
const newAttachment = buildAttachment(b64string, currentCell, filename, filetype, startingAttachments);
// build edits
@ -124,13 +124,11 @@ function encodeBase64(buffer: Uint8Array, padded = true, urlSafe = false) {
}
function buildAttachment(b64: string, cell: vscode.NotebookCell, filename: string, filetype: string, startingAttachments: any): { metadata: { [key: string]: any }; filename: string } {
const outputMetadata = { ...cell.metadata };
const cellMetadata = { ...cell.metadata };
let tempFilename = filename + filetype;
if (!outputMetadata.custom) {
outputMetadata['custom'] = { 'attachments': { [tempFilename]: { 'image/png': b64 } } };
} else if (!outputMetadata.custom.attachments) {
outputMetadata.custom['attachments'] = { [tempFilename]: { 'image/png': b64 } };
if (!cellMetadata.attachments) {
cellMetadata['attachments'] = { [tempFilename]: { 'image/png': b64 } };
} else {
for (let appendValue = 2; tempFilename in startingAttachments; appendValue++) {
const objEntries = Object.entries(startingAttachments[tempFilename]);
@ -143,10 +141,11 @@ function buildAttachment(b64: string, cell: vscode.NotebookCell, filename: strin
}
}
}
outputMetadata.custom.attachments[tempFilename] = { 'image/png': b64 };
cellMetadata.attachments[tempFilename] = { 'image/png': b64 };
}
return {
metadata: outputMetadata,
metadata: cellMetadata,
filename: tempFilename
};
}

View file

@ -5,7 +5,7 @@
import type * as nbformat from '@jupyterlab/nbformat';
import { NotebookCell, NotebookCellData, NotebookCellKind, NotebookCellOutput } from 'vscode';
import { CellMetadata, CellOutputMetadata } from './common';
import { CellOutputMetadata } from './common';
import { textMimeTypes } from './deserializers';
import { compressOutputItemStreams } from './streamCompressor';
@ -56,8 +56,14 @@ export function sortObjectPropertiesRecursively(obj: any): any {
}
export function getCellMetadata(cell: NotebookCell | NotebookCellData) {
return cell.metadata?.custom as CellMetadata | undefined;
return {
// it contains the cell id, and the cell metadata, along with other nb cell metadata
...(cell.metadata?.custom ?? {}),
// promote the cell attachments to the top level
attachments: cell.metadata?.custom?.attachments ?? cell.metadata?.attachments
};
}
function createCodeCellFromNotebookCell(cell: NotebookCellData, preferredLanguage: string | undefined): nbformat.ICodeCell {
const cellMetadata = getCellMetadata(cell);
let metadata = cellMetadata?.metadata || {}; // This cannot be empty.
@ -332,7 +338,7 @@ function convertOutputMimeToJupyterOutput(mime: string, value: Uint8Array) {
}
}
function createMarkdownCellFromNotebookCell(cell: NotebookCellData): nbformat.IMarkdownCell {
export function createMarkdownCellFromNotebookCell(cell: NotebookCellData): nbformat.IMarkdownCell {
const cellMetadata = getCellMetadata(cell);
const markdownCell: any = {
cell_type: 'markdown',

View file

@ -7,6 +7,7 @@ import type * as nbformat from '@jupyterlab/nbformat';
import * as assert from 'assert';
import * as vscode from 'vscode';
import { jupyterCellOutputToCellOutput, jupyterNotebookModelToNotebookData } from '../deserializers';
import { createMarkdownCellFromNotebookCell, getCellMetadata } from '../serializers';
function deepStripProperties(obj: any, props: string[]) {
for (const prop in obj) {
@ -52,6 +53,71 @@ suite('ipynb serializer', () => {
assert.deepStrictEqual(notebook.cells, [expectedCodeCell, expectedMarkdownCell]);
});
test('Serialize', async () => {
const markdownCell = new vscode.NotebookCellData(vscode.NotebookCellKind.Markup, '# header1', 'markdown');
markdownCell.metadata = {
attachments: {
'image.png': {
'image/png': 'abc'
}
},
custom: {
id: '123',
metadata: {
foo: 'bar'
}
}
};
const cellMetadata = getCellMetadata(markdownCell);
assert.deepStrictEqual(cellMetadata, {
id: '123',
metadata: {
foo: 'bar',
},
attachments: {
'image.png': {
'image/png': 'abc'
}
}
});
const markdownCell2 = new vscode.NotebookCellData(vscode.NotebookCellKind.Markup, '# header1', 'markdown');
markdownCell2.metadata = {
custom: {
id: '123',
metadata: {
foo: 'bar'
},
attachments: {
'image.png': {
'image/png': 'abc'
}
}
}
};
const nbMarkdownCell = createMarkdownCellFromNotebookCell(markdownCell);
const nbMarkdownCell2 = createMarkdownCellFromNotebookCell(markdownCell2);
assert.deepStrictEqual(nbMarkdownCell, nbMarkdownCell2);
assert.deepStrictEqual(nbMarkdownCell, {
cell_type: 'markdown',
source: ['# header1'],
metadata: {
foo: 'bar',
},
attachments: {
'image.png': {
'image/png': 'abc'
}
},
id: '123'
});
});
suite('Outputs', () => {
function validateCellOutputTranslation(
outputs: nbformat.IOutput[],

View file

@ -1709,7 +1709,8 @@ export namespace NotebookDocumentContentOptions {
return {
transientOutputs: options?.transientOutputs ?? false,
transientCellMetadata: options?.transientCellMetadata ?? {},
transientDocumentMetadata: options?.transientDocumentMetadata ?? {}
transientDocumentMetadata: options?.transientDocumentMetadata ?? {},
cellContentMetadata: options?.cellContentMetadata ?? {}
};
}
}

View file

@ -87,7 +87,8 @@ export class InteractiveDocumentContribution extends Disposable implements IWork
const contentOptions = {
transientOutputs: true,
transientCellMetadata: {},
transientDocumentMetadata: {}
transientDocumentMetadata: {},
cellContentMetadata: {}
};
const controller: INotebookContentProvider = {

View file

@ -388,6 +388,19 @@ export class SideBySideDiffElementViewModel extends DiffElementViewModelBase {
this._register(this.modified.onDidChangeOutputLayout(() => {
this._layout({ recomputeOutput: true });
}));
this._register(this.modified.textModel.onDidChangeContent(() => {
if (mainDocumentTextModel.transientOptions.cellContentMetadata) {
const cellMetadataKeys = [...Object.keys(mainDocumentTextModel.transientOptions.cellContentMetadata)];
const modifiedMedataRaw = Object.assign({}, this.modified.metadata);
const originalCellMetadata = this.original.metadata;
for (const key of cellMetadataKeys) {
modifiedMedataRaw[key] = originalCellMetadata[key];
}
this.modified.textModel.metadata = modifiedMedataRaw;
}
}));
}
checkIfOutputsModified() {

View file

@ -173,7 +173,7 @@ export class NotebookTextModel extends Disposable implements INotebookTextModel
private _defaultCollapseConfig: NotebookCellDefaultCollapseConfig | undefined;
metadata: NotebookDocumentMetadata = {};
transientOptions: TransientOptions = { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false };
transientOptions: TransientOptions = { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} };
private _versionId = 0;
/**

View file

@ -124,12 +124,14 @@ export interface NotebookCellDefaultCollapseConfig {
export type InteractiveWindowCollapseCodeCells = 'always' | 'never' | 'fromEditor';
export type TransientCellMetadata = { [K in keyof NotebookCellMetadata]?: boolean };
export type CellContentMetadata = { [K in keyof NotebookCellMetadata]?: boolean };
export type TransientDocumentMetadata = { [K in keyof NotebookDocumentMetadata]?: boolean };
export interface TransientOptions {
transientOutputs: boolean;
transientCellMetadata: TransientCellMetadata;
transientDocumentMetadata: TransientDocumentMetadata;
cellContentMetadata: CellContentMetadata;
}
/** Note: enum values are used for sorting */

View file

@ -54,7 +54,8 @@ export class NotebookProviderInfo {
this._options = {
transientCellMetadata: {},
transientDocumentMetadata: {},
transientOutputs: false
transientOutputs: false,
cellContentMetadata: {}
};
}

View file

@ -54,7 +54,7 @@ suite('NotebookFileWorkingCopyModel', function () {
const model = new NotebookFileWorkingCopyModel(
notebook,
new class extends mock<INotebookSerializer>() {
override options: TransientOptions = { transientOutputs: true, transientCellMetadata: {}, transientDocumentMetadata: {} };
override options: TransientOptions = { transientOutputs: true, transientCellMetadata: {}, transientDocumentMetadata: {}, cellContentMetadata: {} };
override async notebookToData(notebook: NotebookData) {
callCount += 1;
assert.strictEqual(notebook.cells.length, 1);
@ -73,7 +73,7 @@ suite('NotebookFileWorkingCopyModel', function () {
const model = new NotebookFileWorkingCopyModel(
notebook,
new class extends mock<INotebookSerializer>() {
override options: TransientOptions = { transientOutputs: false, transientCellMetadata: {}, transientDocumentMetadata: {} };
override options: TransientOptions = { transientOutputs: false, transientCellMetadata: {}, transientDocumentMetadata: {}, cellContentMetadata: {} };
override async notebookToData(notebook: NotebookData) {
callCount += 1;
assert.strictEqual(notebook.cells.length, 1);
@ -102,7 +102,7 @@ suite('NotebookFileWorkingCopyModel', function () {
const model = new NotebookFileWorkingCopyModel(
notebook,
new class extends mock<INotebookSerializer>() {
override options: TransientOptions = { transientOutputs: true, transientCellMetadata: {}, transientDocumentMetadata: { bar: true } };
override options: TransientOptions = { transientOutputs: true, transientCellMetadata: {}, transientDocumentMetadata: { bar: true }, cellContentMetadata: {} };
override async notebookToData(notebook: NotebookData) {
callCount += 1;
assert.strictEqual(notebook.metadata.foo, 123);
@ -121,7 +121,7 @@ suite('NotebookFileWorkingCopyModel', function () {
const model = new NotebookFileWorkingCopyModel(
notebook,
new class extends mock<INotebookSerializer>() {
override options: TransientOptions = { transientOutputs: false, transientCellMetadata: {}, transientDocumentMetadata: {} };
override options: TransientOptions = { transientOutputs: false, transientCellMetadata: {}, transientDocumentMetadata: {}, cellContentMetadata: {} };
override async notebookToData(notebook: NotebookData) {
callCount += 1;
assert.strictEqual(notebook.metadata.foo, 123);
@ -150,7 +150,7 @@ suite('NotebookFileWorkingCopyModel', function () {
const model = new NotebookFileWorkingCopyModel(
notebook,
new class extends mock<INotebookSerializer>() {
override options: TransientOptions = { transientOutputs: true, transientDocumentMetadata: {}, transientCellMetadata: { bar: true } };
override options: TransientOptions = { transientOutputs: true, transientDocumentMetadata: {}, transientCellMetadata: { bar: true }, cellContentMetadata: {} };
override async notebookToData(notebook: NotebookData) {
callCount += 1;
assert.strictEqual(notebook.cells[0].metadata!.foo, 123);
@ -169,7 +169,7 @@ suite('NotebookFileWorkingCopyModel', function () {
const model = new NotebookFileWorkingCopyModel(
notebook,
new class extends mock<INotebookSerializer>() {
override options: TransientOptions = { transientOutputs: false, transientCellMetadata: {}, transientDocumentMetadata: {} };
override options: TransientOptions = { transientOutputs: false, transientCellMetadata: {}, transientDocumentMetadata: {}, cellContentMetadata: {} };
override async notebookToData(notebook: NotebookData) {
callCount += 1;
assert.strictEqual(notebook.cells[0].metadata!.foo, 123);

View file

@ -54,7 +54,7 @@ suite('NotebookViewModel', () => {
suiteTeardown(() => disposables.dispose());
test('ctor', function () {
const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }, undoRedoService, modelService, languageService);
const notebook = new NotebookTextModel('notebook', URI.parse('test'), [], {}, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} }, undoRedoService, modelService, languageService);
const model = new NotebookEditorTestModel(notebook);
const viewContext = new ViewContext(new NotebookOptions(instantiationService.get(IConfigurationService), instantiationService.get(INotebookExecutionStateService)), new NotebookEventDispatcher(), () => ({} as IBaseCellEditorOptions));
const viewModel = new NotebookViewModel('notebook', model.notebook, viewContext, null, { isReadOnly: false }, instantiationService, bulkEditService, undoRedoService, textModelService);

View file

@ -67,7 +67,7 @@ export class TestCell extends NotebookCellTextModel {
outputs: IOutputDto[],
languageService: ILanguageService,
) {
super(CellUri.generate(URI.parse('test:///fake/notebook'), handle), handle, source, language, Mimes.text, cellKind, outputs, undefined, undefined, undefined, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false }, languageService);
super(CellUri.generate(URI.parse('test:///fake/notebook'), handle), handle, source, language, Mimes.text, cellKind, outputs, undefined, undefined, undefined, { transientCellMetadata: {}, transientDocumentMetadata: {}, transientOutputs: false, cellContentMetadata: {} }, languageService);
}
}

View file

@ -21,6 +21,7 @@ export const allApiProposals = Object.freeze({
contribViewsWelcome: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.contribViewsWelcome.d.ts',
customEditorMove: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.customEditorMove.d.ts',
diffCommand: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.diffCommand.d.ts',
diffContentOptions: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.diffContentOptions.d.ts',
documentFiltersExclusive: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.documentFiltersExclusive.d.ts',
documentPaste: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.documentPaste.d.ts',
editSessionIdentityProvider: 'https://raw.githubusercontent.com/microsoft/vscode/main/src/vscode-dts/vscode.proposed.editSessionIdentityProvider.d.ts',

View file

@ -0,0 +1,16 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
declare module 'vscode' {
// TODO@rebornix: add github issue link
export interface NotebookDocumentContentOptions {
/**
* Controls if a cell metadata property should be reverted when the cell content
* is reverted in notebook diff editor.
*/
cellContentMetadata?: { [key: string]: boolean | undefined };
}
}