fix: commit & push now saves pending content and refreshes modified files
Root cause: Two problems caused "0 files changed": 1. loadModifiedFiles() was only called on mount, never refreshed after saves 2. Pending editor content wasn't flushed to disk before git commit Fix: - useEditorSave: add savePending() to flush unsaved content, onAfterSave callback to refresh git status after Cmd+S - useCommitFlow: new hook managing save→commit→push flow with proper sequencing (save pending → refresh files → show dialog → commit) - App.tsx: wire onAfterSave to loadModifiedFiles, use useCommitFlow - git.rs: include stdout in error when stderr is empty (fixes "nothing to commit" message being swallowed) - mock-tauri: track saved files so get_modified_files reflects edits Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -277,7 +277,10 @@ pub fn git_commit(vault_path: &str, message: &str) -> Result<String, String> {
|
||||
|
||||
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'"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
27
src/App.tsx
27
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<GitCommit[]>([])
|
||||
const [showCreateTypeDialog, setShowCreateTypeDialog] = useState(false)
|
||||
const [showQuickOpen, setShowQuickOpen] = useState(false)
|
||||
const [showCommitDialog, setShowCommitDialog] = useState(false)
|
||||
const [toastMessage, setToastMessage] = useState<string | null>(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() {
|
||||
<UpdateBanner status={updateStatus} actions={updateActions} />
|
||||
<div className="app">
|
||||
<div className="app__sidebar" style={{ width: layout.sidebarWidth }}>
|
||||
<Sidebar entries={vault.entries} selection={selection} onSelect={setSelection} onSelectNote={notes.handleSelectNote} onCreateType={handleCreateNoteImmediate} onCreateNewType={openCreateTypeDialog} onCustomizeType={entryActions.handleCustomizeType} onReorderSections={entryActions.handleReorderSections} modifiedCount={vault.modifiedFiles.length} onCommitPush={() => setShowCommitDialog(true)} />
|
||||
<Sidebar entries={vault.entries} selection={selection} onSelect={setSelection} onSelectNote={notes.handleSelectNote} onCreateType={handleCreateNoteImmediate} onCreateNewType={openCreateTypeDialog} onCustomizeType={entryActions.handleCustomizeType} onReorderSections={entryActions.handleReorderSections} modifiedCount={vault.modifiedFiles.length} onCommitPush={commitFlow.openCommitDialog} />
|
||||
</div>
|
||||
<ResizeHandle onResize={layout.handleSidebarResize} />
|
||||
<div className="app__note-list" style={{ width: layout.noteListWidth }}>
|
||||
@@ -276,7 +271,7 @@ function App() {
|
||||
<Toast message={toastMessage} onDismiss={() => setToastMessage(null)} />
|
||||
<QuickOpenPalette open={showQuickOpen} entries={vault.entries} onSelect={notes.handleSelectNote} onClose={() => setShowQuickOpen(false)} />
|
||||
<CreateTypeDialog open={showCreateTypeDialog} onClose={() => setShowCreateTypeDialog(false)} onCreate={handleCreateType} />
|
||||
<CommitDialog open={showCommitDialog} modifiedCount={vault.modifiedFiles.length} onCommit={handleCommitPush} onClose={() => setShowCommitDialog(false)} />
|
||||
<CommitDialog open={commitFlow.showCommitDialog} modifiedCount={vault.modifiedFiles.length} onCommit={commitFlow.handleCommitPush} onClose={commitFlow.closeCommitDialog} />
|
||||
<SettingsPanel open={showSettings} settings={settings} onSave={saveSettings} onClose={() => setShowSettings(false)} />
|
||||
<GitHubVaultModal
|
||||
open={showGitHubVault}
|
||||
|
||||
75
src/hooks/useCommitFlow.test.ts
Normal file
75
src/hooks/useCommitFlow.test.ts
Normal file
@@ -0,0 +1,75 @@
|
||||
import { describe, it, expect, vi, beforeEach } from 'vitest'
|
||||
import { renderHook, act } from '@testing-library/react'
|
||||
import { useCommitFlow } from './useCommitFlow'
|
||||
|
||||
describe('useCommitFlow', () => {
|
||||
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)
|
||||
})
|
||||
})
|
||||
36
src/hooks/useCommitFlow.ts
Normal file
36
src/hooks/useCommitFlow.ts
Normal file
@@ -0,0 +1,36 @@
|
||||
import { useCallback, useState } from 'react'
|
||||
|
||||
interface CommitFlowConfig {
|
||||
savePending: () => Promise<void>
|
||||
loadModifiedFiles: () => Promise<void>
|
||||
commitAndPush: (message: string) => Promise<string>
|
||||
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 }
|
||||
}
|
||||
@@ -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()
|
||||
})
|
||||
})
|
||||
|
||||
@@ -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<boolean> => {
|
||||
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<void> => {
|
||||
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<boolean> => 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<boolean> => flushPending(), [flushPending])
|
||||
|
||||
return { handleSave, handleContentChange, savePendingForPath, savePending }
|
||||
}
|
||||
|
||||
@@ -1700,7 +1700,8 @@ index abc1234..${shortHash} 100644
|
||||
}
|
||||
|
||||
let mockHasChanges = true
|
||||
const mockSavedPaths = new Set<string>()
|
||||
/** Tracks paths saved since the last mock commit — used by get_modified_files */
|
||||
const mockSavedSinceCommit = new Set<string>()
|
||||
|
||||
let mockSettings: Settings = {
|
||||
anthropic_key: null,
|
||||
@@ -1717,18 +1718,23 @@ const mockHandlers: Record<string, (args: any) => 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<string, (args: any) => 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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user