From 23afa89e2ad6182aa76573e9fa2a44176699c3ad Mon Sep 17 00:00:00 2001 From: lucaronin Date: Tue, 24 Feb 2026 22:20:22 +0100 Subject: [PATCH] fix: derive green pallino from git status instead of in-memory tracker The green dot (new note indicator) was disappearing after Cmd+S because markSaved cleared it from the in-memory newPaths set. Now resolveNoteStatus derives "new" status from git status (untracked/added files) so the green dot persists until the note is committed. The in-memory tracker is only used for the brief window between note creation and first save to disk. Co-Authored-By: Claude Opus 4.6 --- src/App.tsx | 2 +- src/components/NoteItem.tsx | 2 +- src/components/TabBar.tsx | 2 +- src/hooks/useEditorSave.test.ts | 14 ++---- src/hooks/useEditorSave.ts | 6 +-- src/hooks/useVaultLoader.test.ts | 81 ++++++++++++++++++++++++-------- src/hooks/useVaultLoader.ts | 18 +++---- 7 files changed, 77 insertions(+), 48 deletions(-) diff --git a/src/App.tsx b/src/App.tsx index 905d6aa8..94be4332 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -87,7 +87,7 @@ function App() { updateVaultContent: vault.updateContent, setTabs: notes.setTabs, setToastMessage, - onAfterSave: vault.loadModifiedFiles, onNoteSaved: vault.markSaved, + onAfterSave: vault.loadModifiedFiles, }) const commitFlow = useCommitFlow({ diff --git a/src/components/NoteItem.tsx b/src/components/NoteItem.tsx index 00329496..ca1be310 100644 --- a/src/components/NoteItem.tsx +++ b/src/components/NoteItem.tsx @@ -47,7 +47,7 @@ function TrashDateLine({ entry }: { entry: VaultEntry }) { } const NOTE_STATUS_DOT: Record = { - new: { color: 'var(--accent-green)', testId: 'new-indicator', title: 'New (unsaved)' }, + new: { color: 'var(--accent-green)', testId: 'new-indicator', title: 'New (uncommitted)' }, modified: { color: 'var(--accent-orange)', testId: 'modified-indicator', title: 'Modified (uncommitted)' }, } diff --git a/src/components/TabBar.tsx b/src/components/TabBar.tsx index 96953ef6..89c46eb0 100644 --- a/src/components/TabBar.tsx +++ b/src/components/TabBar.tsx @@ -174,7 +174,7 @@ function DropIndicator({ side }: { side: 'left' | 'right' }) { } const STATUS_DOT: Record = { - new: { color: 'var(--accent-green)', testId: 'tab-new-indicator', title: 'New (unsaved)' }, + new: { color: 'var(--accent-green)', testId: 'tab-new-indicator', title: 'New (uncommitted)' }, modified: { color: 'var(--accent-orange)', testId: 'tab-modified-indicator', title: 'Modified (uncommitted)' }, } diff --git a/src/hooks/useEditorSave.test.ts b/src/hooks/useEditorSave.test.ts index 6d713dbc..59674ae5 100644 --- a/src/hooks/useEditorSave.test.ts +++ b/src/hooks/useEditorSave.test.ts @@ -107,13 +107,10 @@ describe('useEditorSave', () => { }) }) - it.each([ - { name: 'onAfterSave', key: 'onAfterSave' as const }, - { name: 'onNoteSaved', key: 'onNoteSaved' as const }, - ])('calls $name callback after successful save', async ({ key }) => { + it('calls onAfterSave callback after successful save', async () => { const cb = vi.fn() const { result } = renderHook(() => - useEditorSave({ updateVaultContent, setTabs, setToastMessage, [key]: cb }) + useEditorSave({ updateVaultContent, setTabs, setToastMessage, onAfterSave: cb }) ) act(() => { @@ -142,15 +139,12 @@ describe('useEditorSave', () => { expect(onAfterSave).toHaveBeenCalledOnce() }) - it.each([ - { name: 'onAfterSave', key: 'onAfterSave' as const }, - { name: 'onNoteSaved', key: 'onNoteSaved' as const }, - ])('does not call $name when save fails', async ({ key }) => { + it('does not call onAfterSave when save fails', async () => { mockInvokeFn.mockRejectedValueOnce(new Error('Disk full')) const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) const cb = vi.fn() const { result } = renderHook(() => - useEditorSave({ updateVaultContent, setTabs, setToastMessage, [key]: cb }) + useEditorSave({ updateVaultContent, setTabs, setToastMessage, onAfterSave: cb }) ) act(() => { diff --git a/src/hooks/useEditorSave.ts b/src/hooks/useEditorSave.ts index 2902d9c0..36ca929a 100644 --- a/src/hooks/useEditorSave.ts +++ b/src/hooks/useEditorSave.ts @@ -13,7 +13,6 @@ interface EditorSaveConfig { setTabs: (fn: SetStateAction) => void setToastMessage: (msg: string | null) => void onAfterSave?: () => void - onNoteSaved?: (path: string) => void } /** @@ -22,7 +21,7 @@ interface EditorSaveConfig { */ const noop = () => {} -export function useEditorSave({ updateVaultContent, setTabs, setToastMessage, onAfterSave = noop, onNoteSaved }: EditorSaveConfig) { +export function useEditorSave({ updateVaultContent, setTabs, setToastMessage, onAfterSave = noop }: EditorSaveConfig) { const pendingContentRef = useRef<{ path: string; content: string } | null>(null) const updateTabAndContent = useCallback((path: string, content: string) => { @@ -41,9 +40,8 @@ export function useEditorSave({ updateVaultContent, setTabs, setToastMessage, on if (pathFilter && pending.path !== pathFilter) return false await saveNote(pending.path, pending.content) pendingContentRef.current = null - onNoteSaved?.(pending.path) return true - }, [saveNote, onNoteSaved]) + }, [saveNote]) /** Called by Cmd+S — persists the current editor content to disk */ const handleSave = useCallback(async () => { diff --git a/src/hooks/useVaultLoader.test.ts b/src/hooks/useVaultLoader.test.ts index 73937bdf..42b9ebba 100644 --- a/src/hooks/useVaultLoader.test.ts +++ b/src/hooks/useVaultLoader.test.ts @@ -1,7 +1,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest' import { renderHook, act, waitFor } from '@testing-library/react' import type { VaultEntry, ModifiedFile, GitCommit } from '../types' -import { useVaultLoader } from './useVaultLoader' +import { useVaultLoader, resolveNoteStatus } from './useVaultLoader' const mockEntries: VaultEntry[] = [ { @@ -166,31 +166,42 @@ describe('useVaultLoader', () => { expect(result.current.getNoteStatus('/vault/note/brand-new.md')).toBe('new') }) - it('returns clean after markSaved clears new status', async () => { + it('returns new for git-untracked files (saved but not committed)', async () => { + mockInvokeFn.mockImplementation(((cmd: string) => { + if (cmd === 'list_vault') return Promise.resolve(mockEntries) + if (cmd === 'get_all_content') return Promise.resolve(mockContent) + if (cmd === 'get_modified_files') return Promise.resolve([ + { path: '/vault/note/brand-new.md', relativePath: 'note/brand-new.md', status: 'untracked' }, + ]) + return Promise.resolve(null) + }) as typeof defaultMockInvoke) + const { result } = renderHook(() => useVaultLoader('/vault')) await waitFor(() => { - expect(result.current.entries).toHaveLength(1) - }) - - const newEntry: VaultEntry = { - ...mockEntries[0], - path: '/vault/note/brand-new.md', - filename: 'brand-new.md', - title: 'Brand New', - } - - act(() => { - result.current.addEntry(newEntry, '# Brand New') + expect(result.current.modifiedFiles).toHaveLength(1) }) expect(result.current.getNoteStatus('/vault/note/brand-new.md')).toBe('new') + }) - act(() => { - result.current.markSaved('/vault/note/brand-new.md') + it('returns new for git-added files (staged but not committed)', async () => { + mockInvokeFn.mockImplementation(((cmd: string) => { + if (cmd === 'list_vault') return Promise.resolve(mockEntries) + if (cmd === 'get_all_content') return Promise.resolve(mockContent) + if (cmd === 'get_modified_files') return Promise.resolve([ + { path: '/vault/note/staged.md', relativePath: 'note/staged.md', status: 'added' }, + ]) + return Promise.resolve(null) + }) as typeof defaultMockInvoke) + + const { result } = renderHook(() => useVaultLoader('/vault')) + + await waitFor(() => { + expect(result.current.modifiedFiles).toHaveLength(1) }) - expect(result.current.getNoteStatus('/vault/note/brand-new.md')).toBe('clean') + expect(result.current.getNoteStatus('/vault/note/staged.md')).toBe('new') }) it('new status takes priority over git modified', async () => { @@ -224,7 +235,7 @@ describe('useVaultLoader', () => { expect(result.current.getNoteStatus('/vault/note/new.md')).toBe('new') }) - it('ignores untracked git status for orange dot', async () => { + it('treats untracked files as new (green dot, not orange)', async () => { mockInvokeFn.mockImplementation(((cmd: string) => { if (cmd === 'list_vault') return Promise.resolve(mockEntries) if (cmd === 'get_all_content') return Promise.resolve(mockContent) @@ -240,7 +251,7 @@ describe('useVaultLoader', () => { expect(result.current.modifiedFiles).toHaveLength(1) }) - expect(result.current.getNoteStatus('/vault/note/hello.md')).toBe('clean') + expect(result.current.getNoteStatus('/vault/note/hello.md')).toBe('new') }) }) @@ -350,3 +361,35 @@ describe('useVaultLoader', () => { }) }) }) + +describe('resolveNoteStatus', () => { + const mf = (path: string, status: string): ModifiedFile => ({ path, relativePath: path.replace('/vault/', ''), status }) + + it('returns new when path is in newPaths (not yet on disk)', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(['/vault/x.md']), [])).toBe('new') + }) + + it('returns new for untracked files in git', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(), [mf('/vault/x.md', 'untracked')])).toBe('new') + }) + + it('returns new for added files in git', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(), [mf('/vault/x.md', 'added')])).toBe('new') + }) + + it('returns modified for git-modified files', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(), [mf('/vault/x.md', 'modified')])).toBe('modified') + }) + + it('returns clean for files not in git status', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(), [])).toBe('clean') + }) + + it('returns clean for deleted files', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(), [mf('/vault/x.md', 'deleted')])).toBe('clean') + }) + + it('newPaths takes priority over git modified', () => { + expect(resolveNoteStatus('/vault/x.md', new Set(['/vault/x.md']), [mf('/vault/x.md', 'modified')])).toBe('new') + }) +}) diff --git a/src/hooks/useVaultLoader.ts b/src/hooks/useVaultLoader.ts index ae111b89..5bcf3b92 100644 --- a/src/hooks/useVaultLoader.ts +++ b/src/hooks/useVaultLoader.ts @@ -37,23 +37,17 @@ function useNewNoteTracker() { setNewPaths((prev) => new Set(prev).add(path)) }, []) - const markSaved = useCallback((path: string) => { - setNewPaths((prev) => { - if (!prev.has(path)) return prev - const next = new Set(prev) - next.delete(path) - return next - }) - }, []) - const clear = useCallback(() => setNewPaths(new Set()), []) - return { newPaths, trackNew, markSaved, clear } + return { newPaths, trackNew, clear } } export function resolveNoteStatus(path: string, newPaths: Set, modifiedFiles: ModifiedFile[]): NoteStatus { if (newPaths.has(path)) return 'new' - if (modifiedFiles.some((f) => f.path === path && f.status === 'modified')) return 'modified' + const gitEntry = modifiedFiles.find((f) => f.path === path) + if (!gitEntry) return 'clean' + if (gitEntry.status === 'untracked' || gitEntry.status === 'added') return 'new' + if (gitEntry.status === 'modified') return 'modified' return 'clean' } @@ -120,6 +114,6 @@ export function useVaultLoader(vaultPath: string) { entries, allContent, modifiedFiles, addEntry, updateEntry, replaceEntry, updateContent, loadModifiedFiles, loadGitHistory, loadDiff, loadDiffAtCommit, - getNoteStatus, markSaved: tracker.markSaved, commitAndPush, + getNoteStatus, commitAndPush, } }