better git ui and management#8
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds backend git commands and StatusResult shape, selective async file-watching with generation tracking, virtualized Git file-tree and context menus, debounced GitChanges loading with discard-all, workspace worktree tick support, and Tauri capability/config updates. ChangesGit Operations and File Management
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/commands/git.rs`:
- Around line 746-768: open_file_default contains a command-injection risk on
Windows because it shells the unescaped path through cmd; replace the manual
platform-specific Command::new(...) calls inside the open_file_default function
with the tauri-plugin-opener API (use its open function that accepts the path
and safely handles escaping across platforms) so you no longer invoke
cmd/xdg-open/open directly; if you cannot use tauri-plugin-opener for some
reason, validate the incoming path in open_file_default to ensure it is a real
file path (reject null bytes, shell metacharacters like &, |, >, ;, &&, ||) and
use std::fs and platform-safe launching APIs rather than passing the raw string
to a shell.
In `@src-tauri/src/commands/watcher.rs`:
- Around line 67-97: The current setup_watches only registers
RecursiveMode::Recursive for existing depth-2 dirs, so new folders created later
are never watched; update setup_watches to register recursive watches for each
depth-1 directory (the debouncer.watch call for variable d1) instead of
NonRecursive so new subdirectories created under d1 are covered, keeping the
is_skip_dir checks; alternatively (if you prefer finer control) add logic in
your watcher event loop to handle Create directory events and call
debouncer.watch(new_path, RecursiveMode::Recursive) for the created dirs—refer
to setup_watches, debouncer.watch, and is_skip_dir to locate and implement this
change.
- Around line 117-172: The spawned watcher thread can overwrite shared watcher
state and emit for the wrong project if a newer unwatch/watch happened; fix by
binding a unique watcher/session id or the canonical project_path snapshot when
creating the debouncer (use project_path or a generated u64 token) and before
calling setup_watches, before emitting via app_for_events.emit, and before
storing into watcher_state (WatcherState) compare the current global expected
id/path in app_clone.state with the thread's id/path and abort if they differ;
ensure new_debouncer, setup_watches, and the emission/store paths all check this
identifier so stale threads neither overwrite the Option<debouncer> nor emit
events for a different project.
In `@src/lib/components/panels/GitChanges.svelte`:
- Around line 220-222: The displayed "staged" and "changed" counts are
misleading when totalFiles > RUST_LIMIT because stagedCount is computed from the
truncated files array; update the UI in GitChanges.svelte to either compute
counts from the full dataset if available or mark them as approximate when
files.length < totalFiles: use files, stagedCount and totalFiles to detect
truncation (files.length < totalFiles) and then render a clear indication such
as "≈ {stagedCount} staged · ≈ {totalFiles - stagedCount} changed" or "
{stagedCount} staged (showing {files.length} of {totalFiles})" instead of the
current literal counts; apply the same change to the discard confirmation dialog
code that references stagedCount/totalFiles (the dialog block around line ~329)
so both places consistently show accurate-or-approximate counts.
In `@src/lib/components/panels/GitFileTree.svelte`:
- Line 205: The current {`#each` visible as { node, depth } (node.path + depth)}
key risks collisions; change the key expression in the each block to a
deterministic concatenation that cannot ambigously join path and depth (for
example use a template string with a clear separator like
`${node.path}::${depth}`) or, if available, switch to a unique identifier such
as node.id; update the each block key accordingly in GitFileTree.svelte so the
key uses the new non-ambiguous expression instead of node.path + depth.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 73ac72dd-60dd-4f5b-ab66-37c4ef1040cb
⛔ Files ignored due to path filters (18)
bun.lockis excluded by!**/*.locksrc/lib/components/ui/context-menu/context-menu-checkbox-item.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-content.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-group-heading.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-group.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-item.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-label.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-portal.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-radio-group.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-radio-item.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-separator.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-shortcut.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-sub-content.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-sub-trigger.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-sub.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu-trigger.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/context-menu.svelteis excluded by!src/lib/components/ui/**src/lib/components/ui/context-menu/index.jsis excluded by!src/lib/components/ui/**
📒 Files selected for processing (13)
.claude/settings.jsonpackage.jsonsrc-tauri/capabilities/default.jsonsrc-tauri/capabilities/folder-windows.jsonsrc-tauri/src/commands/git.rssrc-tauri/src/commands/watcher.rssrc-tauri/src/lib.rssrc/lib/commands/git.jssrc/lib/components/panels/GitChanges.sveltesrc/lib/components/panels/GitFileTree.sveltesrc/lib/components/panels/GitPanel.sveltesrc/lib/stores/workspace.svelte.jssrc/routes/app/+layout.svelte
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/lib/components/panels/EnvPanel.svelte (1)
54-58: 💤 Low valuePath separator detection is fragile on Windows with mixed-separator paths.
The separator detection assumes the base path uses consistent separators. On Windows, paths can have mixed separators (e.g.,
C:/Users\foo). Consider using a more robust approach or letting Tauri/the backend handle path construction.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/lib/components/panels/EnvPanel.svelte` around lines 54 - 58, The absPath function’s separator detection is fragile because it assumes base uses a single separator; instead normalize both workspace.folderPath and relPath by replacing all backslashes and forward slashes with a single canonical separator and then construct the path using the platform-aware separator from a path utility (e.g., path.sep) or, better, use a path-joining helper (e.g., path.join or the Tauri/backend path API) to concatenate values; update absPath (which reads workspace.folderPath and relPath) to first normalize/reduce mixed separators, then join via the platform-aware join/sep routine so mixed Windows paths like "C:/Users\foo" are handled correctly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/commands/git.rs`:
- Around line 791-798: The code trims `pattern` for the de-dup check but writes
the raw `pattern` to the file; change the logic to use a trimmed pattern for
both comparison and writing (e.g., introduce a trimmed variable like
`pattern_trimmed = pattern.trim()` and use it in the existing.lines().any(...)
check and when building `new_content`), preserving the existing newline handling
(use `pattern_trimmed` instead of `pattern` in the format calls) so no
accidental leading/trailing whitespace is written to .gitignore.
In `@src-tauri/src/commands/watcher.rs`:
- Around line 175-183: unwatch_project currently clears the watcher state
(WatcherState) but does not increment the shared generation counter, leaving a
race where a stale background thread started by watch_project can later reattach
a debouncer; modify unwatch_project to acquire the same lock on state.0, set the
guard to None and also increment the associated generation counter (the same
generation field used by watch_project) before returning so any in-flight
threads comparing gen will fail and cannot overwrite cleared state.
---
Nitpick comments:
In `@src/lib/components/panels/EnvPanel.svelte`:
- Around line 54-58: The absPath function’s separator detection is fragile
because it assumes base uses a single separator; instead normalize both
workspace.folderPath and relPath by replacing all backslashes and forward
slashes with a single canonical separator and then construct the path using the
platform-aware separator from a path utility (e.g., path.sep) or, better, use a
path-joining helper (e.g., path.join or the Tauri/backend path API) to
concatenate values; update absPath (which reads workspace.folderPath and
relPath) to first normalize/reduce mixed separators, then join via the
platform-aware join/sep routine so mixed Windows paths like "C:/Users\foo" are
handled correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1697ae0c-773b-45ce-be2a-cd0c356c4016
📒 Files selected for processing (6)
src-tauri/src/commands/git.rssrc-tauri/src/commands/watcher.rssrc-tauri/src/lib.rssrc/lib/components/panels/EnvPanel.sveltesrc/lib/components/panels/GitChanges.sveltesrc/lib/components/panels/GitFileTree.svelte
🚧 Files skipped from review as they are similar to previous changes (2)
- src/lib/components/panels/GitChanges.svelte
- src/lib/components/panels/GitFileTree.svelte
Summary by CodeRabbit
New Features
Improvements
Chores