From a007b37089176709df5fcf450eaf9ff213c5280d Mon Sep 17 00:00:00 2001 From: Test Date: Sat, 4 Apr 2026 18:16:52 +0200 Subject: [PATCH] feat: add per-file discard changes in Changes view Right-click a file in the Changes view to discard its uncommitted changes. Shows a confirmation dialog before executing. Handles modified (git checkout), untracked (delete), and deleted (git restore) files. Includes path traversal safety checks in the Rust backend. Co-Authored-By: Claude Opus 4.6 (1M context) --- src-tauri/src/commands/git.rs | 13 +++ src-tauri/src/git/mod.rs | 2 +- src-tauri/src/git/status.rs | 152 +++++++++++++++++++++++++++++++ src-tauri/src/lib.rs | 1 + src/App.tsx | 16 +++- src/components/NoteItem.tsx | 4 +- src/components/NoteList.test.tsx | 58 ++++++++++++ src/components/NoteList.tsx | 75 ++++++++++++++- src/mock-tauri/mock-handlers.ts | 1 + 9 files changed, 315 insertions(+), 7 deletions(-) diff --git a/src-tauri/src/commands/git.rs b/src-tauri/src/commands/git.rs index ff5cb8a8..290e6f83 100644 --- a/src-tauri/src/commands/git.rs +++ b/src-tauri/src/commands/git.rs @@ -128,6 +128,13 @@ pub async fn git_remote_status(vault_path: String) -> Result Result<(), String> { + let vault_path = expand_tilde(&vault_path); + crate::git::discard_file_changes(&vault_path, &relative_path) +} + #[cfg(desktop)] #[tauri::command] pub fn is_git_repo(vault_path: String) -> bool { @@ -247,6 +254,12 @@ pub async fn git_remote_status(_vault_path: String) -> Result Result<(), String> { + Err("Git discard is not available on mobile".into()) +} + #[cfg(mobile)] #[tauri::command] pub fn is_git_repo(_vault_path: String) -> bool { diff --git a/src-tauri/src/git/mod.rs b/src-tauri/src/git/mod.rs index ae858dcf..5bc6881d 100644 --- a/src-tauri/src/git/mod.rs +++ b/src-tauri/src/git/mod.rs @@ -21,7 +21,7 @@ pub use remote::{ git_pull, git_push, git_remote_status, has_remote, GitPullResult, GitPushResult, GitRemoteStatus, }; -pub use status::{get_modified_files, ModifiedFile}; +pub use status::{discard_file_changes, get_modified_files, ModifiedFile}; use serde::Serialize; diff --git a/src-tauri/src/git/status.rs b/src-tauri/src/git/status.rs index 69264f42..065a1068 100644 --- a/src-tauri/src/git/status.rs +++ b/src-tauri/src/git/status.rs @@ -63,6 +63,80 @@ pub fn get_modified_files(vault_path: &str) -> Result, String> Ok(files) } +/// Discard uncommitted changes to a single file. +/// +/// - **Modified / Deleted**: `git checkout -- ` restores the last committed version. +/// - **Untracked / Added**: the file is removed from disk. +/// +/// The `relative_path` must be relative to `vault_path` (the same format +/// returned by [`get_modified_files`]). +pub fn discard_file_changes(vault_path: &str, relative_path: &str) -> Result<(), String> { + let vault = Path::new(vault_path); + let abs = vault.join(relative_path); + + // Safety: ensure the resolved path stays inside the vault. + // Safety: reject any relative_path that tries to escape the vault via `..`. + for component in std::path::Path::new(relative_path).components() { + if matches!(component, std::path::Component::ParentDir) { + return Err("File path is outside the vault".into()); + } + } + if abs.exists() { + let canonical_vault = vault + .canonicalize() + .map_err(|e| format!("Cannot resolve vault path: {e}"))?; + let canonical_file = abs + .canonicalize() + .map_err(|e| format!("Cannot resolve file path: {e}"))?; + if !canonical_file.starts_with(&canonical_vault) { + return Err("File path is outside the vault".into()); + } + } + + // Determine the file status from `git status --porcelain`. + let output = Command::new("git") + .args(["status", "--porcelain", "--", relative_path]) + .current_dir(vault) + .output() + .map_err(|e| format!("Failed to run git status: {e}"))?; + + let stdout = String::from_utf8_lossy(&output.stdout); + let line = stdout.lines().find(|l| l.len() >= 4); + + let status_code = line + .map(|l| l[..2].trim().to_string()) + .unwrap_or_default(); + + match status_code.as_str() { + "??" => { + // Untracked — remove from disk. + std::fs::remove_file(&abs) + .map_err(|e| format!("Failed to delete untracked file: {e}"))?; + } + _ => { + // Modified, deleted, added-to-index, renamed, etc. — restore via git. + // Unstage first (ignore errors — file might not be staged). + let _ = Command::new("git") + .args(["reset", "HEAD", "--", relative_path]) + .current_dir(vault) + .output(); + + let checkout = Command::new("git") + .args(["checkout", "--", relative_path]) + .current_dir(vault) + .output() + .map_err(|e| format!("Failed to run git checkout: {e}"))?; + + if !checkout.status.success() { + let stderr = String::from_utf8_lossy(&checkout.stderr); + return Err(format!("git checkout failed: {}", stderr.trim())); + } + } + } + + Ok(()) +} + #[cfg(test)] mod tests { use super::*; @@ -172,4 +246,82 @@ mod tests { after ); } + + #[test] + fn test_discard_modified_file() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + fs::write(vault.join("note.md"), "# Original\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + // Modify the file + fs::write(vault.join("note.md"), "# Changed\n").unwrap(); + assert_eq!(get_modified_files(vp).unwrap().len(), 1); + + // Discard + discard_file_changes(vp, "note.md").unwrap(); + + let content = fs::read_to_string(vault.join("note.md")).unwrap(); + assert_eq!(content, "# Original\n"); + assert!(get_modified_files(vp).unwrap().is_empty()); + } + + #[test] + fn test_discard_untracked_file() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + fs::write(vault.join("init.md"), "# Init\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + // Create an untracked file + fs::write(vault.join("new.md"), "# New\n").unwrap(); + assert!(vault.join("new.md").exists()); + + discard_file_changes(vp, "new.md").unwrap(); + + assert!(!vault.join("new.md").exists()); + assert!(get_modified_files(vp).unwrap().is_empty()); + } + + #[test] + fn test_discard_deleted_file() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + fs::write(vault.join("note.md"), "# Original\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + // Delete the file + fs::remove_file(vault.join("note.md")).unwrap(); + assert!(!vault.join("note.md").exists()); + + discard_file_changes(vp, "note.md").unwrap(); + + assert!(vault.join("note.md").exists()); + let content = fs::read_to_string(vault.join("note.md")).unwrap(); + assert_eq!(content, "# Original\n"); + } + + #[test] + fn test_discard_rejects_path_outside_vault() { + let dir = setup_git_repo(); + let vault = dir.path(); + let vp = vault.to_str().unwrap(); + + // Need an initial commit so git status works + fs::write(vault.join("init.md"), "# Init\n").unwrap(); + git_commit(vp, "initial").unwrap(); + + let result = discard_file_changes(vp, "../../../etc/passwd"); + assert!(result.is_err(), "Should reject path outside vault, got: {:?}", result); + assert!( + result.unwrap_err().contains("outside the vault"), + "Error should mention 'outside the vault'" + ); + } } diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 7b6187e6..a5b4ee1f 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -140,6 +140,7 @@ pub fn run() { commands::get_conflict_mode, commands::git_resolve_conflict, commands::git_commit_conflict_resolution, + commands::git_discard_file, commands::is_git_repo, commands::init_git_repo, commands::check_claude_cli, diff --git a/src/App.tsx b/src/App.tsx index b976fe38..db221bcc 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -312,6 +312,20 @@ function App() { openNoteInNewWindow(entry.path, resolvedPath, entry.title) }, [resolvedPath]) + const handleDiscardFile = useCallback(async (relativePath: string) => { + try { + if (isTauri()) { + await invoke('git_discard_file', { vaultPath: resolvedPath, relativePath }) + } else { + await mockInvoke('git_discard_file', { vaultPath: resolvedPath, relativePath }) + } + await vault.loadModifiedFiles() + await vault.reloadVault() + } catch (err) { + setToastMessage(typeof err === 'string' ? err : 'Failed to discard changes') + } + }, [resolvedPath, vault, setToastMessage]) + const commitFlow = useCommitFlow({ savePending: appSave.savePending, loadModifiedFiles: vault.loadModifiedFiles, commitAndPush: vault.commitAndPush, setToastMessage, onPushRejected: autoSync.handlePushRejected }) const suggestedCommitMessage = useMemo(() => generateCommitMessage(vault.modifiedFiles), [vault.modifiedFiles]) @@ -579,7 +593,7 @@ function App() { {selection.kind === 'filter' && selection.filter === 'pulse' ? ( setViewMode('all')} /> ) : ( - + )} diff --git a/src/components/NoteItem.tsx b/src/components/NoteItem.tsx index 57edd51a..20fd0084 100644 --- a/src/components/NoteItem.tsx +++ b/src/components/NoteItem.tsx @@ -152,7 +152,7 @@ function getFileKindIcon(fileKind: string | undefined): ComponentType onClickNote: (entry: VaultEntry, e: React.MouseEvent) => void onPrefetch?: (path: string) => void + onContextMenu?: (entry: VaultEntry, e: React.MouseEvent) => void }) { const isBinary = entry.fileKind === 'binary' const isNonMarkdown = !!entry.fileKind && entry.fileKind !== 'markdown' @@ -187,6 +188,7 @@ export function NoteItem({ entry, isSelected, isMultiSelected = false, isHighlig )} style={isBinary ? { padding: '14px 16px' } : noteItemStyle(isSelected, isMultiSelected, typeColor, typeLightColor)} onClick={handleClick} + onContextMenu={onContextMenu ? (e) => onContextMenu(entry, e) : undefined} onMouseEnter={!isBinary && onPrefetch ? () => onPrefetch(entry.path) : undefined} data-testid={isMultiSelected ? 'multi-selected-item' : isBinary ? 'binary-file-item' : undefined} data-highlighted={isHighlighted || undefined} diff --git a/src/components/NoteList.test.tsx b/src/components/NoteList.test.tsx index 2ae38983..b29e0e31 100644 --- a/src/components/NoteList.test.tsx +++ b/src/components/NoteList.test.tsx @@ -1081,6 +1081,64 @@ describe('NoteList — virtual list with large datasets', () => { ) expect(screen.queryByText(/notes? deleted/)).not.toBeInTheDocument() }) + + it('shows context menu with "Discard changes" on right-click when onDiscardFile is provided', () => { + const onDiscard = vi.fn() + render( + + ) + const noteItem = screen.getByText('Build Laputa App').closest('[class*="border-b"]')! + fireEvent.contextMenu(noteItem) + expect(screen.getByTestId('changes-context-menu')).toBeInTheDocument() + expect(screen.getByTestId('discard-changes-button')).toBeInTheDocument() + }) + + it('does not show context menu when onDiscardFile is not provided', () => { + render( + + ) + const noteItem = screen.getByText('Build Laputa App').closest('[class*="border-b"]')! + fireEvent.contextMenu(noteItem) + expect(screen.queryByTestId('changes-context-menu')).not.toBeInTheDocument() + }) + + it('shows confirmation dialog after clicking "Discard changes"', () => { + const onDiscard = vi.fn() + render( + + ) + const noteItem = screen.getByText('Build Laputa App').closest('[class*="border-b"]')! + fireEvent.contextMenu(noteItem) + fireEvent.click(screen.getByTestId('discard-changes-button')) + expect(screen.getByTestId('discard-confirm-dialog')).toBeInTheDocument() + // Dialog mentions the note title + const dialog = screen.getByTestId('discard-confirm-dialog') + expect(dialog.textContent).toContain('Build Laputa App') + }) + + it('calls onDiscardFile with relativePath when discard is confirmed', async () => { + const onDiscard = vi.fn().mockResolvedValue(undefined) + render( + + ) + const noteItem = screen.getByText('Build Laputa App').closest('[class*="border-b"]')! + fireEvent.contextMenu(noteItem) + fireEvent.click(screen.getByTestId('discard-changes-button')) + fireEvent.click(screen.getByTestId('discard-confirm-button')) + expect(onDiscard).toHaveBeenCalledWith('project/26q1-laputa-app.md') + }) + + it('does not call onDiscardFile when cancel is clicked', () => { + const onDiscard = vi.fn() + render( + + ) + const noteItem = screen.getByText('Build Laputa App').closest('[class*="border-b"]')! + fireEvent.contextMenu(noteItem) + fireEvent.click(screen.getByTestId('discard-changes-button')) + fireEvent.click(screen.getByText('Cancel')) + expect(onDiscard).not.toHaveBeenCalled() + }) }) }) diff --git a/src/components/NoteList.tsx b/src/components/NoteList.tsx index 93693f32..25547b62 100644 --- a/src/components/NoteList.tsx +++ b/src/components/NoteList.tsx @@ -1,4 +1,4 @@ -import { useState, useMemo, useCallback, useEffect, memo } from 'react' +import { useState, useMemo, useCallback, useEffect, useRef, memo } from 'react' import type { VaultEntry, SidebarSelection, ModifiedFile, NoteStatus, InboxPeriod, ViewFile } from '../types' import type { NoteListFilter } from '../utils/noteListHelpers' import { countByFilter, countAllByFilter } from '../utils/noteListHelpers' @@ -16,6 +16,11 @@ import { useTypeEntryMap, useNoteListData, useNoteListSearch, useNoteListSort, useMultiSelectKeyboard, useModifiedFilesState, } from './note-list/noteListHooks' +import { + Dialog, DialogContent, DialogHeader, DialogTitle, + DialogDescription, DialogFooter, +} from '@/components/ui/dialog' +import { Button } from '@/components/ui/button' interface NoteListProps { entries: VaultEntry[] @@ -40,10 +45,11 @@ interface NoteListProps { onUpdateTypeSort?: (path: string, key: string, value: string | number | boolean | string[] | null) => void updateEntry?: (path: string, patch: Partial) => void onOpenInNewWindow?: (entry: VaultEntry) => void + onDiscardFile?: (relativePath: string) => Promise views?: ViewFile[] } -function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNoteListFilterChange, inboxPeriod = 'all', modifiedFiles, modifiedFilesError, getNoteStatus, sidebarCollapsed, onSelectNote, onReplaceActiveTab, onCreateNote, onBulkArchive, onBulkTrash, onBulkRestore, onBulkDeletePermanently, onEmptyTrash, onUpdateTypeSort, updateEntry, onOpenInNewWindow, views }: NoteListProps) { +function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNoteListFilterChange, inboxPeriod = 'all', modifiedFiles, modifiedFilesError, getNoteStatus, sidebarCollapsed, onSelectNote, onReplaceActiveTab, onCreateNote, onBulkArchive, onBulkTrash, onBulkRestore, onBulkDeletePermanently, onEmptyTrash, onUpdateTypeSort, updateEntry, onOpenInNewWindow, onDiscardFile, views }: NoteListProps) { const { modifiedPathSet, modifiedSuffixes, resolvedGetNoteStatus } = useModifiedFilesState(modifiedFiles, getNoteStatus) const isSectionGroup = selection.kind === 'sectionGroup' @@ -88,9 +94,41 @@ function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNot const bulkTrashOrDelete = isTrashView ? handleBulkDeletePermanently : handleBulkTrash useMultiSelectKeyboard(multiSelect, isEntityView, bulkArchiveOrRestore, bulkTrashOrDelete) + // ── Changes view: context menu + discard confirmation ── + const [ctxMenu, setCtxMenu] = useState<{ x: number; y: number; entry: VaultEntry } | null>(null) + const [discardTarget, setDiscardTarget] = useState(null) + const ctxMenuRef = useRef(null) + + const handleNoteContextMenu = useCallback((entry: VaultEntry, e: React.MouseEvent) => { + if (!isChangesView || !onDiscardFile) return + e.preventDefault() + e.stopPropagation() + setCtxMenu({ x: e.clientX, y: e.clientY, entry }) + }, [isChangesView, onDiscardFile]) + + const closeCtxMenu = useCallback(() => setCtxMenu(null), []) + + // Close context menu on outside click + useEffect(() => { + if (!ctxMenu) return + const handler = (e: MouseEvent) => { + if (ctxMenuRef.current && !ctxMenuRef.current.contains(e.target as Node)) closeCtxMenu() + } + document.addEventListener('mousedown', handler) + return () => document.removeEventListener('mousedown', handler) + }, [ctxMenu, closeCtxMenu]) + + const handleDiscardConfirm = useCallback(async () => { + if (!discardTarget || !onDiscardFile) return + const mf = modifiedFiles?.find((f) => f.path === discardTarget.path) + if (!mf) return + await onDiscardFile(mf.relativePath) + setDiscardTarget(null) + }, [discardTarget, onDiscardFile, modifiedFiles]) + const renderItem = useCallback((entry: VaultEntry) => ( - - ), [selectedNote?.path, handleClickNote, typeEntryMap, resolvedGetNoteStatus, multiSelect.selectedPaths, noteListKeyboard.highlightedPath]) + + ), [selectedNote?.path, handleClickNote, typeEntryMap, resolvedGetNoteStatus, multiSelect.selectedPaths, noteListKeyboard.highlightedPath, isChangesView, onDiscardFile, handleNoteContextMenu]) const handleCreateNote = useCallback(() => { onCreateNote(selection.kind === 'sectionGroup' ? selection.type : undefined) @@ -115,6 +153,35 @@ function NoteListInner({ entries, selection, selectedNote, noteListFilter, onNot {multiSelect.isMultiSelecting && ( )} + + {/* Changes view: context menu */} + {ctxMenu && ( +
+ +
+ )} + + {/* Discard confirmation dialog */} + { if (!open) setDiscardTarget(null) }}> + + + Discard changes + + Discard changes to {discardTarget?.title ?? 'this file'}? This cannot be undone. + + + + + + + + ) } diff --git a/src/mock-tauri/mock-handlers.ts b/src/mock-tauri/mock-handlers.ts index 293e963a..3d80a299 100644 --- a/src/mock-tauri/mock-handlers.ts +++ b/src/mock-tauri/mock-handlers.ts @@ -155,6 +155,7 @@ export const mockHandlers: Record any> = { }, 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_discard_file: () => {}, git_commit: (args: { message: string }) => { const count = (mockHasChanges ? mockModifiedFiles().length : 0) + mockSavedSinceCommit.size mockHasChanges = false