diff --git a/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.test.tsx b/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.test.tsx index fde6c5e388b..5a0873ab9a5 100644 --- a/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.test.tsx +++ b/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.test.tsx @@ -20,7 +20,9 @@ vi.mock('@/runtime/runtime-paged-file-read', () => ({ const store = vi.hoisted(() => ({ markFileDirty: vi.fn(), openFiles: [{ id: 'book', isDirty: false }], - listeners: new Set<(state: unknown) => void>() + editorBufferIdOf: {}, + editorBufferViews: {}, + listeners: new Set<(state: unknown, previous: unknown) => void>() })) vi.mock('@/store', () => ({ useAppStore: { @@ -75,7 +77,7 @@ beforeEach(() => { afterEach(async () => { // Closing the tab drops its bytes document. for (const listener of store.listeners) { - listener({ openFiles: [] }) + listener({ openFiles: [] }, store) } await act(async () => root.unmount()) container.remove() diff --git a/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.tsx b/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.tsx index 08f4053cec2..e6c34e7fd40 100644 --- a/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.tsx +++ b/src/renderer/src/components/editor/PluginBytesCustomEditorSurface.tsx @@ -14,6 +14,7 @@ import PluginCustomEditorFrame from './PluginCustomEditorFrame' import { ExternalFileChangeBanner } from './ExternalFileChangeBanner' import { editorBytesDocument, + editorBytesEdits, hasEditorBytesDraft, loadEditorBytes, releaseEditorBytes, @@ -99,6 +100,7 @@ export default function PluginBytesCustomEditorSurface({ return } let cancelled = false + const editsAtRead = editorBytesEdits(fileId) readRuntimeFileBytes(source, CUSTOM_EDITOR_DOCUMENT_MAX_BYTES) .then(async (read) => { if (cancelled) { @@ -107,7 +109,7 @@ export default function PluginBytesCustomEditorSurface({ if (read.kind === 'ok') { const sha256 = await sha256Hex(read.bytes) if (!cancelled) { - loadEditorBytes(fileId, { bytes: read.bytes, fileType }, sha256) + loadEditorBytes(fileId, { bytes: read.bytes, fileType }, sha256, editsAtRead) setState({ status: 'ok' }) } } else if (read.kind === 'too_large') { diff --git a/src/renderer/src/components/editor/editor-bytes-document-boxes.ts b/src/renderer/src/components/editor/editor-bytes-document-boxes.ts new file mode 100644 index 00000000000..09c5fd0eb2e --- /dev/null +++ b/src/renderer/src/components/editor/editor-bytes-document-boxes.ts @@ -0,0 +1,35 @@ +import type { CustomEditorBinaryDocument } from './plugin-custom-editor-host' + +/** + * A binary custom editor's bytes last read or saved, and the unsaved edit. Kept apart from the + * text drafts so a text save can never write bytes as text; not kept across restarts. + */ +export type BytesDocument = { + saved: CustomEditorBinaryDocument + savedSha256: string + draft: CustomEditorBinaryDocument | null +} + +// Why a shared box: tabs of one file share one dirty flag, so they must share one draft too. +// `edits` counts edits and saves, so a read that started before one can be told apart. +export type BytesDocumentBox = { current: BytesDocument; edits: number } + +/** Per tab id. Its own module so the store's rename can move entries without importing the store. */ +export const bytesDocumentBoxes = new Map() + +/** A rename gives tabs new ids; their bytes, unsaved edits included, move with them. */ +export function rekeyEditorBytesDocuments( + rekeys: readonly { oldFileId: string; newFileId: string }[] +): void { + const moved = rekeys.flatMap(({ oldFileId, newFileId }) => { + const box = bytesDocumentBoxes.get(oldFileId) + return box ? [{ oldFileId, newFileId, box }] : [] + }) + // Why two passes: a swap of two ids must not delete the entry it just wrote. + for (const { oldFileId } of moved) { + bytesDocumentBoxes.delete(oldFileId) + } + for (const { newFileId, box } of moved) { + bytesDocumentBoxes.set(newFileId, box) + } +} diff --git a/src/renderer/src/components/editor/editor-bytes-documents.test.ts b/src/renderer/src/components/editor/editor-bytes-documents.test.ts index c99f0783549..4a27e5d5bc4 100644 --- a/src/renderer/src/components/editor/editor-bytes-documents.test.ts +++ b/src/renderer/src/components/editor/editor-bytes-documents.test.ts @@ -8,8 +8,12 @@ const state = vi.hoisted(() => { { id: 'book', isDirty: false } ] const tab = (id: string) => openFiles.find((file) => file.id === id) + const editorBufferIdOf: Record = {} + const editorBufferViews: Record = {} return { openFiles, + editorBufferIdOf, + editorBufferViews, markFileDirty: vi.fn((id: string, dirty: boolean) => { const found = tab(id) if (found) { @@ -24,7 +28,19 @@ const state = vi.hoisted(() => { }) } }) -vi.mock('@/store', () => ({ useAppStore: { subscribe: () => () => {}, getState: () => state } })) +const storeListener = vi.hoisted(() => { + const holder: { current: ((next: unknown, prev: unknown) => void) | null } = { current: null } + return holder +}) +vi.mock('@/store', () => ({ + useAppStore: { + subscribe: (listener: (next: unknown, prev: unknown) => void) => { + storeListener.current = listener + return () => {} + }, + getState: () => state + } +})) const writeRuntimeFileBytes = vi.fn() vi.mock('@/runtime/runtime-file-bytes-write', () => ({ writeRuntimeFileBytes: (...args: unknown[]) => writeRuntimeFileBytes(...args) @@ -32,13 +48,14 @@ vi.mock('@/runtime/runtime-file-bytes-write', () => ({ import { editorBytesDocument, + editorBytesEdits, loadEditorBytes, releaseEditorBytes, retainEditorBytes, saveEditorBytesDraft, - setEditorBytesDraft, - sha256Hex + setEditorBytesDraft } from './editor-bytes-documents' +import { bytesDocumentBoxes, rekeyEditorBytesDocuments } from './editor-bytes-document-boxes' // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the save reads only openFiles, markFileDirty and setExternalMutation. const getState = () => state as unknown as AppState @@ -49,12 +66,14 @@ const save = () => saveEditorBytesDraft(getState, file, context, 'user', () => f const loaded = new Uint8Array([0x50, 0x4b, 1]) const edited = { bytes: new Uint8Array([0x50, 0x4b, 2, 0xff]), fileType: 'xlsx' } const hex = (bytes: Uint8Array) => createHash('sha256').update(bytes).digest('hex') +const load = (id: string, bytes: Uint8Array, editsAtRead = editorBytesEdits(id)) => + loadEditorBytes(id, { bytes, fileType: 'xlsx' }, hex(bytes), editsAtRead) beforeEach(async () => { writeRuntimeFileBytes.mockReset() state.openFiles[0].isDirty = false state.openFiles[0].externalMutation = undefined - loadEditorBytes('book', { bytes: loaded, fileType: 'xlsx' }, await sha256Hex(loaded)) + load('book', loaded) }) describe('binary custom editor saves', () => { @@ -112,7 +131,7 @@ describe('binary custom editor views', () => { it('keeps an edit guarded by the bytes it started from when a later read lands', async () => { setEditorBytesDraft('book', edited) const newer = new Uint8Array([9]) - loadEditorBytes('book', { bytes: newer, fileType: 'xlsx' }, await sha256Hex(newer)) + load('book', newer) writeRuntimeFileBytes.mockResolvedValue({ ok: true }) await save() expect(writeRuntimeFileBytes.mock.calls[0]?.[4]).toEqual({ @@ -121,3 +140,62 @@ describe('binary custom editor views', () => { }) }) }) + +describe('binary custom editor reloads', () => { + it('keeps the same document when a reload reads unchanged bytes', async () => { + const before = editorBytesDocument('book') + const reread = new Uint8Array(loaded) + load('book', reread) + // A new object would resend the document and turn the frame's in-flight edit into a conflict. + expect(editorBytesDocument('book')).toBe(before) + }) +}) + +describe('binary custom editor tabs', () => { + it('ignores a read that started before an edit and save it does not include', async () => { + const editsAtRead = editorBytesEdits('book') + setEditorBytesDraft('book', edited) + writeRuntimeFileBytes.mockResolvedValue({ ok: true }) + await save() + load('book', loaded, editsAtRead) + expect(editorBytesDocument('book')).toBe(edited) + }) + + it('moves an unsaved edit with its tab when the file is renamed', () => { + setEditorBytesDraft('book', edited) + rekeyEditorBytesDocuments([{ oldFileId: 'book', newFileId: 'renamed' }]) + expect(editorBytesDocument('renamed')).toBe(edited) + expect(editorBytesDocument('book')).toBeNull() + rekeyEditorBytesDocuments([{ oldFileId: 'renamed', newFileId: 'book' }]) + }) + + it('shares one draft between tabs of one file, which share one dirty flag', async () => { + state.editorBufferIdOf = { book: 'record', twin: 'record' } + state.editorBufferViews = { record: ['book', 'twin'] } + state.openFiles.push({ id: 'twin', isDirty: false }) + load('twin', new Uint8Array(loaded)) + setEditorBytesDraft('twin', edited) + expect(editorBytesDocument('book')).toBe(edited) + // The saving tab closes mid-write; its twin still ends up clean against the written bytes. + writeRuntimeFileBytes.mockImplementation(async () => { + bytesDocumentBoxes.delete('book') + return { ok: true } + }) + await save() + expect(state.markFileDirty).toHaveBeenLastCalledWith('twin', false, { written: true }) + bytesDocumentBoxes.set('book', bytesDocumentBoxes.get('twin')!) + + // The tab holding a draft closes before its twin has shown it: the twin inherits the draft. + setEditorBytesDraft('book', edited) + bytesDocumentBoxes.delete('twin') + const previous = { ...state, openFiles: [...state.openFiles] } + state.openFiles.splice(0, 1) + storeListener.current?.(state, previous) + expect(editorBytesDocument('twin')).toBe(edited) + state.openFiles.unshift({ id: 'book', isDirty: false }) + bytesDocumentBoxes.set('book', bytesDocumentBoxes.get('twin')!) + state.openFiles.pop() + state.editorBufferIdOf = {} + state.editorBufferViews = {} + }) +}) diff --git a/src/renderer/src/components/editor/editor-bytes-documents.ts b/src/renderer/src/components/editor/editor-bytes-documents.ts index 9d62b8a7404..851cd979be3 100644 --- a/src/renderer/src/components/editor/editor-bytes-documents.ts +++ b/src/renderer/src/components/editor/editor-bytes-documents.ts @@ -13,19 +13,9 @@ import { } from './editor-self-write-registry' import { markEditorSaveRefusal } from './editor-save-rebaseline' import { EditorSaveDiscardedError } from './editor-discard-epoch' +import { editorBufferSiblingViewIds } from '@/store/slices/editor/buffers/editor-buffer-views' +import { bytesDocumentBoxes } from './editor-bytes-document-boxes' -/** - * A binary custom editor's document per tab: the bytes last read or saved, and the unsaved edit. - * Kept here, not in the text drafts, so a text save can never write them as text. Unsaved bytes - * live only while the tab is dirty and open; they are not kept across restarts. - */ -type BytesDocument = { - saved: CustomEditorBinaryDocument - savedSha256: string - draft: CustomEditorBinaryDocument | null -} - -const documents = new Map() const listeners = new Set<() => void>() let unsubscribeStore: (() => void) | null = null @@ -36,19 +26,27 @@ function notify(): void { } /** Drops a draft once its tab is clean (saved, reverted, discarded) and the entry once it closes. */ -function dropSettledDrafts(state: AppState): void { +function dropSettledDrafts(state: AppState, previous: AppState): void { let changed = false - for (const [fileId, document] of documents) { + const isOpen = (id: string): boolean => state.openFiles.some((file) => file.id === id) + for (const [fileId, box] of bytesDocumentBoxes) { const tab = state.openFiles.find((file) => file.id === fileId) if (!tab) { - documents.delete(fileId) + bytesDocumentBoxes.delete(fileId) + // Why: a twin still open keeps the record dirty, so it inherits the draft even if not yet shown. + const heir = box.current.draft + ? editorBufferSiblingViewIds(previous, fileId).find((id) => id !== fileId && isOpen(id)) + : undefined + if (heir !== undefined && !bytesDocumentBoxes.has(heir)) { + bytesDocumentBoxes.set(heir, box) + } changed = true - } else if (document.draft && !tab.isDirty) { - documents.set(fileId, { ...document, draft: null }) + } else if (box.current.draft && !tab.isDirty) { + box.current = { ...box.current, draft: null } changed = true } } - if (documents.size === 0) { + if (bytesDocumentBoxes.size === 0) { unsubscribeStore?.() unsubscribeStore = null } @@ -69,7 +67,7 @@ export function subscribeEditorBytes(listener: () => void): () => void { /** The document the editor shows: the unsaved edit, else the bytes on disk. */ export function editorBytesDocument(fileId: string): CustomEditorBinaryDocument | null { - const document = documents.get(fileId) + const document = bytesDocumentBoxes.get(fileId)?.current return document ? (document.draft ?? document.saved) : null } @@ -77,21 +75,46 @@ export function editorBytesDocument(fileId: string): CustomEditorBinaryDocument export function loadEditorBytes( fileId: string, saved: CustomEditorBinaryDocument, - savedSha256: string + savedSha256: string, + /** `editorBytesEdits` when the read started. */ + editsAtRead: number | undefined ): void { - const existing = documents.get(fileId) - // Why keep the draft's baseline: a later read must not vouch for bytes the edit never saw. - documents.set(fileId, existing?.draft ? existing : { saved, savedSha256, draft: null }) unsubscribeStore ??= useAppStore.subscribe(dropSettledDrafts) - notify() + const own = bytesDocumentBoxes.get(fileId) + if (!own) { + const siblings = editorBufferSiblingViewIds(useAppStore.getState(), fileId) + const shared = siblings.map((id) => bytesDocumentBoxes.get(id)).find(Boolean) + // Why adopt as is: the twin's box is newer than a read that may have started before its save. + bytesDocumentBoxes.set( + fileId, + shared ?? { current: { saved, savedSha256, draft: null }, edits: 0 } + ) + notify() + return + } + const existing = own.current + // Why: an edit or save since the read started is newer than what the read returns. + if (own.edits !== editsAtRead) { + return + } + // Why keep the draft's baseline: a later read must not vouch for bytes the edit never saw. + // Why keep unchanged bytes: a new object resends the document, so an edit in flight conflicts. + if ( + !existing.draft && + (existing.savedSha256 !== savedSha256 || existing.saved.fileType !== saved.fileType) + ) { + own.current = { saved, savedSha256, draft: null } + notify() + } } export function setEditorBytesDraft(fileId: string, draft: CustomEditorBinaryDocument): void { - const document = documents.get(fileId) - if (!document) { + const box = bytesDocumentBoxes.get(fileId) + if (!box) { return } - documents.set(fileId, { ...document, draft }) + box.current = { ...box.current, draft } + box.edits += 1 useAppStore.getState().markFileDirty(fileId, true) notify() } @@ -111,14 +134,18 @@ export function releaseEditorBytes(fileId: string): void { return } mountedViews.delete(fileId) - if (documents.get(fileId)?.draft === null) { - documents.delete(fileId) + if (bytesDocumentBoxes.get(fileId)?.current.draft === null) { + bytesDocumentBoxes.delete(fileId) notify() } } +export function editorBytesEdits(fileId: string): number | undefined { + return bytesDocumentBoxes.get(fileId)?.edits +} + export function hasEditorBytesDraft(fileId: string): boolean { - return Boolean(documents.get(fileId)?.draft) + return Boolean(bytesDocumentBoxes.get(fileId)?.current.draft) } /** Writes the tab's unsaved bytes, guarded by the hash of the bytes the edit started from. */ @@ -130,9 +157,10 @@ export async function saveEditorBytesDraft( /** A Don't Save since the request; checked at the last moment before the write. */ discarded: () => boolean ): Promise { - const document = documents.get(file.id) + const box = bytesDocumentBoxes.get(file.id) + const document = box?.current const draft = document?.draft - if (!document || !draft) { + if (!box || !document || !draft) { return 'written' } const remote = Boolean(context.connectionId || file.runtimeEnvironmentId?.trim()) @@ -172,15 +200,17 @@ export async function saveEditorBytesDraft( return 'blocked-stale' } const savedSha256 = await sha256Hex(draft.bytes) - const current = documents.get(file.id) - if (current) { + // Why any holder: the saving tab may have closed while a twin of the same file stays open. + const holder = [...bytesDocumentBoxes].find(([, held]) => held === box)?.[0] + if (holder !== undefined) { // An edit made while the write ran stays unsaved, now against the bytes just written. - const newer = current.draft !== draft ? current.draft : null - documents.set(file.id, { saved: draft, savedSha256, draft: newer }) + const newer = box.current.draft !== draft ? box.current.draft : null + box.current = { saved: draft, savedSha256, draft: newer } + box.edits += 1 const state = getState() - state.markFileDirty(file.id, newer !== null, { written: true }) - if (state.openFiles.find((tab) => tab.id === file.id)?.externalMutation) { - state.setExternalMutation(file.id, null) + state.markFileDirty(holder, newer !== null, { written: true }) + if (state.openFiles.find((tab) => tab.id === holder)?.externalMutation) { + state.setExternalMutation(holder, null) } notify() } diff --git a/src/renderer/src/components/editor/editor-panel-render-model.ts b/src/renderer/src/components/editor/editor-panel-render-model.ts index 82ab3d037c8..651bac25234 100644 --- a/src/renderer/src/components/editor/editor-panel-render-model.ts +++ b/src/renderer/src/components/editor/editor-panel-render-model.ts @@ -140,7 +140,7 @@ export function getEditorPanelRenderModel({ // Read-only tabs keep raw source, as for every other rich view. const textEditorEligible = customEditorContentReady && !rawReadOnly && inlineFileContent?.isBinary !== true - // Bytes editors are read-only, so they also view read-only tabs; they need a pageable route. + // Bytes editors honor read-only themselves, so they open read-only tabs too; they need a pageable route. const bytesEditorEligible = customEditorContentReady && inlineFileContent?.byteSource !== undefined const eligibleCustomEditors = customEditors.filter((editor) => diff --git a/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts b/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts index a72b06aaefa..0fc06634982 100644 --- a/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts +++ b/src/renderer/src/store/slices/editor/actions/rekey-open-files-action.ts @@ -7,6 +7,7 @@ import { rekeyEditorBuffers } from '../buffers/editor-buffer-record' import { migrateHydratedEditorTabsAndGroups } from '../file-ids/hydrated-editor-file-ids' import { renameEditorFilesThroughLayoutOwner } from '../tabs/editor-file-rename-through-layout-owner' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../../../shared/layout-owner-switch' +import { rekeyEditorBytesDocuments } from '@/components/editor/editor-bytes-document-boxes' // The store change is pure, so a layout-owner build applies it in the frame that shows main's rename. export type RekeyOpenFilesPatch = @@ -168,6 +169,7 @@ export function createRekeyOpenFilesAction( result = next return s } + rekeyEditorBytesDocuments(rekeys) return next.patch }) return result diff --git a/src/renderer/src/store/slices/editor/tabs/editor-file-rename-through-layout-owner.ts b/src/renderer/src/store/slices/editor/tabs/editor-file-rename-through-layout-owner.ts index 4f41a648ef4..61684d9746d 100644 --- a/src/renderer/src/store/slices/editor/tabs/editor-file-rename-through-layout-owner.ts +++ b/src/renderer/src/store/slices/editor/tabs/editor-file-rename-through-layout-owner.ts @@ -15,6 +15,7 @@ import type { OpenFilePathRekey, RekeyOpenFilesResult } from '../types/open-file import { isEditorTabContentType } from './editor-tab-content-type' import { restorableEditorFile } from './editor-tab-layout-owner' import { layoutEditorViewOfOpenFile } from './open-file-layout-view' +import { rekeyEditorBytesDocuments } from '@/components/editor/editor-bytes-document-boxes' /** Main's renames: an edit-mode file's record moves from its old path; a view takes the new one. */ function renamesOf( @@ -81,7 +82,12 @@ export function renameEditorFilesThroughLayoutOwner( const after = new Map((planned.patch.openFiles ?? []).map((file) => [file.id, file])) const rekeyHere = (own: readonly OpenFilePathRekey[]) => (s: AppState) => { const next = rekeyOpenFilesPatch(s, own, moveOperationId) - return next.ok ? next.patch : {} + if (!next.ok) { + return {} + } + // Why here: this runs as the rename lands, so the old tab's close never frees its bytes. + rekeyEditorBytesDocuments(own) + return next.patch } // Each file's own workspace: the same path can be open in more than one. const byWorktree = new Map()