diff --git a/src/renderer/src/components/editor/IpynbCellEditor.external-sync.test.tsx b/src/renderer/src/components/editor/IpynbCellEditor.external-sync.test.tsx index 777e452ee1e..699d1b878ed 100644 --- a/src/renderer/src/components/editor/IpynbCellEditor.external-sync.test.tsx +++ b/src/renderer/src/components/editor/IpynbCellEditor.external-sync.test.tsx @@ -82,7 +82,7 @@ vi.mock('@/lib/monaco-setup', async () => { }) function notebookContent( - cells: { id: string; source: string; kind?: 'code' | 'markdown' }[], + cells: { id: string | null; source: string; kind?: 'code' | 'markdown' }[], language = 'python' ): string { return JSON.stringify({ @@ -90,7 +90,7 @@ function notebookContent( nbformat_minor: 5, metadata: { language_info: { name: language } }, cells: cells.map(({ id, source, kind }) => ({ - id, + ...(id === null ? {} : { id }), cell_type: kind ?? 'code', metadata: {}, execution_count: null, @@ -126,6 +126,7 @@ function NotebookCells({ return ( { + modelScope += 1 vi.useFakeTimers() vi.clearAllMocks() }) @@ -314,3 +318,118 @@ describe('active notebook cell external reload', () => { expect(onContentChange).not.toHaveBeenCalled() }) }) + +describe('notebook cell undo lifetime', () => { + const callbacks = () => ({ + onContentChange: vi.fn<(content: string) => void>(), + onDirtyStateHint: vi.fn<(dirty: boolean) => void>() + }) + + async function undo(model: editor.ITextModel): Promise { + await act(async () => { + await model.undo() + }) + } + + it('restores cell undo after switching cells and after the notebook view remounts', async () => { + const handlers = callbacks() + const content = notebookContent([ + { id: 'a', source: 'first source' }, + { id: 'b', source: 'second source' } + ]) + const firstView = render() + fireEvent.mouseDown(screen.getByRole('button', { name: 'first source' }), { button: 0 }) + appendText(activeModel(), '!') + fireEvent.mouseDown(screen.getByRole('button', { name: 'second source' }), { button: 0 }) + appendText(activeModel(), '?') + fireEvent.mouseDown(screen.getByRole('button', { name: 'first source!' }), { button: 0 }) + await undo(activeModel()) + expect(activeModel().getValue()).toBe('first source') + expect(screen.getByRole('button', { name: 'second source?' })).toBeDefined() + + const edited = handlers.onContentChange.mock.calls.at(-1)?.[0] ?? '' + firstView.unmount() + render() + fireEvent.mouseDown(screen.getByRole('button', { name: 'second source?' }), { button: 0 }) + await undo(activeModel()) + expect(activeModel().getValue()).toBe('second source') + }) + + it('drops cell undo when the source changed while the cell was inactive', async () => { + const handlers = callbacks() + const { rerender } = render( + + ) + fireEvent.mouseDown(screen.getByRole('button', { name: 'original' }), { button: 0 }) + appendText(activeModel(), '!') + fireEvent.mouseDown(screen.getByRole('button', { name: 'other' }), { button: 0 }) + rerender( + + ) + rerender( + + ) + fireEvent.mouseDown(screen.getByRole('button', { name: 'from disk' }), { button: 0 }) + await undo(activeModel()) + expect(activeModel().getValue()).toBe('from disk') + }) + + it('does not carry undo to another id-less cell that moves into the same position', async () => { + const handlers = callbacks() + const { rerender } = render( + + ) + fireEvent.mouseDown(screen.getByRole('button', { name: 'text' }), { button: 0 }) + appendText(activeModel(), '!') + fireEvent.mouseDown(screen.getByRole('button', { name: 'text!' }), { button: 0 }) + rerender( + + ) + rerender( + + ) + fireEvent.mouseDown(screen.getByRole('button', { name: 'text!' }), { button: 0 }) + handlers.onContentChange.mockClear() + await undo(activeModel()) + expect(activeModel().getValue()).toBe('text!') + expect(handlers.onContentChange).not.toHaveBeenCalled() + }) + + it('opens a cell whose id is duplicated in the notebook', () => { + render( + + ) + fireEvent.mouseDown(screen.getByRole('button', { name: 'first' }), { button: 0 }) + expect(liveModels.map((model) => model.getValue())).toEqual(['first', 'second']) + }) +}) diff --git a/src/renderer/src/components/editor/IpynbCellEditor.tsx b/src/renderer/src/components/editor/IpynbCellEditor.tsx index 8758ca37ad2..238bc4d5a8e 100644 --- a/src/renderer/src/components/editor/IpynbCellEditor.tsx +++ b/src/renderer/src/components/editor/IpynbCellEditor.tsx @@ -37,6 +37,8 @@ export function IpynbMarkdownCell({ source }: { source: string }): React.JSX.Ele } type IpynbCellSourceProps = { + /** Null for cells without a stable id, whose position-based key can move to another cell. */ + modelKey: string | null cell: IpynbCell source: string active: boolean @@ -140,6 +142,7 @@ function IpynbCodePreview({ } function IpynbSourceEditor({ + modelKey, cell, source, pressedAt, @@ -172,7 +175,16 @@ function IpynbSourceEditor({ return } const { lineHeight, paddingX, paddingY } = CODE_LAYOUT - const model = monaco.editor.createModel(source, cell.language) + // Monaco restores undo for disposed models with this scheme; no other Monaco feature special-cases it. + const uri = + modelKey === null + ? null + : monaco.Uri.from({ scheme: 'vscode-userdata', path: `/${modelKey}` }) + const model = monaco.editor.createModel( + source, + cell.language, + uri && !monaco.editor.getModel(uri) ? uri : undefined + ) const editorInstance = monaco.editor.create(container, { dropIntoEditor: { enabled: false }, model, diff --git a/src/renderer/src/components/editor/IpynbRendering.test.tsx b/src/renderer/src/components/editor/IpynbRendering.test.tsx index 12952e4709f..c7b1ba7aecf 100644 --- a/src/renderer/src/components/editor/IpynbRendering.test.tsx +++ b/src/renderer/src/components/editor/IpynbRendering.test.tsx @@ -24,6 +24,7 @@ function cell(kind: IpynbCell['kind'], source: string, outputs: IpynbOutput[] = function renderSource(target: IpynbCell, onActivate = vi.fn()) { render(