Skip to content

Commit 689c28e

Browse files
Fix copy/paste in notebooks (#11024)
Addresses #10240 This PR addresses the issue where copying a cell (`C`) and pasting it inside another cell's editor (`Cmd+V`) would paste the cell metadata instead of the cell contents. Additionally, this PR addres support for cross-notebook copy/paste. Cells can now be copy/pasted between different notebooks which was not previously supported. -- Positron notebooks has specific code to handle copy/pasting cell objects into notebooks. The system clipboard is used to handle pasting into editors (including cell editors). When users copied a cell (`C` key or "Copy cell" button), we were writing the cell json to the notebook clipboard and the system clipboard. This is why when a users copied a cell (`C` key or "Copy cell" button) and then pasted inside a cell editor (edit mode), they would get json instead of the text in the cell. A code cell with the contents: `print("Hello World")` would return the following: ``` { "cells": [ { "cell_type": "code", "source": [ "print(\"Hello World\")" ], "metadata": {}, "outputs": [], "execution_count": null } ], "metadata": { "kernelspec": {}, "language_info": {} }, "nbformat": 4, "nbformat_minor": 2 } ``` This happened because we were writing cell metadata to the system clipboard instead of the cell contents. We should have only been writing the cell contents to the system clipboard. ### Screenshots **copy/paste single cell into cell editor** https://github.com/user-attachments/assets/d9b869c1-e8d1-4a9d-a052-b257a5fb6ba2 **copy/paste multiple cells into cell editor** https://github.com/user-attachments/assets/e262a21b-206f-4c02-a179-56fc23b8d7f7 **copy/paste multiple cells into non-notebook editor** https://github.com/user-attachments/assets/70f9c22c-ca65-4c4c-898d-1afcefda66f3 **copy/paste cells into a different notebook** https://github.com/user-attachments/assets/64047f2e-1740-4282-a2a5-e4941dd155fd ### Release Notes <!-- Optionally, replace `N/A` with text to be included in the next release notes. The `N/A` bullets are ignored. If you refer to one or more Positron issues, these issues are used to collect information about the feature or bugfix, such as the relevant language pack as determined by Github labels of type `lang: `. The note will automatically be tagged with the language. These notes are typically filled by the Positron team. If you are an external contributor, you may ignore this section. --> #### New Features - N/A #### Bug Fixes - N/A ### QA Notes @:positron-notebooks - Copy cell and paste anywhere in a notebook -> pastes new cell with metadata - Copy cell and paste within a cell -> paste just the source text - Copy cell and paste into a non .ipynb file -> paste just the source text --------- Signed-off-by: Dhruvi Sompura <[email protected]> Co-authored-by: Nick Strayer <[email protected]>
1 parent 9dadae4 commit 689c28e

File tree

4 files changed

+69
-170
lines changed

4 files changed

+69
-170
lines changed

src/vs/workbench/contrib/positronNotebook/browser/PositronNotebookInstance.ts

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ import { IPositronWebviewPreloadService } from '../../../services/positronWebvie
3838
import { autorunDelta, observableFromEvent, observableValue, runOnChange } from '../../../../base/common/observable.js';
3939
import { ResourceMap } from '../../../../base/common/map.js';
4040
import { ICodeEditor } from '../../../../editor/browser/editorBrowser.js';
41-
import { cellToCellDto2, serializeCellsToClipboard } from './cellClipboardUtils.js';
41+
import { cellToCellDto2 } from './cellClipboardUtils.js';
4242
import { IClipboardService } from '../../../../platform/clipboard/common/clipboardService.js';
4343
import { IPositronConsoleService } from '../../../services/positronConsole/browser/interfaces/positronConsoleService.js';
4444
import { isNotebookLanguageRuntimeSession } from '../../../services/runtimeSession/common/runtimeSession.js';
@@ -1440,11 +1440,6 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
14401440
// =============================================================================================
14411441
// #region Clipboard Methods
14421442

1443-
/**
1444-
* Internal clipboard for storing cells with full fidelity
1445-
*/
1446-
private _clipboardCells: ICellDto2[] = [];
1447-
14481443
/**
14491444
* Copies the specified cells to the clipboard.
14501445
* @param cells The cells to copy. If not provided, copies the currently selected cells
@@ -1456,11 +1451,19 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
14561451
return;
14571452
}
14581453

1459-
// Store internally for full-fidelity paste
1460-
this._clipboardCells = cellsToCopy.map(cell => cellToCellDto2(cell));
1454+
const clipboardCells: ICellDto2[] = [];
1455+
let clipboardText = '';
1456+
cellsToCopy.forEach(cell => {
1457+
clipboardCells.push(cellToCellDto2(cell));
1458+
clipboardText += cell.getContent() + '\n\n';
1459+
});
1460+
1461+
// Store in shared notebook service clipboard for within-window paste (same or different notebook)
1462+
this._positronNotebookService.setClipboardCells(clipboardCells);
14611463

1462-
// Also write to system clipboard as text
1463-
const clipboardText = serializeCellsToClipboard(cellsToCopy);
1464+
// Remove trailing newlines from clipboard text
1465+
clipboardText = clipboardText.trimEnd();
1466+
// Write cell contents to system clipboard for pasting into other editors (including cell editors)
14641467
this._clipboardService.writeText(clipboardText);
14651468

14661469
// Log for debugging
@@ -1500,10 +1503,13 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
15001503
try {
15011504
this._assertTextModel();
15021505

1506+
// Get cells from shared notebook service clipboard
1507+
const cellsToPaste = this._positronNotebookService.getClipboardCells();
1508+
15031509
const textModel = this.textModel;
15041510
const computeUndoRedo = !this.isReadOnly || textModel.viewType === 'interactive';
15051511
const pasteIndex = index ?? this.getInsertionIndex();
1506-
const cellCount = this._clipboardCells.length;
1512+
const cellCount = cellsToPaste.length;
15071513

15081514
// Use textModel.applyEdits to properly create and register cells
15091515
const synchronous = true;
@@ -1522,7 +1528,7 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
15221528
editType: CellEditType.Replace,
15231529
index: pasteIndex,
15241530
count: 0,
1525-
cells: this._clipboardCells
1531+
cells: cellsToPaste
15261532
}
15271533
],
15281534
synchronous,
@@ -1560,7 +1566,7 @@ export class PositronNotebookInstance extends Disposable implements IPositronNot
15601566
* @returns True if cells can be pasted, false otherwise
15611567
*/
15621568
canPaste(): boolean {
1563-
return this._clipboardCells.length > 0;
1569+
return this._positronNotebookService.hasClipboardCells();
15641570
}
15651571

15661572
/**

src/vs/workbench/contrib/positronNotebook/browser/cellClipboardUtils.ts

Lines changed: 1 addition & 148 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,7 @@
44
*--------------------------------------------------------------------------------------------*/
55

66
import { ICellDto2 } from '../../notebook/common/notebookCommon.js';
7-
import { VSBuffer } from '../../../../base/common/buffer.js';
8-
import { CellKind, IPositronNotebookCell } from './PositronNotebookCells/IPositronNotebookCell.js';
7+
import { IPositronNotebookCell } from './PositronNotebookCells/IPositronNotebookCell.js';
98

109
/**
1110
* Converts a Positron notebook cell to ICellDto2 format for clipboard storage.
@@ -32,149 +31,3 @@ export function cellToCellDto2(cell: IPositronNotebookCell): ICellDto2 {
3231
};
3332
}
3433

35-
/**
36-
* Serializes cells to a JSON string for system clipboard storage.
37-
* This enables pasting cells into other applications or notebooks.
38-
*/
39-
export function serializeCellsToClipboard(cells: IPositronNotebookCell[]): string {
40-
const cellsData = cells.map(cell => {
41-
const cellModel = cell.model;
42-
43-
// Create a serializable representation of the cell
44-
const cellData = {
45-
cell_type: cellModel.cellKind === CellKind.Code ? 'code' : 'markdown',
46-
source: splitIntoLines(cell.getContent()),
47-
metadata: {},
48-
// For code cells, include outputs
49-
...(cellModel.cellKind === CellKind.Code ? {
50-
outputs: cellModel.outputs.map(output => ({
51-
output_type: 'display_data',
52-
data: output.outputs.reduce((acc, item) => {
53-
// Convert output items to a format compatible with Jupyter
54-
acc[item.mime] = item.data.toString();
55-
return acc;
56-
// eslint-disable-next-line local/code-no-dangerous-type-assertions
57-
}, {} as Record<string, string>),
58-
metadata: {}
59-
})),
60-
execution_count: null
61-
} : {})
62-
};
63-
64-
return cellData;
65-
});
66-
67-
// Wrap in a notebook-like structure for compatibility
68-
const notebookData = {
69-
cells: cellsData,
70-
metadata: {
71-
kernelspec: {},
72-
language_info: {}
73-
},
74-
nbformat: 4,
75-
nbformat_minor: 2
76-
};
77-
78-
return JSON.stringify(notebookData, null, 2);
79-
}
80-
81-
/**
82-
* Deserializes cells from a clipboard string to ICellDto2 format.
83-
* Handles both Positron and standard Jupyter notebook formats.
84-
*/
85-
export function deserializeCellsFromClipboard(clipboardData: string): ICellDto2[] | null {
86-
try {
87-
const data = JSON.parse(clipboardData);
88-
89-
// Check if it's a notebook format (has cells array)
90-
const cells = data.cells || (Array.isArray(data) ? data : null);
91-
92-
if (!cells) {
93-
return null;
94-
}
95-
96-
return cells.map((cellData: any) => {
97-
// Determine cell kind
98-
const cellKind = cellData.cell_type === 'code' ? CellKind.Code : CellKind.Markup;
99-
100-
// Handle source - could be array of strings or single string
101-
const source = Array.isArray(cellData.source)
102-
? cellData.source.join('')
103-
: (cellData.source || '');
104-
105-
// Handle outputs for code cells
106-
const outputs = [];
107-
if (cellKind === CellKind.Code && cellData.outputs) {
108-
for (const output of cellData.outputs) {
109-
const outputItems = [];
110-
111-
// Handle different output formats
112-
if (output.data) {
113-
for (const [mime, content] of Object.entries(output.data)) {
114-
outputItems.push({
115-
mime,
116-
data: VSBuffer.fromString(content as string)
117-
});
118-
}
119-
} else if (output.text) {
120-
// Handle stream outputs
121-
outputItems.push({
122-
mime: 'text/plain',
123-
data: VSBuffer.fromString(Array.isArray(output.text) ? output.text.join('') : output.text)
124-
});
125-
}
126-
127-
if (outputItems.length > 0) {
128-
outputs.push({
129-
outputId: output.output_id || crypto.randomUUID(),
130-
outputs: outputItems
131-
});
132-
}
133-
}
134-
}
135-
136-
// Return ICellDto2 format
137-
return {
138-
source,
139-
language: cellData.language || (cellKind === CellKind.Code ? 'python' : 'markdown'),
140-
mime: cellData.mime,
141-
cellKind,
142-
outputs,
143-
metadata: cellData.metadata || {},
144-
internalMetadata: {
145-
executionOrder: cellData.execution_count || undefined,
146-
lastRunSuccess: undefined,
147-
runStartTime: undefined,
148-
runEndTime: undefined
149-
},
150-
collapseState: undefined
151-
};
152-
});
153-
} catch (error) {
154-
// Failed to parse clipboard data
155-
return null;
156-
}
157-
}
158-
159-
/**
160-
* Helper function to split content into lines for Jupyter format
161-
*/
162-
function splitIntoLines(content: string): string[] {
163-
if (!content) {
164-
return [];
165-
}
166-
167-
// Split by newlines but preserve the newline characters
168-
const lines = content.split(/(\r?\n)/);
169-
const result: string[] = [];
170-
171-
for (let i = 0; i < lines.length; i += 2) {
172-
if (i + 1 < lines.length) {
173-
result.push(lines[i] + lines[i + 1]);
174-
} else if (lines[i]) {
175-
result.push(lines[i]);
176-
}
177-
}
178-
179-
return result;
180-
}

src/vs/workbench/contrib/positronNotebook/browser/positronNotebookService.ts

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { IPositronNotebookInstance } from './IPositronNotebookInstance.js';
1212
import { usingPositronNotebooks as utilUsingPositronNotebooks } from '../common/positronNotebookCommon.js';
1313
import { isEqual } from '../../../../base/common/resources.js';
1414
import { Emitter, Event } from '../../../../base/common/event.js';
15+
import { ICellDto2 } from '../../notebook/common/notebookCommon.js';
1516

1617
export const IPositronNotebookService = createDecorator<IPositronNotebookService>('positronNotebookService');
1718
export interface IPositronNotebookService {
@@ -58,6 +59,29 @@ export interface IPositronNotebookService {
5859
* @returns true if Positron notebooks are the default editor, false otherwise
5960
*/
6061
usingPositronNotebooks(): boolean;
62+
63+
/**
64+
* Stores cells in the shared clipboard.
65+
* @param cells The cells to store in the clipboard
66+
*/
67+
setClipboardCells(cells: ICellDto2[]): void;
68+
69+
/**
70+
* Retrieves cells from the shared clipboard.
71+
* @returns The cells currently stored in the clipboard, or empty array if none
72+
*/
73+
getClipboardCells(): ICellDto2[];
74+
75+
/**
76+
* Checks if there are cells available in the shared clipboard.
77+
* @returns True if cells are available, false otherwise
78+
*/
79+
hasClipboardCells(): boolean;
80+
81+
/**
82+
* Clears the shared clipboard.
83+
*/
84+
clearClipboard(): void;
6185
}
6286

6387
export class PositronNotebookService extends Disposable implements IPositronNotebookService {
@@ -75,6 +99,7 @@ export class PositronNotebookService extends Disposable implements IPositronNote
7599
//#region Private Properties
76100
private _instanceById = new Map<string, IPositronNotebookInstance>();
77101
private _activeInstance: IPositronNotebookInstance | null = null;
102+
private _clipboardCells: ICellDto2[] = [];
78103
//#endregion Private Properties
79104

80105
//#region Constructor & Dispose
@@ -122,6 +147,22 @@ export class PositronNotebookService extends Disposable implements IPositronNote
122147
public usingPositronNotebooks(): boolean {
123148
return utilUsingPositronNotebooks(this._configurationService);
124149
}
150+
151+
public setClipboardCells(cells: ICellDto2[]): void {
152+
this._clipboardCells = cells;
153+
}
154+
155+
public getClipboardCells(): ICellDto2[] {
156+
return this._clipboardCells;
157+
}
158+
159+
public hasClipboardCells(): boolean {
160+
return this._clipboardCells.length > 0;
161+
}
162+
163+
public clearClipboard(): void {
164+
this._clipboardCells = [];
165+
}
125166
//#endregion Public Methods
126167
}
127168

test/e2e/tests/notebooks-positron/notebook-cell-action-bar.test.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -140,16 +140,15 @@ test.describe('Positron Notebooks: Action Bar Behavior', {
140140
await notebooksPositron.triggerCellAction(0, 'Paste cell below');
141141
await notebooksPositron.expectCellContentsToBe(['# Cell 0', '# Cell 0', '# Cell 1']);
142142

143-
// ISSUE #10240: Pasting inside of cell includes metadata
144143
// Copy cell using action bar and paste into existing cell using keyboard
145-
// await notebooksPositron.selectCellAtIndex(0);
146-
// await notebooksPositron.expectCellIndexToBeSelected(0, { inEditMode: true });
147-
// await notebooksPositron.selectFromMoreActionsMenu(0, 'Copy cell');
148-
// await notebooksPositron.selectCellAtIndex(2);
149-
// await notebooksPositron.expectCellIndexToBeSelected(2, { inEditMode: true });
150-
// await hotKeys.selectAll();
151-
// await hotKeys.paste();
152-
// await notebooksPositron.expectCellContentsToBe(['# Cell 0', '# Cell 0', '# Cell 0']);
144+
await notebooksPositron.selectCellAtIndex(0);
145+
await notebooksPositron.expectCellIndexToBeSelected(0, { inEditMode: true });
146+
await notebooksPositron.triggerCellAction(0, 'Copy cell');
147+
await notebooksPositron.selectCellAtIndex(2);
148+
await notebooksPositron.expectCellIndexToBeSelected(2, { inEditMode: true });
149+
await hotKeys.selectAll();
150+
await hotKeys.paste();
151+
await notebooksPositron.expectCellContentsToBe(['# Cell 0', '# Cell 0', '# Cell 0']);
153152
});
154153

155154
test('Cell actions with multiple cells selected', async function ({ app }) {

0 commit comments

Comments
 (0)