diff --git a/src/renderer/src/components/editor/editor-cmd-save-target.test.ts b/src/renderer/src/components/editor/editor-cmd-save-target.test.ts index 2a1024875a0..518f8c8b859 100644 --- a/src/renderer/src/components/editor/editor-cmd-save-target.test.ts +++ b/src/renderer/src/components/editor/editor-cmd-save-target.test.ts @@ -24,23 +24,57 @@ describe('getEditorCmdSaveFileId', () => { expect( getEditorCmdSaveFileId( - { activeFileId: 'main-file', activeTabType: 'editor', getActiveTab }, + { + activeFileId: 'main-file', + activeTabType: 'editor', + activeView: 'terminal', + getActiveTab + }, false ) ).toBe('main-file') expect(getActiveTab).not.toHaveBeenCalled() }) + it('claims nothing on a non-workspace view so the shortcut is not swallowed', () => { + const getActiveTab = vi.fn(() => null) + + for (const activeView of ['tasks', 'settings', 'activity'] as const) { + expect( + getEditorCmdSaveFileId( + { activeFileId: 'main-file', activeTabType: 'editor', activeView, getActiveTab }, + false + ) + ).toBeNull() + } + }) + it('targets only an active floating editor and never falls through to main', () => { const getActiveTab = vi .fn<(worktreeId: string) => Tab | null>() .mockReturnValueOnce(makeTab('editor', 'floating-file')) .mockReturnValueOnce(makeTab('browser', 'floating-browser')) - const state = { activeFileId: 'main-file', activeTabType: 'editor', getActiveTab } + const state = { + activeFileId: 'main-file', + activeTabType: 'editor', + activeView: 'terminal' as const, + getActiveTab + } expect(getEditorCmdSaveFileId(state, true)).toBe('floating-file') expect(getEditorCmdSaveFileId(state, true)).toBeNull() expect(getActiveTab).toHaveBeenNthCalledWith(1, FLOATING_TERMINAL_WORKTREE_ID) expect(getActiveTab).toHaveBeenNthCalledWith(2, FLOATING_TERMINAL_WORKTREE_ID) }) + + it('still targets the floating editor from a non-workspace view', () => { + const getActiveTab = vi.fn(() => makeTab('editor', 'floating-file')) + + expect( + getEditorCmdSaveFileId( + { activeFileId: 'main-file', activeTabType: 'editor', activeView: 'tasks', getActiveTab }, + true + ) + ).toBe('floating-file') + }) }) diff --git a/src/renderer/src/components/editor/editor-cmd-save-target.ts b/src/renderer/src/components/editor/editor-cmd-save-target.ts index 157eb269fdb..0d021a87790 100644 --- a/src/renderer/src/components/editor/editor-cmd-save-target.ts +++ b/src/renderer/src/components/editor/editor-cmd-save-target.ts @@ -1,5 +1,5 @@ import { FLOATING_TERMINAL_WORKTREE_ID } from '../../../../shared/constants' -import type { Tab, TabContentType } from '../../../../shared/types' +import type { Tab, TabContentType, TopLevelView } from '../../../../shared/types' export const EDITOR_TAB_CONTENT_TYPES = new Set([ 'editor', @@ -11,6 +11,7 @@ export const EDITOR_TAB_CONTENT_TYPES = new Set([ type EditorCmdSaveState = { activeFileId: string | null activeTabType: string | null + activeView: TopLevelView getActiveTab: (worktreeId: string) => Tab | null } @@ -19,7 +20,12 @@ export function getEditorCmdSaveFileId( floatingPanelOwnsEvent: boolean ): string | null { if (!floatingPanelOwnsEvent) { - return state.activeTabType === 'editor' ? state.activeFileId : null + // Why: outside the workspace view no mounted panel claims the request, so + // returning an id would swallow Cmd/Ctrl+S on Tasks/Settings without saving. + // The floating panel floats above every view and keeps its own ownership. + return state.activeView === 'terminal' && state.activeTabType === 'editor' + ? state.activeFileId + : null } const activeTab = state.getActiveTab(FLOATING_TERMINAL_WORKTREE_ID) return activeTab && EDITOR_TAB_CONTENT_TYPES.has(activeTab.contentType) diff --git a/src/renderer/src/components/editor/editor-panel-content-types.ts b/src/renderer/src/components/editor/editor-panel-content-types.ts index c44df176150..98591ffa8e6 100644 --- a/src/renderer/src/components/editor/editor-panel-content-types.ts +++ b/src/renderer/src/components/editor/editor-panel-content-types.ts @@ -25,6 +25,8 @@ export type FileContent = { mimeType?: string fileIdentity?: string loadError?: string + /** Superseded by an external change; still rendered until the lazy reload lands. */ + isStale?: boolean } export type DiffContent = GitDiffResult diff --git a/src/renderer/src/components/editor/useEditorPanelContentState.ts b/src/renderer/src/components/editor/useEditorPanelContentState.ts index 14a4375d196..42ae831264e 100644 --- a/src/renderer/src/components/editor/useEditorPanelContentState.ts +++ b/src/renderer/src/components/editor/useEditorPanelContentState.ts @@ -85,6 +85,18 @@ function stampCleanTabDiskBaseline(id: string, result: FileContent): void { } } +// Why: the newest read for a tab is already on its way, so a re-run of the lazy +// effect (a worktree flip-flop re-adds `isVisible`) must not fire a second RPC. +// An invalidation bumps the tab's generation, so a superseded read never counts. +function hasLiveRead( + generationsById: Record, + outstandingById: Record, + id: string +): boolean { + const generation = generationsById[id] + return generation !== undefined && outstandingById[id] === generation +} + function inFlightReadKey(connectionId: string | undefined, filePath: string): string { return `${connectionId ?? ''}::${filePath}` } @@ -123,6 +135,8 @@ export function useEditorPanelContentState({ const diffReadGenerationRef = useRef>({}) const fileReadGenerationCounterRef = useRef(0) const diffReadGenerationCounterRef = useRef(0) + const outstandingFileReadsRef = useRef>({}) + const outstandingDiffReadsRef = useRef>({}) const openFilesRef = useRef(openFiles) const editorViewModeRef = useRef(editorViewMode) const isVisibleRef = useRef(isVisible) @@ -152,8 +166,11 @@ export function useEditorPanelContentState({ const next = { ...prev } let changed = false for (const fileId of uniqueIds) { - if (fileId in next) { - delete next[fileId] + const existing = next[fileId] + // Why: keep the last-known bytes rendered and swap them when the lazy + // reload lands — dropping them flashes "Loading…" on every reveal. + if (existing && existing.isStale !== true) { + next[fileId] = { ...existing, isStale: true } changed = true } } @@ -198,6 +215,7 @@ export function useEditorPanelContentState({ const generation = fileReadGenerationCounterRef.current + 1 fileReadGenerationCounterRef.current = generation fileReadGenerationRef.current[id] = generation + outstandingFileReadsRef.current[id] = generation try { const resolvedConnectionId = getConnectionIdForFile(worktreeId ?? null, filePath) const connectionId = resolvedConnectionId ?? undefined @@ -326,6 +344,10 @@ export function useEditorPanelContentState({ ...prev, [id]: { content: '', isBinary: false, loadError: message } })) + } finally { + if (outstandingFileReadsRef.current[id] === generation) { + delete outstandingFileReadsRef.current[id] + } } }, [] @@ -339,6 +361,7 @@ export function useEditorPanelContentState({ const generation = diffReadGenerationCounterRef.current + 1 diffReadGenerationCounterRef.current = generation diffReadGenerationRef.current[file.id] = generation + outstandingDiffReadsRef.current[file.id] = generation try { const worktreePath = file.filePath.slice( 0, @@ -451,6 +474,10 @@ export function useEditorPanelContentState({ modifiedIsBinary: false } })) + } finally { + if (outstandingDiffReadsRef.current[file.id] === generation) { + delete outstandingDiffReadsRef.current[file.id] + } } }, [] @@ -491,6 +518,17 @@ export function useEditorPanelContentState({ useLocalLogTail({ openFiles, fileContents, setFileContents, reloadContent }) + const needsFileRead = (fileId: string): boolean => { + const cached = fileContents[fileId] + return ( + (!cached || cached.isStale === true) && + !hasLiveRead(fileReadGenerationRef.current, outstandingFileReadsRef.current, fileId) + ) + } + const needsDiffRead = (fileId: string): boolean => + !diffContents[fileId] && + !hasLiveRead(diffReadGenerationRef.current, outstandingDiffReadsRef.current, fileId) + useEffect(() => { if (!isVisible) { return @@ -514,7 +552,7 @@ export function useEditorPanelContentState({ } const absolutePath = joinPath(activeFile.filePath, entry.path) - if (!fileContents[absolutePath]) { + if (needsFileRead(absolutePath)) { void loadFileContent(absolutePath, absolutePath, activeFile.worktreeId, entry.path) } } @@ -529,7 +567,7 @@ export function useEditorPanelContentState({ if (fileToLoad.conflict?.kind === 'conflict-placeholder') { return } - if (!fileContents[fileToLoad.id]) { + if (needsFileRead(fileToLoad.id)) { void loadFileContent( fileToLoad.filePath, fileToLoad.id, @@ -537,10 +575,10 @@ export function useEditorPanelContentState({ fileToLoad.relativePath ) } - if (isChangesMode && !diffContents[fileToLoad.id]) { + if (isChangesMode && needsDiffRead(fileToLoad.id)) { void loadDiffContent(fileToLoad) } - } else if (isReloadableSingleFileDiffTab(fileToLoad) && !diffContents[fileToLoad.id]) { + } else if (isReloadableSingleFileDiffTab(fileToLoad) && needsDiffRead(fileToLoad.id)) { void loadDiffContent(fileToLoad) } // oxlint-disable-next-line react-hooks/exhaustive-deps diff --git a/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx b/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx index eacc6c8fec3..bfa26936c15 100644 --- a/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx +++ b/src/renderer/src/components/editor/useEditorPanelVisibilityContentState.test.tsx @@ -217,6 +217,62 @@ describe('useEditorPanelContentState visibility', () => { ) }) + it('keeps invalidated bytes on screen until the reveal read lands', async () => { + const file = makeFile('no-flash') + const freshRead = createDeferred() + mocks.readRuntimeFileContent + .mockResolvedValueOnce({ content: 'old', isBinary: false }) + .mockReturnValueOnce(freshRead.promise) + + await act(async () => root.render()) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + ) + + await act(async () => root.render()) + dispatchExternalChange(file) + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + + await act(async () => root.render()) + await vi.waitFor(() => expect(mocks.readRuntimeFileContent).toHaveBeenCalledTimes(2)) + // Why: an unresolved reveal read must not blank the pane to "Loading…". + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('old') + + await act(async () => { + freshRead.resolve({ content: 'fresh', isBinary: false }) + await freshRead.promise + }) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]).toEqual({ + content: 'fresh', + isBinary: false + }) + ) + }) + + it('does not re-read while the reveal read is still in flight', async () => { + const file = makeFile('flip-flop') + const pendingRead = createDeferred() + mocks.readRuntimeFileContent.mockReturnValue(pendingRead.promise) + + await act(async () => root.render()) + await act(async () => root.render()) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + + await act(async () => root.render()) + await act(async () => root.render()) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + + await act(async () => { + pendingRead.resolve({ content: 'remote', isBinary: false }) + await pendingRead.promise + }) + await vi.waitFor(() => + expect(snapshots.get('main')?.fileContents[file.id]?.content).toBe('remote') + ) + expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() + }) + it('publishes hidden visibility before a watcher event in the same commit', async () => { const file = makeFile('commit-visibility') mocks.readRuntimeFileContent.mockResolvedValue({ content: 'old', isBinary: false }) @@ -234,7 +290,11 @@ describe('useEditorPanelContentState visibility', () => { ) expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() - expect(snapshots.get('main')?.fileContents[file.id]).toBeUndefined() + expect(snapshots.get('main')?.fileContents[file.id]).toEqual({ + content: 'old', + isBinary: false, + isStale: true + }) }) it('shares one post-change read between visible source and preview panels', async () => { @@ -285,7 +345,7 @@ describe('useEditorPanelContentState visibility', () => { const changed = { ...file, fileContentReloadNonce: 1 } await act(async () => root.render()) - expect(snapshots.get('main')?.fileContents[file.id]).toBeUndefined() + expect(snapshots.get('main')?.fileContents[file.id]?.isStale).toBe(true) expect(mocks.readRuntimeFileContent).toHaveBeenCalledOnce() await act(async () => root.render())