diff --git a/src-tauri/src/git.rs b/src-tauri/src/git.rs index d7d200c1..af4f0b79 100644 --- a/src-tauri/src/git.rs +++ b/src-tauri/src/git.rs @@ -277,7 +277,10 @@ pub fn git_commit(vault_path: &str, message: &str) -> Result { if !commit.status.success() { let stderr = String::from_utf8_lossy(&commit.stderr); - return Err(format!("git commit failed: {}", stderr)); + let stdout = String::from_utf8_lossy(&commit.stdout); + // git writes "nothing to commit" to stdout, not stderr + let detail = if stderr.trim().is_empty() { stdout } else { stderr }; + return Err(format!("git commit failed: {}", detail.trim())); } Ok(String::from_utf8_lossy(&commit.stdout).to_string()) @@ -543,4 +546,60 @@ mod tests { let log_str = String::from_utf8_lossy(&log.stdout); assert!(log_str.contains("Test commit")); } + + #[test] + fn test_commit_flow_modified_files_then_commit_clears() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + // Create and commit initial file + fs::write(vault.join("flow.md"), "# Original\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + // Modify the file on disk + fs::write(vault.join("flow.md"), "# Modified\n").unwrap(); + + // get_modified_files should detect the change + let modified = get_modified_files(vp).unwrap(); + assert!( + modified.iter().any(|f| f.relative_path == "flow.md"), + "Modified file should be detected after write" + ); + + // Commit the change + let result = git_commit(vp, "update flow").unwrap(); + assert!( + result.contains("1 file changed") || result.contains("flow.md"), + "Commit output should reference the changed file: {}", + result + ); + + // After commit, get_modified_files should return empty + let after = get_modified_files(vp).unwrap(); + assert!( + after.is_empty(), + "No modified files should remain after commit, found: {:?}", + after + ); + } + + #[test] + fn test_commit_nothing_to_commit_returns_error() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + // Create and commit, so working tree is clean + fs::write(vault.join("clean.md"), "# Clean\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + // Committing again with no changes should fail + let result = git_commit(vp, "nothing here"); + assert!(result.is_err(), "Commit should fail when nothing to commit"); + assert!( + result.unwrap_err().contains("nothing to commit"), + "Error should mention 'nothing to commit'" + ); + } } diff --git a/src/App.tsx b/src/App.tsx index 5ff6c969..5f7554ff 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -14,6 +14,7 @@ import { useVaultLoader } from './hooks/useVaultLoader' import { useSettings } from './hooks/useSettings' import { useNoteActions, generateUntitledName } from './hooks/useNoteActions' import { useEditorSave } from './hooks/useEditorSave' +import { useCommitFlow } from './hooks/useCommitFlow' import { useAppKeyboard } from './hooks/useAppKeyboard' import { useViewMode } from './hooks/useViewMode' import { useEntryActions } from './hooks/useEntryActions' @@ -67,7 +68,6 @@ function App() { const [gitHistory, setGitHistory] = useState([]) const [showCreateTypeDialog, setShowCreateTypeDialog] = useState(false) const [showQuickOpen, setShowQuickOpen] = useState(false) - const [showCommitDialog, setShowCommitDialog] = useState(false) const [toastMessage, setToastMessage] = useState(null) const [vaultPath, setVaultPath] = useState(DEFAULT_VAULTS[0].path) const [showAIChat, setShowAIChat] = useState(false) @@ -87,13 +87,20 @@ function App() { const notes = useNoteActions({ addEntry: vault.addEntry, updateContent: vault.updateContent, entries: vault.entries, setToastMessage, updateEntry: vault.updateEntry }) - const { handleSave, handleContentChange, savePendingForPath } = useEditorSave({ + const { handleSave, handleContentChange, savePendingForPath, savePending } = useEditorSave({ updateVaultContent: vault.updateContent, setTabs: notes.setTabs, setToastMessage, onAfterSave: vault.loadModifiedFiles, }) + const commitFlow = useCommitFlow({ + savePending, + loadModifiedFiles: vault.loadModifiedFiles, + commitAndPush: vault.commitAndPush, + setToastMessage, + }) + const entryActions = useEntryActions({ entries: vault.entries, updateEntry: vault.updateEntry, @@ -210,18 +217,6 @@ function App() { onSelectNote: notes.handleSelectNote, }) - const handleCommitPush = useCallback(async (message: string) => { - setShowCommitDialog(false) - try { - const result = await vault.commitAndPush(message) - setToastMessage(result) - vault.loadModifiedFiles() - } catch (err) { - console.error('Commit failed:', err) - setToastMessage(`Commit failed: ${err}`) - } - }, [vault]) - const activeTab = notes.tabs.find((t) => t.entry.path === notes.activeTabPath) ?? null return ( @@ -229,7 +224,7 @@ function App() {
- setShowCommitDialog(true)} /> +
@@ -276,7 +271,7 @@ function App() { setToastMessage(null)} /> setShowQuickOpen(false)} /> setShowCreateTypeDialog(false)} onCreate={handleCreateType} /> - setShowCommitDialog(false)} /> + setShowSettings(false)} /> { + let savePending: vi.Mock + let loadModifiedFiles: vi.Mock + let commitAndPush: vi.Mock + let setToastMessage: vi.Mock + + beforeEach(() => { + savePending = vi.fn().mockResolvedValue(undefined) + loadModifiedFiles = vi.fn().mockResolvedValue(undefined) + commitAndPush = vi.fn().mockResolvedValue('Committed and pushed') + setToastMessage = vi.fn() + }) + + function renderCommitFlow() { + return renderHook(() => useCommitFlow({ savePending, loadModifiedFiles, commitAndPush, setToastMessage })) + } + + it('openCommitDialog saves pending, refreshes files, then opens dialog', async () => { + const { result } = renderCommitFlow() + expect(result.current.showCommitDialog).toBe(false) + + await act(async () => { + await result.current.openCommitDialog() + }) + + expect(savePending).toHaveBeenCalledTimes(1) + expect(loadModifiedFiles).toHaveBeenCalledTimes(1) + expect(result.current.showCommitDialog).toBe(true) + }) + + it('handleCommitPush saves pending, commits, shows toast, and refreshes files', async () => { + const { result } = renderCommitFlow() + + await act(async () => { + await result.current.handleCommitPush('test message') + }) + + expect(savePending).toHaveBeenCalled() + expect(commitAndPush).toHaveBeenCalledWith('test message') + expect(setToastMessage).toHaveBeenCalledWith('Committed and pushed') + expect(loadModifiedFiles).toHaveBeenCalled() + expect(result.current.showCommitDialog).toBe(false) + }) + + it('handleCommitPush shows error toast on failure', async () => { + commitAndPush.mockRejectedValueOnce(new Error('push failed')) + const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}) + const { result } = renderCommitFlow() + + await act(async () => { + await result.current.handleCommitPush('test') + }) + + expect(setToastMessage).toHaveBeenCalledWith(expect.stringContaining('Commit failed')) + consoleSpy.mockRestore() + }) + + it('closeCommitDialog closes the dialog', async () => { + const { result } = renderCommitFlow() + + await act(async () => { + await result.current.openCommitDialog() + }) + expect(result.current.showCommitDialog).toBe(true) + + act(() => { + result.current.closeCommitDialog() + }) + expect(result.current.showCommitDialog).toBe(false) + }) +}) diff --git a/src/hooks/useCommitFlow.ts b/src/hooks/useCommitFlow.ts new file mode 100644 index 00000000..e4d66ba2 --- /dev/null +++ b/src/hooks/useCommitFlow.ts @@ -0,0 +1,36 @@ +import { useCallback, useState } from 'react' + +interface CommitFlowConfig { + savePending: () => Promise + loadModifiedFiles: () => Promise + commitAndPush: (message: string) => Promise + setToastMessage: (msg: string | null) => void +} + +/** Manages the Commit & Push dialog state and the save→commit→push flow. */ +export function useCommitFlow({ savePending, loadModifiedFiles, commitAndPush, setToastMessage }: CommitFlowConfig) { + const [showCommitDialog, setShowCommitDialog] = useState(false) + + const openCommitDialog = useCallback(async () => { + await savePending() + await loadModifiedFiles() + setShowCommitDialog(true) + }, [savePending, loadModifiedFiles]) + + const handleCommitPush = useCallback(async (message: string) => { + setShowCommitDialog(false) + try { + await savePending() + const result = await commitAndPush(message) + setToastMessage(result) + loadModifiedFiles() + } catch (err) { + console.error('Commit failed:', err) + setToastMessage(`Commit failed: ${err}`) + } + }, [savePending, commitAndPush, loadModifiedFiles, setToastMessage]) + + const closeCommitDialog = useCallback(() => setShowCommitDialog(false), []) + + return { showCommitDialog, openCommitDialog, handleCommitPush, closeCommitDialog } +} diff --git a/src/hooks/useEditorSave.test.ts b/src/hooks/useEditorSave.test.ts index 689070c9..75c19d4e 100644 --- a/src/hooks/useEditorSave.test.ts +++ b/src/hooks/useEditorSave.test.ts @@ -155,4 +155,66 @@ describe('useEditorSave', () => { // The ref should hold the latest value — verified via save // (We'll check via the next handleSave call) }) + + it('savePending flushes pending content to disk silently', async () => { + const { result } = renderSaveHook() + + // No pending content — should be a no-op + await act(async () => { + await result.current.savePending() + }) + expect(mockInvokeFn).not.toHaveBeenCalled() + + // Buffer content + act(() => { + result.current.handleContentChange('/test/note.md', 'pending content') + }) + + // savePending should save without toasting + await act(async () => { + await result.current.savePending() + }) + expect(mockInvokeFn).toHaveBeenCalledWith('save_note_content', { + path: '/test/note.md', + content: 'pending content', + }) + expect(setToastMessage).not.toHaveBeenCalled() + + // After savePending, pending is cleared + await act(async () => { + await result.current.savePending() + }) + // No additional save call + expect(mockInvokeFn).toHaveBeenCalledTimes(1) + }) + + it('onAfterSave: called after handleSave but NOT after savePending', async () => { + const onAfterSave = vi.fn() + const { result } = renderHook(() => + useEditorSave({ updateVaultContent, setTabs, setToastMessage, onAfterSave }) + ) + + // savePending does not trigger onAfterSave (callers manage their own refresh) + act(() => { result.current.handleContentChange('/test/note.md', 'v1') }) + await act(async () => { await result.current.savePending() }) + expect(onAfterSave).not.toHaveBeenCalled() + + // handleSave DOES trigger onAfterSave + act(() => { result.current.handleContentChange('/test/note.md', 'v2') }) + await act(async () => { await result.current.handleSave() }) + expect(onAfterSave).toHaveBeenCalledTimes(1) + }) + + it('onAfterSave is NOT called when there is nothing to save', async () => { + const onAfterSave = vi.fn() + const { result } = renderHook(() => + useEditorSave({ updateVaultContent, setTabs, setToastMessage, onAfterSave }) + ) + + await act(async () => { + await result.current.handleSave() + }) + + expect(onAfterSave).not.toHaveBeenCalled() + }) }) diff --git a/src/hooks/useEditorSave.ts b/src/hooks/useEditorSave.ts index 6cf01a79..e6fdb657 100644 --- a/src/hooks/useEditorSave.ts +++ b/src/hooks/useEditorSave.ts @@ -31,23 +31,28 @@ export function useEditorSave({ updateVaultContent, setTabs, setToastMessage, on const { saveNote } = useSaveNote(updateTabAndContent) + /** Persist pending content matching an optional path filter; returns true if saved */ + const flushPending = useCallback(async (pathFilter?: string): Promise => { + const pending = pendingContentRef.current + if (!pending) return false + if (pathFilter && pending.path !== pathFilter) return false + await saveNote(pending.path, pending.content) + pendingContentRef.current = null + return true + }, [saveNote]) + /** Called by Cmd+S — persists the current editor content to disk */ const handleSave = useCallback(async () => { - const pending = pendingContentRef.current - if (!pending) { - setToastMessage('Nothing to save') - return - } try { - await saveNote(pending.path, pending.content) - pendingContentRef.current = null + const saved = await flushPending() + if (!saved) { setToastMessage('Nothing to save'); return } setToastMessage('Saved') onAfterSave?.() } catch (err) { console.error('Save failed:', err) setToastMessage(`Save failed: ${err}`) } - }, [saveNote, setToastMessage, onAfterSave]) + }, [flushPending, setToastMessage, onAfterSave]) /** Called by Editor onChange — buffers the latest content without saving */ const handleContentChange = useCallback((path: string, content: string) => { @@ -55,13 +60,14 @@ export function useEditorSave({ updateVaultContent, setTabs, setToastMessage, on }, []) /** Save pending content for a specific path (used before rename) */ - const savePendingForPath = useCallback(async (path: string): Promise => { - const pending = pendingContentRef.current - if (pending && pending.path === path) { - await saveNote(pending.path, pending.content) - pendingContentRef.current = null - } - }, [saveNote]) + const savePendingForPath = useCallback( + (path: string): Promise => flushPending(path), + [flushPending], + ) - return { handleSave, handleContentChange, savePendingForPath } + /** Flush any pending content to disk silently (used before git commit). + * Does NOT call onAfterSave — callers manage their own refresh. */ + const savePending = useCallback((): Promise => flushPending(), [flushPending]) + + return { handleSave, handleContentChange, savePendingForPath, savePending } } diff --git a/src/mock-tauri.ts b/src/mock-tauri.ts index e9ed8f0c..67ae3aae 100644 --- a/src/mock-tauri.ts +++ b/src/mock-tauri.ts @@ -1700,7 +1700,8 @@ index abc1234..${shortHash} 100644 } let mockHasChanges = true -const mockSavedPaths = new Set() +/** Tracks paths saved since the last mock commit — used by get_modified_files */ +const mockSavedSinceCommit = new Set() let mockSettings: Settings = { anthropic_key: null, @@ -1717,18 +1718,23 @@ const mockHandlers: Record any> = { get_file_history: (args: { path: string }) => mockFileHistory(args.path), get_modified_files: () => { const base = mockHasChanges ? mockModifiedFiles() : [] - const basePaths = new Set(base.map((f) => f.path)) - const extra: ModifiedFile[] = [...mockSavedPaths] - .filter((p) => !basePaths.has(p)) - .map((p) => ({ path: p, relativePath: p.split('/').slice(-2).join('/'), status: 'modified' as const })) + const basePaths = new Set(base.map(f => f.path)) + const extra: ModifiedFile[] = [...mockSavedSinceCommit] + .filter(p => !basePaths.has(p)) + .map(p => ({ + path: p, + relativePath: p.replace(/^.*?\/Laputa\//, ''), + status: 'modified' as const, + })) return [...base, ...extra] }, get_file_diff: (args: { path: string }) => mockFileDiff(args.path), get_file_diff_at_commit: (args: { path: string; commitHash: string }) => mockFileDiffAtCommit(args.path, args.commitHash), git_commit: (args: { message: string }) => { + const count = (mockHasChanges ? mockModifiedFiles().length : 0) + mockSavedSinceCommit.size mockHasChanges = false - mockSavedPaths.clear() - return `[main abc1234] ${args.message}\n 3 files changed` + mockSavedSinceCommit.clear() + return `[main abc1234] ${args.message}\n ${count} files changed` }, git_push: () => { return 'Everything up-to-date' @@ -1752,7 +1758,7 @@ const mockHandlers: Record any> = { }, save_note_content: (args: { path: string; content: string }) => { MOCK_CONTENT[args.path] = args.content - mockSavedPaths.add(args.path) + mockSavedSinceCommit.add(args.path) if (typeof window !== 'undefined') { window.__mockContent = MOCK_CONTENT }