From eaebd08ff9f96a796b608b30e3838688cfd50aba Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 3 May 2026 16:17:07 -0700 Subject: [PATCH] =?UTF-8?q?feat(editor):=20Changes=20view=20mode=20?= =?UTF-8?q?=E2=80=94=20in-tab=20HEAD-vs-working-tree=20diff=20(#1353)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Orca --- src/main/git/status.ts | 7 +- src/main/ipc/filesystem.ts | 17 +- src/main/providers/ssh-git-provider.ts | 10 +- src/main/providers/types.ts | 7 +- src/preload/api-types.ts | 1 + src/preload/index.ts | 1 + src/relay/git-handler-ops.ts | 7 +- src/relay/git-handler.ts | 9 +- .../src/components/editor/ChangesModeView.tsx | 102 ++++++++++ .../src/components/editor/DiffViewer.tsx | 13 +- .../src/components/editor/EditorContent.tsx | 29 ++- .../src/components/editor/EditorPanel.tsx | 180 ++++++++++++++++-- .../components/editor/EditorViewToggle.tsx | 112 +++++++++++ .../components/editor/MarkdownViewToggle.tsx | 75 -------- .../editor/markdown-preview-controls.ts | 20 ++ .../right-sidebar/SourceControl.tsx | 41 +++- src/renderer/src/store/slices/editor.test.ts | 44 +++++ src/renderer/src/store/slices/editor.ts | 48 +++++ .../src/store/slices/worktrees.test.ts | 30 +++ src/renderer/src/store/slices/worktrees.ts | 4 + 20 files changed, 639 insertions(+), 118 deletions(-) create mode 100644 src/renderer/src/components/editor/ChangesModeView.tsx create mode 100644 src/renderer/src/components/editor/EditorViewToggle.tsx delete mode 100644 src/renderer/src/components/editor/MarkdownViewToggle.tsx diff --git a/src/main/git/status.ts b/src/main/git/status.ts index 8678cbe74..4c9345fc1 100644 --- a/src/main/git/status.ts +++ b/src/main/git/status.ts @@ -290,7 +290,8 @@ export async function resolveGitDir(worktreePath: string): Promise { export async function getDiff( worktreePath: string, filePath: string, - staged: boolean + staged: boolean, + compareAgainstHead = false ): Promise { let originalContent = '' let modifiedContent = '' @@ -300,7 +301,9 @@ export async function getDiff( try { const leftBlob = staged ? await readGitBlobAtOidPath(worktreePath, 'HEAD', filePath) - : await readUnstagedLeftBlob(worktreePath, filePath) + : compareAgainstHead + ? await readGitBlobAtOidPath(worktreePath, 'HEAD', filePath) + : await readUnstagedLeftBlob(worktreePath, filePath) originalContent = leftBlob.content originalIsBinary = leftBlob.isBinary diff --git a/src/main/ipc/filesystem.ts b/src/main/ipc/filesystem.ts index 902462363..756e702c9 100644 --- a/src/main/ipc/filesystem.ts +++ b/src/main/ipc/filesystem.ts @@ -479,18 +479,29 @@ export function registerFilesystemHandlers(store: Store): void { 'git:diff', async ( _event, - args: { worktreePath: string; filePath: string; staged: boolean; connectionId?: string } + args: { + worktreePath: string + filePath: string + staged: boolean + compareAgainstHead?: boolean + connectionId?: string + } ): Promise => { if (args.connectionId) { const provider = getSshGitProvider(args.connectionId) if (!provider) { throw new Error(`No git provider for connection "${args.connectionId}"`) } - return provider.getDiff(args.worktreePath, args.filePath, args.staged) + return provider.getDiff( + args.worktreePath, + args.filePath, + args.staged, + args.compareAgainstHead + ) } const worktreePath = await resolveRegisteredWorktreePath(args.worktreePath, store) const filePath = validateGitRelativeFilePath(worktreePath, args.filePath) - return getDiff(worktreePath, filePath, args.staged) + return getDiff(worktreePath, filePath, args.staged, args.compareAgainstHead) } ) diff --git a/src/main/providers/ssh-git-provider.ts b/src/main/providers/ssh-git-provider.ts index 1d91fc561..75ced3966 100644 --- a/src/main/providers/ssh-git-provider.ts +++ b/src/main/providers/ssh-git-provider.ts @@ -26,11 +26,17 @@ export class SshGitProvider implements IGitProvider { return (await this.mux.request('git.status', { worktreePath })) as GitStatusResult } - async getDiff(worktreePath: string, filePath: string, staged: boolean): Promise { + async getDiff( + worktreePath: string, + filePath: string, + staged: boolean, + compareAgainstHead?: boolean + ): Promise { return (await this.mux.request('git.diff', { worktreePath, filePath, - staged + staged, + compareAgainstHead })) as GitDiffResult } diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index db7fc4d3c..6c31de31b 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -132,7 +132,12 @@ export type IFilesystemProvider = { export type IGitProvider = { getStatus(worktreePath: string): Promise - getDiff(worktreePath: string, filePath: string, staged: boolean): Promise + getDiff( + worktreePath: string, + filePath: string, + staged: boolean, + compareAgainstHead?: boolean + ): Promise stageFile(worktreePath: string, filePath: string): Promise unstageFile(worktreePath: string, filePath: string): Promise bulkStageFiles(worktreePath: string, filePaths: string[]): Promise diff --git a/src/preload/api-types.ts b/src/preload/api-types.ts index 4bb36f79c..ef80f59d0 100644 --- a/src/preload/api-types.ts +++ b/src/preload/api-types.ts @@ -763,6 +763,7 @@ export type PreloadApi = { worktreePath: string filePath: string staged: boolean + compareAgainstHead?: boolean connectionId?: string }) => Promise branchCompare: (args: { diff --git a/src/preload/index.ts b/src/preload/index.ts index a71c25e0f..249d43b96 100644 --- a/src/preload/index.ts +++ b/src/preload/index.ts @@ -1247,6 +1247,7 @@ const api = { worktreePath: string filePath: string staged: boolean + compareAgainstHead?: boolean connectionId?: string }): Promise => ipcRenderer.invoke('git:diff', args), branchCompare: (args: { diff --git a/src/relay/git-handler-ops.ts b/src/relay/git-handler-ops.ts index b437ff446..8d89a85d3 100644 --- a/src/relay/git-handler-ops.ts +++ b/src/relay/git-handler-ops.ts @@ -73,7 +73,8 @@ export async function computeDiff( git: GitBufferExec, worktreePath: string, filePath: string, - staged: boolean + staged: boolean, + compareAgainstHead = false ) { let originalContent = '' let modifiedContent = '' @@ -90,7 +91,9 @@ export async function computeDiff( modifiedContent = right.content modifiedIsBinary = right.isBinary } else { - const left = await readUnstagedLeft(git, worktreePath, filePath) + const left = compareAgainstHead + ? await readBlobAtOid(git, worktreePath, 'HEAD', filePath) + : await readUnstagedLeft(git, worktreePath, filePath) originalContent = left.content originalIsBinary = left.isBinary diff --git a/src/relay/git-handler.ts b/src/relay/git-handler.ts index 0875fb2d3..7e312bea0 100644 --- a/src/relay/git-handler.ts +++ b/src/relay/git-handler.ts @@ -146,8 +146,13 @@ export class GitHandler { if (rel.startsWith('..') || path.isAbsolute(rel)) { throw new Error(`Path "${filePath}" resolves outside the worktree`) } - const staged = params.staged as boolean - return computeDiff(this.gitBuffer.bind(this), worktreePath, filePath, staged) + return computeDiff( + this.gitBuffer.bind(this), + worktreePath, + filePath, + params.staged as boolean, + params.compareAgainstHead as boolean | undefined + ) } private async stage(params: Record) { diff --git a/src/renderer/src/components/editor/ChangesModeView.tsx b/src/renderer/src/components/editor/ChangesModeView.tsx new file mode 100644 index 000000000..7260475dc --- /dev/null +++ b/src/renderer/src/components/editor/ChangesModeView.tsx @@ -0,0 +1,102 @@ +import React, { lazy } from 'react' +import type { OpenFile } from '@/store/slices/editor' +import type { GitDiffResult, GitStatusEntry } from '../../../../shared/types' +import { ConflictBanner } from './ConflictComponents' + +const DiffViewer = lazy(() => import('./DiffViewer')) + +function getContentSignature(content: string): string { + let hash = 2166136261 + for (let i = 0; i < content.length; i += 1) { + hash ^= content.charCodeAt(i) + hash = Math.imul(hash, 16777619) + } + return (hash >>> 0).toString(16) +} + +// Why: Changes view mode renders an edit-mode tab as a HEAD-vs-working-tree +// diff without creating a separate diff-tab object. The draft is the live +// source on the modified side; onContentChange is the same callback as normal +// edit mode so dirty tracking, autosave, and close-prompt plumbing all continue +// to work unchanged. See reviews/changes-view-mode-plan.md. +export function ChangesModeView({ + activeFile, + dc, + modifiedContent, + activeConflictEntry, + resolvedLanguage, + sideBySide, + viewStateScopeId, + diffViewStateKey, + onContentChange, + onSave +}: { + activeFile: OpenFile + dc: GitDiffResult | undefined + modifiedContent: string + activeConflictEntry: GitStatusEntry | null + resolvedLanguage: string + sideBySide: boolean + viewStateScopeId: string + diffViewStateKey: string + onContentChange: (content: string) => void + onSave: (content: string) => Promise +}): React.JSX.Element { + if (!dc) { + return ( +
+ Loading diff... +
+ ) + } + if (dc.kind === 'binary') { + return ( +
+
+
Binary file
+
+ Text diff is unavailable for this file. +
+
+
+ ) + } + // Why: Monaco renders an empty diff when the two sides match, which reads as + // a broken view. Surface an inline banner so the user knows Changes mode is + // active but there is simply nothing to diff right now. + const isIdentical = dc.originalContent === modifiedContent + // Why: after a terminal commit/pull/rebase, Changes mode refreshes the + // HEAD-side blob in React state, but Monaco can keep painting the previous + // diff if we reuse the same kept model identities. Rotate only the + // original-side model identity so Monaco rebuilds the stale HEAD snapshot + // without throwing away the modified-side undo history. + const headContentSignature = getContentSignature(dc.originalContent) + const originalModelKey = `${diffViewStateKey}:original:${headContentSignature}` + return ( +
+ {activeFile.conflict && } + {isIdentical && ( +
+ No uncommitted changes. +
+ )} +
+ +
+
+ ) +} diff --git a/src/renderer/src/components/editor/DiffViewer.tsx b/src/renderer/src/components/editor/DiffViewer.tsx index 8c0e9200e..ca6f5165e 100644 --- a/src/renderer/src/components/editor/DiffViewer.tsx +++ b/src/renderer/src/components/editor/DiffViewer.tsx @@ -14,6 +14,8 @@ import type { DiffComment } from '../../../../shared/types' type DiffViewerProps = { modelKey: string + originalModelKey?: string + modifiedModelKey?: string originalContent: string modifiedContent: string language: string @@ -38,6 +40,8 @@ type DiffViewerProps = { export default function DiffViewer({ modelKey, + originalModelKey, + modifiedModelKey, originalContent, modifiedContent, language, @@ -167,6 +171,8 @@ export default function DiffViewer({ const propsRef = useRef({ relativePath, language, onSave }) propsRef.current = { relativePath, language, onSave } + const resolvedOriginalModelKey = originalModelKey ?? modelKey + const resolvedModifiedModelKey = modifiedModelKey ?? modelKey const handleMount: DiffOnMount = useCallback( (diffEditor, monaco) => { @@ -268,8 +274,11 @@ export default function DiffViewer({ // (staged, unstaged, branch compare versions). The kept Monaco models // must therefore key off the tab identity, not the raw file path, or // one diff tab can incorrectly reuse another tab's model contents. - originalModelPath={`diff:original:${modelKey}`} - modifiedModelPath={`diff:modified:${modelKey}`} + // Why: Changes mode sometimes needs to rotate only the original-side + // model after HEAD moves, while preserving the modified-side model's + // undo stack for continued editing. + originalModelPath={`diff:original:${resolvedOriginalModelKey}`} + modifiedModelPath={`diff:modified:${resolvedModifiedModelKey}`} keepCurrentOriginalModel keepCurrentModifiedModel options={{ diff --git a/src/renderer/src/components/editor/EditorContent.tsx b/src/renderer/src/components/editor/EditorContent.tsx index 35f92f5c9..c4038e6ac 100644 --- a/src/renderer/src/components/editor/EditorContent.tsx +++ b/src/renderer/src/components/editor/EditorContent.tsx @@ -1,10 +1,13 @@ -/* eslint-disable max-lines -- Why: this component is the central dispatcher -that maps (language, viewMode, binary, conflict) onto the correct editor -surface. Splitting the branches across files would force the view-mode state -machine to live behind indirection that obscures the exhaustive conditionals. */ +/* eslint-disable max-lines -- Why: EditorContent is the dispatch surface for +every editor mode (edit, diff, conflict, markdown-preview, combined-diff, and +now Changes view mode). Keeping the mode-selection branches colocated is easier +to reason about than scattering the switch across per-mode wrappers. Individual +renderers (MonacoEditor, DiffViewer, ChangesModeView, MarkdownPreview, etc.) +already live in their own modules. */ import React, { lazy } from 'react' import { detectLanguage } from '@/lib/language-detect' import { useAppStore } from '@/store' +import { ChangesModeView } from './ChangesModeView' import { ConflictBanner, ConflictPlaceholderView, ConflictReviewPanel } from './ConflictComponents' import type { MarkdownViewMode, OpenFile } from '@/store/slices/editor' import type { GitStatusEntry, GitDiffResult } from '../../../../shared/types' @@ -49,6 +52,7 @@ export function EditorContent({ isMermaid, isCsv, mdViewMode, + isChangesMode, sideBySide, pendingEditorReveal, handleContentChange, @@ -66,6 +70,7 @@ export function EditorContent({ isMermaid: boolean isCsv: boolean mdViewMode: MarkdownViewMode + isChangesMode: boolean sideBySide: boolean pendingEditorReveal: { filePath?: string @@ -344,6 +349,22 @@ export function EditorContent({ ) } + if (isChangesMode) { + return ( + + ) + } return (
{activeFile.conflict && } diff --git a/src/renderer/src/components/editor/EditorPanel.tsx b/src/renderer/src/components/editor/EditorPanel.tsx index a7c733c8b..805e5d44f 100644 --- a/src/renderer/src/components/editor/EditorPanel.tsx +++ b/src/renderer/src/components/editor/EditorPanel.tsx @@ -23,7 +23,7 @@ import { } from '@/components/ui/dropdown-menu' import { CLOSE_ALL_CONTEXT_MENUS_EVENT } from '../tab-bar/SortableTab' import type { MarkdownViewMode, OpenFile } from '@/store/slices/editor' -import MarkdownViewToggle, { CSV_VIEW_MODE_METADATA } from './MarkdownViewToggle' +import EditorViewToggle, { CSV_VIEW_MODE_METADATA } from './EditorViewToggle' import { EditorContent } from './EditorContent' import { scrollTopCache, cursorPositionCache, diffViewStateCache } from '@/lib/scroll-cache' import type { GitDiffResult } from '../../../../shared/types' @@ -42,10 +42,12 @@ import { exportActiveMarkdownToPdf } from './export-active-markdown' import { canOpenMarkdownPreview, getDefaultMarkdownViewMode, + getEditorToggleModes, getMarkdownPreviewShortcutLabel, getMarkdownViewModes, isMarkdownPreviewShortcut } from './markdown-preview-controls' +import type { EditorToggleValue } from './EditorViewToggle' const isMac = navigator.userAgent.includes('Mac') const isLinux = navigator.userAgent.includes('Linux') @@ -111,7 +113,11 @@ function inFlightReadKey(connectionId: string | undefined, filePath: string): st return `${connectionId ?? ''}::${filePath}` } -function inFlightDiffKey(file: OpenFile, connectionId: string | undefined): string { +function inFlightDiffKey( + file: OpenFile, + connectionId: string | undefined, + compareAgainstHead = false +): string { // Why: diff content depends on the file path AND which diff source is // being rendered (unstaged/staged/branch). Branch diffs further depend // on the base+head oids so switching compare points doesn't alias, and @@ -121,7 +127,7 @@ function inFlightDiffKey(file: OpenFile, connectionId: string | undefined): stri file.diffSource === 'branch' && file.branchCompare ? `${file.branchCompare.baseOid ?? ''}..${file.branchCompare.headOid ?? ''}::${file.branchOldPath ?? ''}` : '' - return `${connectionId ?? ''}::${file.diffSource ?? ''}::${file.filePath}::${branch}` + return `${connectionId ?? ''}::${file.diffSource ?? ''}::${compareAgainstHead ? 'head' : 'default'}::${file.filePath}::${branch}` } function EditorPanelInner({ @@ -140,6 +146,8 @@ function EditorPanelInner({ const gitBranchChangesByWorktree = useAppStore((s) => s.gitBranchChangesByWorktree) const markdownViewMode = useAppStore((s) => s.markdownViewMode) const setMarkdownViewMode = useAppStore((s) => s.setMarkdownViewMode) + const editorViewMode = useAppStore((s) => s.editorViewMode) + const setEditorViewMode = useAppStore((s) => s.setEditorViewMode) const openFile = useAppStore((s) => s.openFile) const openMarkdownPreview = useAppStore((s) => s.openMarkdownPreview) const closeFile = useAppStore((s) => s.closeFile) @@ -155,9 +163,21 @@ function EditorPanelInner({ const activeFileMode = activeFile?.mode ?? null const activeFileDiffSource = activeFile?.diffSource const activeViewStateId = activeViewStateIdProp ?? activeFileId - const [fileContents, setFileContents] = useState>({}) const [diffContents, setDiffContents] = useState>({}) + // Why: Changes view mode only applies on top of a regular edit-mode tab. It + // swaps the MonacoEditor for a DiffViewer (HEAD vs working tree incl. unsaved + // draft) without creating a new tab. Transient tabs (diff, conflict-review, + // markdown-preview) keep their own rendering pipeline. + // Binary content short-circuits to the binary placeholder in EditorContent + // before isChangesMode is consulted, so we must also exclude binary files + // here — otherwise the header toggle would still show Changes as selected + // and expose the inline/side-by-side toggle even though no diff is rendered. + const isChangesMode = + !!activeFile && + activeFile.mode === 'edit' && + editorViewMode[activeFile.id] === 'changes' && + !fileContents[activeFile.id]?.isBinary const [copiedPathToast, setCopiedPathToast] = useState<{ fileId: string; token: number } | null>( null ) @@ -191,6 +211,13 @@ function EditorPanelInner({ const openFilesRef = useRef(openFiles) openFilesRef.current = openFiles + // Why: the external-file-change handler below needs to consult the latest + // editorViewMode, but we do not want to re-register its window listener + // every time an unrelated editor-mode toggle flips. A ref lets the handler + // read the current value without adding editorViewMode to the effect deps. + const editorViewModeRef = useRef(editorViewMode) + editorViewModeRef.current = editorViewMode + useEffect(() => { const closeMenu = (): void => setPathMenuOpen(false) window.addEventListener(CLOSE_ALL_CONTEXT_MENUS_EVENT, closeMenu) @@ -286,10 +313,15 @@ function EditorPanelInner({ if (activeFile.conflict?.kind === 'conflict-placeholder') { return } - if (fileContents[activeFile.id]) { - return + if (!fileContents[activeFile.id]) { + void loadFileContent(activeFile.filePath, activeFile.id, activeFile.worktreeId) + } + // Why: Changes view mode needs the HEAD-side blob as well as the + // working-tree content. Kick off the diff load alongside the normal + // file read so both are ready by the time DiffViewer mounts. + if (isChangesMode && !diffContents[activeFile.id]) { + void loadDiffContent(activeFile) } - void loadFileContent(activeFile.filePath, activeFile.id, activeFile.worktreeId) } else if ( activeFile.mode === 'diff' && activeFile.diffSource !== undefined && @@ -301,7 +333,7 @@ function EditorPanelInner({ } void loadDiffContent(activeFile) } - }, [activeFile?.id]) // eslint-disable-line react-hooks/exhaustive-deps + }, [activeFile?.id, isChangesMode]) // eslint-disable-line react-hooks/exhaustive-deps useEffect(() => { if (!copiedPathToast) { @@ -362,7 +394,20 @@ function EditorPanelInner({ ? file.branchCompare : null const connectionId = getConnectionId(file.worktreeId) ?? undefined - const key = inFlightDiffKey(file, connectionId) + // Why: Changes view mode runs on top of an edit-mode tab and asks git + // for an unstaged diff against HEAD for that file. Use the 'unstaged' + // diff-source key so multiple Changes tabs across split panes share one + // IPC round-trip with any open unstaged diff-tab for the same path. + // Compute this once and reuse it for both the dedup key and the IPC + // branch selection so the two can never drift apart. + const effectiveDiffSource: typeof file.diffSource = + file.mode === 'edit' ? 'unstaged' : file.diffSource + const compareAgainstHead = file.mode === 'edit' + const key = inFlightDiffKey( + { ...file, diffSource: effectiveDiffSource }, + connectionId, + compareAgainstHead + ) // Why: same rationale as inFlightFileReads above — a single external // change fans out to every mounted EditorPanel, and two split panes // showing the same diff tab should share one git.diff IPC instead of @@ -370,7 +415,7 @@ function EditorPanelInner({ let pending = inFlightDiffReads.get(key) if (!pending) { pending = ( - file.diffSource === 'branch' && branchCompare + effectiveDiffSource === 'branch' && branchCompare ? window.api.git.branchDiff({ worktreePath, compare: { @@ -386,7 +431,8 @@ function EditorPanelInner({ : window.api.git.diff({ worktreePath, filePath: file.relativePath, - staged: file.diffSource === 'staged', + staged: effectiveDiffSource === 'staged', + compareAgainstHead, connectionId }) ) as Promise @@ -413,6 +459,40 @@ function EditorPanelInner({ } }, []) + // Why: refetch the HEAD-side blob for Changes mode when the worktree's git + // status array identity changes. A commit, pull, or rebase updates the + // status poll result, which is the cheapest signal we have that HEAD moved + // — without this, users see a stale diff after committing from Changes mode. + // Subscribing to the status array keeps parity with the Changes sidebar. + const changesStatusEntries = activeFile?.worktreeId + ? gitStatusByWorktree[activeFile.worktreeId] + : undefined + // Why: depend on the primitive identifiers of the active file rather than + // the `activeFile` object. `openFiles` is rebuilt on any store update that + // touches an open file (dirty flips, saves, status polling), so the + // `activeFile` object reference changes on many unrelated renders. Each + // identity change would otherwise retrigger the effect and dispatch a + // spurious git.diff IPC that the in-flight dedup map cannot coalesce + // across time. Resolve the current file via `openFilesRef` inside the + // effect so we still pass a live OpenFile to loadDiffContent. + useEffect(() => { + if (!isChangesMode || !activeFile?.id) { + return + } + const current = openFilesRef.current.find((f) => f.id === activeFile.id) + if (!current) { + return + } + void loadDiffContent(current) + }, [ + changesStatusEntries, + isChangesMode, + activeFile?.id, + activeFile?.worktreeId, + activeFile?.relativePath, + loadDiffContent + ]) + const handleContentChange = useCallback( (content: string) => { if (!activeFile) { @@ -484,6 +564,31 @@ function EditorPanelInner({ [activeFile, openFiles] ) + // Why: hooks must run unconditionally, so this useCallback lives above the + // `if (!activeFile) return null` guard; the callback itself no-ops when + // no file is active. Memoised to match the other editor handlers in this + // file and avoid churning EditorViewToggle's onChange identity. + const handleEditorToggleChange = useCallback( + (next: EditorToggleValue): void => { + const fileId = activeFile?.id + if (!fileId) { + return + } + if (next === 'changes') { + setEditorViewMode(fileId, 'changes') + return + } + // Why: selecting any non-Changes segment implicitly exits Changes mode. + // For markdown/mermaid files, also persist the chosen language sub-mode + // so that next time Changes is toggled off, the file returns to that view. + setEditorViewMode(fileId, 'edit') + if (next !== 'edit') { + setMarkdownViewMode(fileId, next) + } + }, + [activeFile?.id, setEditorViewMode, setMarkdownViewMode] + ) + // Why: global Cmd+S (from Terminal.tsx) dispatches this event when // focus is outside the editor content area. Delegate to handleSave // so untitled files still show the rename dialog. @@ -541,6 +646,16 @@ function EditorPanelInner({ for (const file of matchingFiles) { if (file.mode === 'edit' || file.mode === 'markdown-preview') { void loadFileContent(file.filePath, file.id, file.worktreeId) + // Why: if this edit tab is currently in Changes view mode, the + // rendered DiffViewer also depends on the HEAD-side blob. An + // external write (e.g. a git checkout) can change both the working + // tree *and* shift the reference blob, so refetch the diff too. + // Read through a ref so the handler reflects the subscribed store + // value without forcing the listener to re-register on every mode + // toggle. + if (editorViewModeRef.current[file.id] === 'changes') { + void loadDiffContent(file) + } } else if ( file.mode === 'diff' && file.diffSource !== 'combined-uncommitted' && @@ -795,6 +910,9 @@ function EditorPanelInner({ activeFile.diffSource !== undefined && activeFile.diffSource !== 'combined-uncommitted' && activeFile.diffSource !== 'combined-branch' + // Why: Changes view mode renders a DiffViewer, so expose the same inline / + // side-by-side toggle the diff-tab path already offers. + const isDiffSurface = isSingleDiff || isChangesMode const isCombinedDiff = activeFile.mode === 'diff' && (activeFile.diffSource === 'combined-uncommitted' || @@ -869,6 +987,33 @@ function EditorPanelInner({ markdownViewModes.includes(storedMarkdownViewMode) ? storedMarkdownViewMode : defaultMarkdownViewMode + // Why: the header toggle surfaces both the language-specific view mode + // (Source / Rich / Preview) and the orthogonal Changes view mode in one + // segmented control. Plain code files (no language-specific modes) still get + // an Edit | Changes toggle because Changes applies to every editable tab. + const editorToggleModes = getEditorToggleModes({ + language: resolvedLanguage, + mode: activeFile.mode, + diffSource: activeFile.diffSource + }) + const isBinaryEditSurface = + activeFile.mode === 'edit' && fileContents[activeFile.id]?.isBinary === true + // Why: edit-mode binary/image tabs already have their own dedicated renderers + // and cannot enter the Changes diff surface. Hide that segment rather than + // offering a toggle state the renderer will immediately ignore. + const availableEditorToggleModes = isBinaryEditSurface + ? editorToggleModes.filter((mode) => mode !== 'changes') + : editorToggleModes + // Why: a toggle with a single option is just a decorative pill with nothing + // to switch to. Binary plain-code tabs end up here after 'changes' is + // stripped — on main they had no header toggle at all, so requiring >1 mode + // preserves that behavior instead of leaving a lone "Edit" segment. + const hasEditorToggle = availableEditorToggleModes.length > 1 + const effectiveToggleValue: EditorToggleValue = isChangesMode + ? 'changes' + : hasViewModeToggle + ? mdViewMode + : 'edit' const canShowMarkdownPreview = canOpenMarkdownPreview({ language: resolvedLanguage, mode: activeFile.mode, @@ -1021,7 +1166,7 @@ function EditorPanelInner({ )} - {isSingleDiff && ( + {isDiffSurface && ( @@ -1038,11 +1183,11 @@ function EditorPanelInner({ )} - {hasViewModeToggle && ( - setMarkdownViewMode(activeFile.id, mode)} + {hasEditorToggle && ( + )} @@ -1092,6 +1237,7 @@ function EditorPanelInner({ isMermaid={isMermaid} isCsv={isCsv} mdViewMode={mdViewMode} + isChangesMode={isChangesMode} sideBySide={sideBySide} pendingEditorReveal={pendingEditorReveal} handleContentChange={handleContentChange} diff --git a/src/renderer/src/components/editor/EditorViewToggle.tsx b/src/renderer/src/components/editor/EditorViewToggle.tsx new file mode 100644 index 000000000..c84964526 --- /dev/null +++ b/src/renderer/src/components/editor/EditorViewToggle.tsx @@ -0,0 +1,112 @@ +import React from 'react' +import { + Code, + Eye, + FileText, + GitCompareArrows, + Pencil, + Table as TableIcon, + type LucideIcon +} from 'lucide-react' +import { ToggleGroup, ToggleGroupItem } from '@/components/ui/toggle-group' +import type { MarkdownViewMode } from '@/store/slices/editor' + +// Why: 'changes' is not a MarkdownViewMode in the store — it lives on the +// orthogonal editorViewMode slice. This toggle unifies both dimensions into a +// single segmented control because they are mutually exclusive at render time: +// a file can show Source, Rich, Preview, Edit, OR Changes, but never two at +// once. 'edit' is the code-file counterpart to markdown's 'source' — it means +// "the normal editor for this file" without implying the markdown source/raw +// distinction. See reviews/changes-view-mode-plan.md. +export type EditorToggleValue = MarkdownViewMode | 'edit' | 'changes' + +type ViewModeMetadata = { label: string; icon: LucideIcon; title?: string } + +const DEFAULT_VIEW_MODE_METADATA: Record = { + source: { + label: 'Source', + icon: Code + }, + rich: { + label: 'Rich Editor', + icon: Pencil + }, + preview: { + label: 'Preview', + icon: Eye + }, + edit: { + label: 'Edit', + icon: FileText + }, + changes: { + label: 'Changes', + icon: GitCompareArrows, + // Why: "Changes" collides with the Source Control sidebar's "Branch + // Changes" section, which diffs against the base ref. This toggle shows + // uncommitted changes (working tree vs HEAD), so disambiguate in the + // hover title without repeating the button label. + title: 'Uncommitted changes' + } +} + +// Why: CSV/TSV files reuse the 'rich' view mode slot but the rendered surface +// is a read-only table, not an editor. The Pencil icon implies editability, +// which we don't offer, so callers can override the per-mode presentation. +export const CSV_VIEW_MODE_METADATA: Partial> = { + rich: { + label: 'Table', + icon: TableIcon + } +} + +type EditorViewToggleProps = { + value: EditorToggleValue + modes: readonly EditorToggleValue[] + onChange: (value: EditorToggleValue) => void + metadataOverride?: Partial> +} + +export default function EditorViewToggle({ + value, + modes, + onChange, + metadataOverride +}: EditorViewToggleProps): React.JSX.Element { + return ( + { + if (v) { + onChange(v as EditorToggleValue) + } + }} + > + {modes.map((viewMode) => { + // Why: metadataOverride is keyed by MarkdownViewMode (source/rich/preview) + // because only those slots have language-specific presentation variants + // (e.g. CSV's "Table" label on the 'rich' slot). 'edit'/'changes' are + // orthogonal toggle values and always use the default metadata. + const override = ( + metadataOverride as Partial> | undefined + )?.[viewMode] + const metadata = override ?? DEFAULT_VIEW_MODE_METADATA[viewMode] + const Icon = metadata.icon + return ( + + + + ) + })} + + ) +} diff --git a/src/renderer/src/components/editor/MarkdownViewToggle.tsx b/src/renderer/src/components/editor/MarkdownViewToggle.tsx deleted file mode 100644 index 40fb610e5..000000000 --- a/src/renderer/src/components/editor/MarkdownViewToggle.tsx +++ /dev/null @@ -1,75 +0,0 @@ -import React from 'react' -import { Code, Eye, Pencil, Table as TableIcon, type LucideIcon } from 'lucide-react' -import { ToggleGroup, ToggleGroupItem } from '@/components/ui/toggle-group' -import type { MarkdownViewMode } from '@/store/slices/editor' - -type ViewModeMetadata = { label: string; icon: LucideIcon } - -const DEFAULT_VIEW_MODE_METADATA: Record = { - source: { - label: 'Source', - icon: Code - }, - rich: { - label: 'Rich Editor', - icon: Pencil - }, - preview: { - label: 'Preview', - icon: Eye - } -} - -// Why: CSV/TSV files reuse the 'rich' view mode slot but the rendered surface -// is a read-only table, not an editor. The Pencil icon implies editability, -// which we don't offer, so callers can override the per-mode presentation. -export const CSV_VIEW_MODE_METADATA: Partial> = { - rich: { - label: 'Table', - icon: TableIcon - } -} - -type MarkdownViewToggleProps = { - mode: MarkdownViewMode - modes: readonly MarkdownViewMode[] - onChange: (mode: MarkdownViewMode) => void - metadataOverride?: Partial> -} - -export default function MarkdownViewToggle({ - mode, - modes, - onChange, - metadataOverride -}: MarkdownViewToggleProps): React.JSX.Element { - return ( - { - if (v) { - onChange(v as MarkdownViewMode) - } - }} - > - {modes.map((viewMode) => { - const metadata = metadataOverride?.[viewMode] ?? DEFAULT_VIEW_MODE_METADATA[viewMode] - const Icon = metadata.icon - return ( - - - - ) - })} - - ) -} diff --git a/src/renderer/src/components/editor/markdown-preview-controls.ts b/src/renderer/src/components/editor/markdown-preview-controls.ts index 78ad2cff6..a0bff0e99 100644 --- a/src/renderer/src/components/editor/markdown-preview-controls.ts +++ b/src/renderer/src/components/editor/markdown-preview-controls.ts @@ -1,4 +1,5 @@ import type { MarkdownViewMode, OpenFile } from '@/store/slices/editor' +import type { EditorToggleValue } from './EditorViewToggle' type MarkdownPreviewTarget = Pick & { language: string @@ -13,6 +14,25 @@ const MERMAID_VIEW_MODES = ['source', 'rich'] as const satisfies readonly Markdo const CSV_VIEW_MODES = ['source', 'rich'] as const satisfies readonly MarkdownViewMode[] const NO_VIEW_MODES = [] as const satisfies readonly MarkdownViewMode[] +// Why: every editable file (markdown, mermaid, or plain code) can flip into +// Changes view mode. The toggle surfaces this alongside any language-specific +// modes so there is one UI control per pane, not two. Non-edit tabs (diff, +// conflict) do NOT get Changes because they are already a diff/review surface. +// Plain code files have no markdown-style sub-modes, so their toggle is just +// Edit | Changes. +const CODE_EDIT_TOGGLE_MODES = ['edit', 'changes'] as const satisfies readonly EditorToggleValue[] + +export function getEditorToggleModes(target: MarkdownPreviewTarget): readonly EditorToggleValue[] { + if (target.mode !== 'edit') { + return getMarkdownViewModes(target) + } + const languageModes = getMarkdownViewModes(target) + if (languageModes.length > 0) { + return [...languageModes, 'changes'] + } + return CODE_EDIT_TOGGLE_MODES +} + export function getMarkdownViewModes(target: MarkdownPreviewTarget): readonly MarkdownViewMode[] { if (target.language === 'markdown') { if (target.mode === 'edit') { diff --git a/src/renderer/src/components/right-sidebar/SourceControl.tsx b/src/renderer/src/components/right-sidebar/SourceControl.tsx index 1ae4ee2bd..d10e843bf 100644 --- a/src/renderer/src/components/right-sidebar/SourceControl.tsx +++ b/src/renderer/src/components/right-sidebar/SourceControl.tsx @@ -121,6 +121,8 @@ function SourceControlInner(): React.JSX.Element { const revealInExplorer = useAppStore((s) => s.revealInExplorer) const trackConflictPath = useAppStore((s) => s.trackConflictPath) const openDiff = useAppStore((s) => s.openDiff) + const openFile = useAppStore((s) => s.openFile) + const setEditorViewMode = useAppStore((s) => s.setEditorViewMode) const openConflictFile = useAppStore((s) => s.openConflictFile) const openConflictReview = useAppStore((s) => s.openConflictReview) const openBranchDiff = useAppStore((s) => s.openBranchDiff) @@ -341,15 +343,38 @@ function SourceControlInner(): React.JSX.Element { openConflictFile(activeWorktreeId, worktreePath, entry, detectLanguage(entry.path)) return } - openDiff( - activeWorktreeId, - joinPath(worktreePath, entry.path), - entry.path, - detectLanguage(entry.path), - entry.area === 'staged' - ) + const language = detectLanguage(entry.path) + const filePath = joinPath(worktreePath, entry.path) + // Why: unstaged markdown diffs open as a normal edit tab in Changes + // view mode rather than a dedicated diff tab. This unifies sidebar + // clicks with the header's Edit|Changes toggle: there is exactly one + // tab per markdown file, and the sidebar click flips that tab's view + // mode. Staged diffs still open as a separate diff tab because the + // staged content is not what the editor would be editing. Non-markdown + // files keep the existing diff-tab flow until the diff-tab type is + // eventually collapsed (see reviews/changes-view-mode-plan.md §"Follow-up"). + if (language === 'markdown' && entry.area === 'unstaged') { + openFile({ + filePath, + relativePath: entry.path, + worktreeId: activeWorktreeId, + language, + mode: 'edit' + }) + setEditorViewMode(filePath, 'changes') + return + } + openDiff(activeWorktreeId, filePath, entry.path, language, entry.area === 'staged') }, - [activeWorktreeId, worktreePath, trackConflictPath, openConflictFile, openDiff] + [ + activeWorktreeId, + worktreePath, + trackConflictPath, + openConflictFile, + openDiff, + openFile, + setEditorViewMode + ] ) const { selectedKeys, handleSelect, handleContextMenu, clearSelection } = diff --git a/src/renderer/src/store/slices/editor.test.ts b/src/renderer/src/store/slices/editor.test.ts index de06ea87f..48a6bf8e7 100644 --- a/src/renderer/src/store/slices/editor.test.ts +++ b/src/renderer/src/store/slices/editor.test.ts @@ -144,6 +144,50 @@ describe('createEditorSlice markdown view state', () => { }) }) +describe('createEditorSlice editor view mode', () => { + it('stores changes mode as an explicit entry keyed by fileId', () => { + const store = createEditorStore() + + store.getState().setEditorViewMode('/repo/app.ts', 'changes') + + expect(store.getState().editorViewMode).toEqual({ '/repo/app.ts': 'changes' }) + }) + + it('deletes the entry when mode resets to edit', () => { + const store = createEditorStore() + store.getState().setEditorViewMode('/repo/app.ts', 'changes') + + store.getState().setEditorViewMode('/repo/app.ts', 'edit') + + expect(store.getState().editorViewMode).toEqual({}) + }) + + it('is a no-op when resetting a file that was never in changes mode', () => { + const store = createEditorStore() + const before = store.getState().editorViewMode + + store.getState().setEditorViewMode('/repo/app.ts', 'edit') + + expect(store.getState().editorViewMode).toBe(before) + }) + + it('drops editor view mode when the file is closed', () => { + const store = createEditorStore() + store.getState().openFile({ + filePath: '/repo/app.ts', + relativePath: 'app.ts', + worktreeId: 'wt-1', + language: 'typescript', + mode: 'edit' + }) + store.getState().setEditorViewMode('/repo/app.ts', 'changes') + + store.getState().closeFile('/repo/app.ts') + + expect(store.getState().editorViewMode).toEqual({}) + }) +}) + describe('createEditorSlice openMarkdownPreview', () => { it('opens markdown preview as a separate read-only tab', () => { const store = createEditorStore() diff --git a/src/renderer/src/store/slices/editor.ts b/src/renderer/src/store/slices/editor.ts index 909274e54..2a55f4364 100644 --- a/src/renderer/src/store/slices/editor.ts +++ b/src/renderer/src/store/slices/editor.ts @@ -124,6 +124,13 @@ export type ActivityBarPosition = 'top' | 'side' export type MarkdownViewMode = 'source' | 'rich' | 'preview' +// Why: orthogonal to MarkdownViewMode. 'changes' flips the editor tab to a +// diff-against-HEAD rendering (working tree incl. unsaved draft vs HEAD) in +// place of the normal editor, without creating a separate tab. The per-tab +// Tab.contentType stays 'editor' for the whole lifetime; this slice drives +// what EditorPanel *renders* for that tab. See reviews/changes-view-mode-plan.md. +export type EditorViewMode = 'edit' | 'changes' + /** Enough state to restore a tab via `openFile` after `closeFile` (id is always filePath). */ export type ClosedEditorTabSnapshot = Omit @@ -143,6 +150,12 @@ export type EditorSlice = { markdownViewMode: Record setMarkdownViewMode: (fileId: string, mode: MarkdownViewMode) => void + // Editor view mode per file (fileId -> mode). Orthogonal to markdownViewMode: + // a markdown file can be in Raw+Changes, Rendered+Changes, etc. Absent entry + // means 'edit'. + editorViewMode: Record + setEditorViewMode: (fileId: string, mode: EditorViewMode) => void + // Right sidebar rightSidebarOpen: boolean rightSidebarWidth: number @@ -367,6 +380,24 @@ export const createEditorSlice: StateCreator = (s markdownViewMode: { ...s.markdownViewMode, [fileId]: mode } })), + // Editor view mode (edit vs changes-diff). See EditorViewMode. + editorViewMode: {}, + setEditorViewMode: (fileId, mode) => + set((s) => { + // Why: default is 'edit'. Writing 'edit' explicitly when no entry exists + // would grow the record unnecessarily; delete instead so the shape stays + // minimal and hydration round-trips cleanly. + if (mode === 'edit') { + if (!(fileId in s.editorViewMode)) { + return s + } + const next = { ...s.editorViewMode } + delete next[fileId] + return { editorViewMode: next } + } + return { editorViewMode: { ...s.editorViewMode, [fileId]: mode } } + }), + // Right sidebar rightSidebarOpen: false, rightSidebarWidth: 280, @@ -520,6 +551,14 @@ export const createEditorSlice: StateCreator = (s ([fileId]) => fileId !== replacedPreview.id ) ) + const nextEditorViewMode = + replacedPreview.id === id + ? s.editorViewMode + : Object.fromEntries( + Object.entries(s.editorViewMode).filter( + ([fileId]) => fileId !== replacedPreview.id + ) + ) // Why: editorCursorLine entries accumulate per file; clean up the // evicted preview's entry so it does not leak across tab replacements. const nextEditorCursorLine = @@ -566,6 +605,7 @@ export const createEditorSlice: StateCreator = (s editorDrafts: nextEditorDrafts, editorCursorLine: nextEditorCursorLine, markdownViewMode: nextMarkdownViewMode, + editorViewMode: nextEditorViewMode, recentlyClosedEditorTabsByWorktree: nextRecentlyClosed, ...previewTabBarUpdate, ...activeResult @@ -738,6 +778,8 @@ export const createEditorSlice: StateCreator = (s delete newEditorDrafts[fileId] const newMarkdownViewMode = { ...s.markdownViewMode } delete newMarkdownViewMode[fileId] + const newEditorViewMode = { ...s.editorViewMode } + delete newEditorViewMode[fileId] // Why: editorCursorLine entries are keyed by fileId and accumulate on // every cursor move. Without cleanup they grow without bound across a // long session as files are opened and closed. @@ -862,6 +904,7 @@ export const createEditorSlice: StateCreator = (s activeFileIdByWorktree: newActiveFileIdByWorktree, activeTabTypeByWorktree: newActiveTabTypeByWorktree, markdownViewMode: newMarkdownViewMode, + editorViewMode: newEditorViewMode, tabBarOrderByWorktree: nextTabBarOrderByWorktree, pendingEditorReveal: null, recentlyClosedEditorTabsByWorktree: nextRecentlyClosed @@ -948,6 +991,7 @@ export const createEditorSlice: StateCreator = (s activeFileId: null, activeTabType: 'terminal', markdownViewMode: {}, + editorViewMode: {}, pendingEditorReveal: null } } @@ -960,6 +1004,9 @@ export const createEditorSlice: StateCreator = (s const newMarkdownViewMode = Object.fromEntries( Object.entries(s.markdownViewMode).filter(([fileId]) => remainingFileIds.has(fileId)) ) + const newEditorViewMode = Object.fromEntries( + Object.entries(s.editorViewMode).filter(([fileId]) => remainingFileIds.has(fileId)) + ) const newEditorCursorLine = Object.fromEntries( Object.entries(s.editorCursorLine).filter(([fileId]) => remainingFileIds.has(fileId)) ) @@ -1021,6 +1068,7 @@ export const createEditorSlice: StateCreator = (s : s.activeBrowserTabId, activeTabType: browserTabsForWorktree.length > 0 ? 'browser' : 'terminal', markdownViewMode: newMarkdownViewMode, + editorViewMode: newEditorViewMode, activeFileIdByWorktree: newActiveFileIdByWorktree, activeTabTypeByWorktree: newActiveTabTypeByWorktree, tabBarOrderByWorktree: nextTabBarOrderByWorktree, diff --git a/src/renderer/src/store/slices/worktrees.test.ts b/src/renderer/src/store/slices/worktrees.test.ts index 4f5f0b370..dd6bcf23e 100644 --- a/src/renderer/src/store/slices/worktrees.test.ts +++ b/src/renderer/src/store/slices/worktrees.test.ts @@ -49,6 +49,7 @@ function createTestStore() { openFiles: [], editorDrafts: {}, markdownViewMode: {}, + editorViewMode: {}, expandedDirs: {}, gitStatusByWorktree: {}, gitConflictOperationByWorktree: {}, @@ -233,6 +234,35 @@ describe('removeWorktree state cleanup', () => { expect(store.getState().markdownViewMode).toEqual({ 'file-2': 'source' }) }) + it('cleans up editorViewMode for files in the removed worktree', async () => { + const store = createTestStore() + const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) + + store.setState({ + worktreesByRepo: { repo1: [wt] }, + openFiles: [ + { + id: 'file-1', + worktreeId: 'repo1::/path/wt1', + filePath: '/path/wt1/app.ts', + relativePath: 'app.ts', + language: 'typescript', + isDirty: false, + isPreview: false, + mode: 'edit' as const + } + ], + editorViewMode: { + 'file-1': 'changes' as const, + 'file-2': 'changes' as const + } + } as unknown as Partial) + + await store.getState().removeWorktree('repo1::/path/wt1') + + expect(store.getState().editorViewMode).toEqual({ 'file-2': 'changes' }) + }) + it('cleans up expandedDirs for the removed worktree', async () => { const store = createTestStore() const wt = makeWorktree({ id: 'repo1::/path/wt1', repoId: 'repo1', path: '/path/wt1' }) diff --git a/src/renderer/src/store/slices/worktrees.ts b/src/renderer/src/store/slices/worktrees.ts index 69c6f95ce..f88da8d10 100644 --- a/src/renderer/src/store/slices/worktrees.ts +++ b/src/renderer/src/store/slices/worktrees.ts @@ -334,10 +334,13 @@ export const createWorktreeSlice: StateCreator const nextEditorDrafts = removedFileIds.size > 0 ? { ...s.editorDrafts } : s.editorDrafts const nextMarkdownViewMode = removedFileIds.size > 0 ? { ...s.markdownViewMode } : s.markdownViewMode + const nextEditorViewMode = + removedFileIds.size > 0 ? { ...s.editorViewMode } : s.editorViewMode if (removedFileIds.size > 0) { for (const fileId of removedFileIds) { delete nextEditorDrafts[fileId] delete nextMarkdownViewMode[fileId] + delete nextEditorViewMode[fileId] } } const nextExpandedDirs = { ...s.expandedDirs } @@ -382,6 +385,7 @@ export const createWorktreeSlice: StateCreator activeGroupIdByWorktree: nextActiveGroupIdByWorktree, editorDrafts: nextEditorDrafts, markdownViewMode: nextMarkdownViewMode, + editorViewMode: nextEditorViewMode, expandedDirs: nextExpandedDirs, gitStatusByWorktree: nextGitStatusByWorktree, gitConflictOperationByWorktree: nextGitConflictOperationByWorktree,