Enhance repository management with caching and UI improvements#2
Merged
Conversation
…ations - Extend AppState with cached git data (commits, branches, remotes, tags, status) - Add RefreshGitData, ClearGitCache, and SetRepoError actions - Implement 2-second refresh throttling via needs_refresh() - Add show_cached variants for UI components (body, sidebar, commit_panel) - Update main.rs to refresh git data periodically instead of every frame - Add CachedCommit, CachedBranch, CachedRemote, CachedTag, CachedRepoStatus types - Fix repo_error handling via state store instead of local field
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
There was a problem hiding this comment.
Pull request overview
This PR introduces a new Git integration layer (via git2), a caching-backed application state store, and updates the egui UI to display repository-aware information (repo name, branches, recents, debug/log UI) while refactoring UI modules.
Changes:
- Added
src/git/*module with core Git repository access (GitRepo), models, and error types. - Introduced
src/state/*store/caching layer to reduce repeated Git reads and drive UI state (recents, cached commits/branches/status). - Refactored UI into
src/ui/*, wiring repo-aware titlebar/toolbar/sidebar/body and adding a debug log window + file picker flow.
Reviewed changes
Copilot reviewed 17 out of 20 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/toolbar.rs | Shows repo name + current branch in the toolbar; now accepts repo context. |
| src/ui/titlebar.rs | Adds Open/Recents actions, window button toggle, help links, and debug toggle. |
| src/ui/tabbar.rs | Adjusts tab display to be repo-aware and fixes sidebar width import path. |
| src/ui/sidebar.rs | Adds repo-aware sidebar rendering and a cached rendering path driven by AppState. |
| src/ui/mod.rs | New UI module root exporting UI submodules. |
| src/ui/commit_panel.rs | New commit panel UI that can render live or from cached status. |
| src/ui/body.rs | Uses Git commits (or cached commits) to render the commit graph; conditionally shows commit panel. |
| src/state/mod.rs | Introduces AppState + reducer/actions for repo selection, caching, and refresh logic. |
| src/main.rs | Wires store, logging, repo open flow (folder picker/recents), periodic refresh, cached UI rendering, and debug log window. |
| src/logger/mod.rs | Adds a tracing subscriber layer that buffers logs for in-app display. |
| src/lib.rs | Exposes git, logger, state, and ui as library modules. |
| src/git/repo.rs | Implements GitRepo operations (open, commits, branches, remotes, tags, status, stashes). |
| src/git/models.rs | Adds basic Git domain models (Commit/Branch/Remote/Tag/Stash/RepoStatus). |
| src/git/mod.rs | Git module root + re-exports (GitRepo, Commit). |
| src/git/error.rs | Introduces GitError with conversions from git2 and IO errors. |
| src/components/commit_panel.rs | Removes old commit panel component (migrated into src/ui). |
| Makefile | Adds dev target for cargo run --release. |
| Cargo.toml | Bumps version and adds dependencies for git access, logging, dialogs, and state store. |
| Cargo.lock | Updates lockfile for new dependencies. |
| .gitignore | Adds /references to ignored paths. |
Comments suppressed due to low confidence (2)
src/ui/toolbar.rs:147
- When
git_repois present buthead_branch()fails (or returns detached), the UI falls back to displaying"master". This can misrepresent the actual repo state (e.g.,main, detached HEAD, or error) and hide real failures. Prefer falling back to something neutral like"HEAD"/"(unknown)", or surface an error indicator instead of hardcoding a branch name.
src/ui/body.rs:836 format_commit_date_from_secsimplements manual calendar arithmetic but only treats leap years asyear % 4 == 0, which is incorrect for century years (e.g., 2100) and is generally error-prone to maintain. Consider using a dedicated date/time crate (e.g.,time/chrono) or formatting via an existing utility to avoid incorrect dates in the commit list.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+108
to
+121
| pub fn needs_refresh(&self) -> bool { | ||
| match self.last_refresh { | ||
| Some(last) => { | ||
| let elapsed = Instant::now() | ||
| .duration_since(Instant::now() - Duration::from_millis(last as u64)); | ||
| elapsed.as_millis() >= REFRESH_INTERVAL_MS as u128 | ||
| } | ||
| None => true, | ||
| } | ||
| } | ||
|
|
||
| pub fn mark_refreshed(mut self) -> Self { | ||
| self.last_refresh = Some(Instant::now().elapsed().as_millis()); | ||
| self |
Comment on lines
+219
to
+225
| if let Ok(diff) = self.repo.diff_index_to_workdir(None, None) { | ||
| let stats = diff.stats().ok(); | ||
| if let Some(s) = stats { | ||
| additions = s.insertions(); | ||
| deletions = s.deletions(); | ||
| } | ||
| } |
Owner
Author
|
@CodeRabbit review |
✅ Actions performedReview triggered.
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds a git abstraction (errors, models, GitRepo), a tracing-backed in-memory LogBuffer, an AppState+zed store with cached git data and actions, main wiring to open/refresh repos and surface errors, and refactors UI to consume live or cached repository context. ChangesGit integration and state-driven UI refactor
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…tion - Store Instant directly instead of reconstructing from epoch millis - Skip last_refresh from serde and PartialEq (Instant doesn't impl them) - Compute diff_index_to_workdir stats once outside status loop (was O(n)) - Fix files_changed to sum staged + unstaged counts
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🤖 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 `@Cargo.toml`:
- Line 14: The rfd dependency in Cargo.toml is pinned to linux-only GTK3 via
`rfd = { version = "0.17.2", default-features = false, features = ["gtk3"] }`,
which breaks non-Linux builds; update Cargo.toml to declare rfd as a
platform-specific dependency (e.g., move the GTK3-backed declaration into a
target.'cfg(target_os = "linux")'.dependencies block and add a separate rfd
entry for other platforms or a default-features-enabled rfd in a
target.'cfg(not(target_os = "linux"))'.dependencies block) so that the `rfd`
dependency uses gtk3 only on Linux and uses the appropriate/default features on
macOS/Windows.
In `@Makefile`:
- Around line 3-4: The Makefile's dev target currently runs `cargo run
--release`, which is misleading for a development workflow; update the Makefile
so the `dev` target executes `cargo run` (debug build) instead, or if the
release build is intentional, rename the target from `dev` to a clearer name
like `run-release` and add a separate `dev` target that runs `cargo run`;
specifically update the line containing `dev:` and the command `cargo run
--release` to reflect your chosen option (either replace `--release` with
nothing or rename the target and add a new `run-release` target).
In `@src/git/repo.rs`:
- Around line 195-226: The diff stats (calls to
self.repo.diff_index_to_workdir(...) and diff.stats()) are being recomputed
inside the for entry in statuses.iter() loop; move the diff calculation and
stats extraction out of that loop so it's executed once per refresh: call
self.repo.diff_index_to_workdir(None, None) and compute let stats =
diff.stats().ok() (then set additions = stats.insertions() and deletions =
stats.deletions() if present) before iterating statuses.iter(), leaving the
existing per-entry logic that updates unstaged_count, staged_count,
staged_files, and path unchanged.
- Around line 46-53: The filter_map chains in revwalk/commits, remotes(),
tags(), and stashes() currently use .ok()? which swallows git errors; change
these to propagate failures instead: make the iterator closures return Result<T,
GitError> (or map git2::Error into your GitError) and collect using a fallible
collect (e.g. collect::<Result<Vec<_>, GitError>>() or use
try_for_each/try_fold) so any failing self.repo.find_commit, remote/tag lookup,
or stash iteration returns Err(GitError) instead of being silently dropped;
update functions (e.g. the commits() block using revwalk, remotes(), tags(),
stashes(), and uses of self.commit_from_git2) to return Result<Vec<...>,
GitError> and replace .ok()? with ? after mapping errors into your GitError
type.
In `@src/logger/mod.rs`:
- Around line 20-24: The constructor pub fn new currently calls
VecDeque::with_capacity(max_entries) which still permits storing entries when
max_entries == 0; update pub fn new (and the other constructor at the 35-40
region) to explicitly handle max_entries == 0 by initializing entries to an
empty VecDeque and retaining max_entries == 0, and then change the insertion
code paths (the methods that push into entries) to early-return/no-op when
self.max_entries == 0 so no entries are ever stored; reference the fields
entries and max_entries and the use of VecDeque::with_capacity when locating the
code to change.
In `@src/main.rs`:
- Around line 98-122: The current refresh code in refresh_git_data calls
repo.commits(), repo.branches(), repo.remotes(), repo.tags(), and repo.status()
and then always dispatches AppAction::SetRepoError(None), which clears existing
repo errors even if some calls failed; change the logic to capture each call's
Result (don't unconditionally unwrap_or_default), record any errors, still
dispatch AppAction::RefreshGitData with whatever successful data you have, and
only dispatch AppAction::SetRepoError(None) when all git calls succeeded; if any
call failed dispatch AppAction::SetRepoError(Some(error)) or leave the previous
error intact instead of clearing it (use the Result variants from
commits/branches/remotes/tags/status to determine success/failure).
- Around line 87-92: When repository open fails, do not clear the in-memory
handle — remove the assignment self.git_repo = None in the Err(e) branch so you
preserve the existing live repository state; instead only dispatch the error
with self.store.dispatch(AppAction::SetRepoError(Some(e.to_string()))) (and keep
tracing::error!(...)) so UI/store error state is updated without wiping the
repo/cache. Ensure this change is applied in the function handling the open
result (the code block containing Err(e) with tracing::error!, self.git_repo,
and store.dispatch).
In `@src/state/mod.rs`:
- Around line 108-121: Replace the Instant-based logic in needs_refresh() and
mark_refreshed() with stable UNIX epoch milliseconds: in mark_refreshed() set
self.last_refresh =
Some(SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_millis()); and in
needs_refresh() compute let now_ms =
SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_millis(); then return
match self.last_refresh { Some(last) => now_ms.saturating_sub(last) >=
REFRESH_INTERVAL_MS as u128, None => true }; update types if necessary so
last_refresh holds the same millisecond integer type and keep using the existing
symbols needs_refresh, mark_refreshed, last_refresh, and REFRESH_INTERVAL_MS.
In `@src/ui/body.rs`:
- Around line 355-381: The lane assignment in from_git (and similar from_cached
functions) can grow unbounded causing nodes to render outside the fixed graph
lanes; limit lanes by computing a max_lanes (e.g., branch_colors.len() or a
configurable max) and when assigning a new lane in lane_map/next_lane clamp or
wrap the assigned value into 0..max_lanes (or evict/reuse the
least-recently-used lane) so lane IDs never exceed the rendered capacity; update
the logic around lane_map.entry(key).or_insert_with(...) and next_lane to
produce values modulo max_lanes (or use an LRU reuse strategy) and ensure any
code that reads lane IDs (rendering/connection code) still works with the
constrained range.
- Around line 118-126: The render path is calling git I/O (repo.commits and
repo.status) inside painting which blocks frames; refactor so
CommitGraph::from_git is not invoked during render. Move git access out of the
render loop by computing commits/status asynchronously (e.g., spawn a background
task or use an init/update function) and store the resulting CommitGraph or
cached commits in component state (replace direct uses of git_repo.commit(s) and
repo.status in the render with an Option or CachedCommitGraph field). Update the
rendering code to read the precomputed CommitGraph or fall back to
CommitGraph::from_json(MOCK_COMMITS_JSON) if the cache is empty; do the same for
the other occurrences that call repo.status to ensure all git I/O happens off
the paint path.
In `@src/ui/commit_panel.rs`:
- Around line 81-103: The code calls repo.status() multiple times per paint pass
(e.g., in the block that calls header_stats and later before top_strip), causing
repeated git work; call repo.status() once, bind the result to a local variable
(e.g., let status = repo.status()?) and reuse that Status for header_stats,
top_strip and any other places in show/middle_row (also at the locations around
lines 307-310) by passing a reference to the same Status value instead of
calling repo.status() repeatedly.
- Around line 25-27: The clamp call in the width calculation can panic when
body_rect.width() - PANEL_MARGIN * 2.0 is less than 620.0; in both show and
show_cached compute let available = body_rect.width() - PANEL_MARGIN * 2.0 and
then derive safe bounds before clamping (e.g. let min_bound =
620.0.min(available); let max_bound = 620.0.max(available)) so min_bound <=
max_bound, then use (body_rect.width() * 0.56).clamp(min_bound,
max_bound).max(320.0); apply this exact defensive change to the width
calculation in both show and show_cached.
In `@src/ui/toolbar.rs`:
- Around line 132-147: The code currently falls back to the hardcoded string
"master" when branch is None; update the logic around git_repo / head_branch()
so branch_name displays a clear unavailable/detached placeholder instead of
"master" (e.g., "no branch" or "detached HEAD"); change the computation of
branch_name (used in the closure where branch.as_deref().unwrap_or("master") is
called) to map None -> a descriptive placeholder and keep real branch names
unchanged, so the UI shows correct status when no repo is open or head_branch()
fails.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 5ca5a27e-87fd-4236-9c66-6fe5d7788a1e
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
.gitignoreCargo.tomlMakefilesrc/components/commit_panel.rssrc/git/error.rssrc/git/mod.rssrc/git/models.rssrc/git/repo.rssrc/lib.rssrc/logger/mod.rssrc/main.rssrc/state/mod.rssrc/ui/body.rssrc/ui/commit_panel.rssrc/ui/mod.rssrc/ui/sidebar.rssrc/ui/tabbar.rssrc/ui/titlebar.rssrc/ui/toolbar.rs
💤 Files with no reviewable changes (1)
- src/components/commit_panel.rs
Comment on lines
+355
to
+381
| fn from_git(commits: &[crate::git::Commit]) -> Self { | ||
| let branch_colors = [ | ||
| egui::Color32::from_rgb(255, 165, 16), | ||
| egui::Color32::from_rgb(238, 202, 34), | ||
| egui::Color32::from_rgb(255, 45, 72), | ||
| egui::Color32::from_rgb(151, 113, 73), | ||
| egui::Color32::from_rgb(42, 167, 222), | ||
| egui::Color32::from_rgb(56, 193, 114), | ||
| ]; | ||
| let mut lane_map = std::collections::HashMap::new(); | ||
| let mut next_lane = 0; | ||
|
|
||
| let commits = commits | ||
| .iter() | ||
| .enumerate() | ||
| .map(|(i, c)| { | ||
| let lane = if c.parents.len() <= 1 { | ||
| 0 | ||
| } else { | ||
| let key = c.hash.clone(); | ||
| let lane = lane_map.entry(key).or_insert_with(|| { | ||
| let l = next_lane; | ||
| next_lane += 1; | ||
| l | ||
| }); | ||
| *lane | ||
| }; |
There was a problem hiding this comment.
Lane assignment can exceed rendered lane capacity.
from_git/from_cached allocate unbounded lane IDs, but drawing is effectively fixed-width/fixed-lane. Merge-heavy histories can push nodes outside the graph column or miss connecting lines.
Suggested fix
- let lane = if c.parents.len() <= 1 {
- 0
- } else {
+ let lane = if c.parents.len() <= 1 {
+ 0
+ } else {
let key = c.hash.clone();
let lane = lane_map.entry(key).or_insert_with(|| {
let l = next_lane;
next_lane += 1;
l
});
- *lane
+ (*lane).min(3)
};-fn draw_graph(ui: &egui::Ui, rect: egui::Rect, commits: &[Commit]) {
- for lane in 0..4 {
+fn draw_graph(ui: &egui::Ui, rect: egui::Rect, commits: &[Commit]) {
+ let max_lane = commits.iter().map(|c| c.lane).max().unwrap_or(0).min(3);
+ for lane in 0..=max_lane {Also applies to: 416-442, 554-566, 672-678, 793-795
🤖 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/ui/body.rs` around lines 355 - 381, The lane assignment in from_git (and
similar from_cached functions) can grow unbounded causing nodes to render
outside the fixed graph lanes; limit lanes by computing a max_lanes (e.g.,
branch_colors.len() or a configurable max) and when assigning a new lane in
lane_map/next_lane clamp or wrap the assigned value into 0..max_lanes (or
evict/reuse the least-recently-used lane) so lane IDs never exceed the rendered
capacity; update the logic around lane_map.entry(key).or_insert_with(...) and
next_lane to produce values modulo max_lanes (or use an LRU reuse strategy) and
ensure any code that reads lane IDs (rendering/connection code) still works with
the constrained range.
- Delete MOCK_COMMITS_JSON constant and MockCommit/MockBranch structs - Remove CommitGraph::from_json method - Remove unused from_git, parse_color, format_commit_date functions - Remove BranchLabel struct and commit.branch field - Remove unused GITHUB_LOGO and serde Deserialize imports - Fall back to empty graph when no cached commits available
- Cargo.toml: make rfd platform-specific (gtk3 on Linux, default elsewhere) - Makefile: dev target runs debug build, add run-release target - state/mod.rs: replace Instant with SystemTime epoch millis for stable timing - logger/mod.rs: early-return in push() when max_entries == 0 - main.rs: preserve git_repo on open failure, capture errors in refresh_git_data - toolbar.rs: show 'no branch' instead of 'master' when head unavailable - commit_panel.rs: call status() once per paint, fix clamp panic with safe bounds - body.rs: clamp lane assignment to max_lanes (branch_colors.len())
- Implement proper lane-based graph algorithm from Zed editor - Add LaneState, CommitLine, CommitLineSegment, GraphData models - Implement add_commits with parent tracking and bezier curves - Draw commit circles with proper coloring from accent palette - Render merge/checkout curves using cubic bezier shapes - Add row selection and hover states - Remove mock commit data and legacy rendering code - Use cached commits from AppState for graph data
…odes - Enforce single-line commit messages with ellipsis truncation - Change graph nodes from filled circles to hollow outline circles - Fix date column showing commit message instead of formatted timestamp
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/git/repo.rs (1)
44-50:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPropagate git lookup failures instead of returning partial lists.
Line 44 and the analogous blocks in lines 104, 123, and 154 convert
ResulttoOptionwith.ok()?, which silently drops git errors and returns incomplete data as success.Proposed fix
- let commits: Vec<Commit> = revwalk + let commits: Vec<Commit> = revwalk .take(limit) - .filter_map(|oid_result| { - let oid = oid_result.ok()?; - let commit = self.repo.find_commit(oid).ok()?; - Some(self.commit_from_git2(&commit)) - }) - .collect(); + .map(|oid_result| { + let oid = oid_result?; + let commit = self.repo.find_commit(oid)?; + Ok::<Commit, git2::Error>(self.commit_from_git2(&commit)) + }) + .collect::<Result<Vec<_>, _>>()?;Apply the same pattern to
remotes(),tags(), andstashes()by replacingfilter_map + .ok()?with fallible mapping andcollect::<Result<Vec<_>, _>>()?.Also applies to: 104-107, 123-129, 154-156
🤖 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/git/repo.rs` around lines 44 - 50, The current revwalk mapping uses filter_map + .ok()? which swallows git errors and returns partial lists; change the pattern to propagate errors by mapping to Result and using collect::<Result<Vec<_>, _>>()? instead: for the commit list replace the closure in revwalk.take(limit).filter_map(...) with revwalk.take(limit).map(|oid_result| { let oid = oid_result?; let commit = self.repo.find_commit(oid)?; Ok(self.commit_from_git2(&commit)) }).collect::<Result<Vec<_>, git2::Error>>()? so failures bubble up; apply the same transformation to the remotes(), tags(), and stashes() blocks (replace their filter_map + .ok()? closures with fallible map closures returning Result of the corresponding mapped value and collect::<Result<Vec<_>, _>>()?).
🤖 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/git/repo.rs`:
- Around line 186-219: The current logic increments staged_count and
unstaged_count separately causing a file with both index and worktree changes to
be double-counted; instead, iterate the statuses and collect unique file paths
into two HashSets (e.g., staged_paths and unstaged_paths) based on
status.is_index_* and status.is_wt_* checks (use
entry.path().unwrap_or("unknown").to_string() as the key), then set staged_count
= staged_paths.len(), unstaged_count = unstaged_paths.len(), staged_files =
staged_paths.into_iter().collect(), and compute files_changed =
staged_paths.union(&unstaged_paths).count() (or build a third HashSet for union)
while leaving the diff_index_to_workdir() additions/deletions logic unchanged.
In `@src/ui/body.rs`:
- Around line 530-535: The current conditional only refreshes state.graph_data
when either side is empty, so updates to app_state.cached_commits leave the old
graph intact; change the check to detect differences between
state.graph_data.commits and app_state.cached_commits (e.g., compare lengths or
use != on the commit Vec) and when they differ call state.graph_data.clear() and
then, if app_state.cached_commits is non-empty, call
state.graph_data.add_commits(&app_state.cached_commits) so the graph always
syncs to updated cached_commits.
---
Duplicate comments:
In `@src/git/repo.rs`:
- Around line 44-50: The current revwalk mapping uses filter_map + .ok()? which
swallows git errors and returns partial lists; change the pattern to propagate
errors by mapping to Result and using collect::<Result<Vec<_>, _>>()? instead:
for the commit list replace the closure in revwalk.take(limit).filter_map(...)
with revwalk.take(limit).map(|oid_result| { let oid = oid_result?; let commit =
self.repo.find_commit(oid)?; Ok(self.commit_from_git2(&commit))
}).collect::<Result<Vec<_>, git2::Error>>()? so failures bubble up; apply the
same transformation to the remotes(), tags(), and stashes() blocks (replace
their filter_map + .ok()? closures with fallible map closures returning Result
of the corresponding mapped value and collect::<Result<Vec<_>, _>>()?).
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9bf597b4-48da-44ff-9cac-6c78d675f755
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (10)
Cargo.tomlMakefilesrc/git/repo.rssrc/logger/mod.rssrc/main.rssrc/state/mod.rssrc/ui/body.rssrc/ui/commit_panel.rssrc/ui/sidebar.rssrc/ui/toolbar.rs
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
Cargo.toml (1)
9-18:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUpdate git2 to 0.21.0 and consider updating serde to address dependency advisories.
As of May 2026, RustSec advisories show:
- git2 0.20.4: RUSTSEC-2026-0008 flags unsoundness in the crate; upgrade to version 0.21.0 (released May 18, 2026)
- serde 1.0.219: version 1.0.228 is available (September 2025); review changelog for relevant updates
- tracing-subscriber 0.3.23: already patched against RUSTSEC-2025-0055 (ANSI escape injection), no action needed
- tracing 0.1.44: current latest version
🤖 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 `@Cargo.toml` around lines 9 - 18, The Cargo.toml dependencies include git2 = "0.20.4" (unsound per RUSTSEC-2026-0008) and serde = "1.0.219"; update git2 to "0.21.0" to address the unsoundness advisory and consider bumping serde to a newer safe release (e.g., "1.0.228") by editing the [dependencies] entries for git2 and serde in Cargo.toml, then run cargo update and cargo build/test to ensure no breakage and adjust any code if API changes surface in the git2 or serde crates.
🤖 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/git/repo.rs`:
- Around line 436-441: The unstage_file implementation currently calls
index.remove_path(), which deletes the index entry (git rm --cached) instead of
reverting staged changes; change unstage_file to reset the index entry to match
HEAD: lookup HEAD (self.repo.find_reference("HEAD") and peel to the tree),
update the index entry from that HEAD tree for the given path (e.g. use
index.read_tree()/update-from-tree semantics or the index API that imports the
tree entry for that path), then write the index and return Ok(()); remove the
use of index.remove_path() so tracked files revert to their HEAD version rather
than being removed from the index.
- Around line 453-474: The stage_all implementation currently calls
index.add_path(...) for all working-tree changes, but add_path requires the file
to exist and will fail for deleted entries; update the loop in stage_all to call
index.remove_path(Path::new(path)) when entry.status().is_wt_deleted() (and
continue to call index.add_path for new/modified/typechange/renamed), ensuring
you still guard with if let Some(path) = entry.path() and propagate any index
errors as before; keep index.write() at the end.
In `@src/main.rs`:
- Around line 166-192: The CommitAction handlers currently ignore errors from
repo.stage_file, repo.unstage_file, repo.discard_file, repo.stage_all, and
repo.discard_all which causes silent failures; update each match arm handling
those operations (e.g., CommitAction::StageFile, UnstageFile, DiscardFile,
StageAll, DiscardAll) to capture the Result and on Err either log the error
(including the error message and the related path/operation) or dispatch an
error action to the store before returning, and only call
self.refresh_git_data() on Ok — include contextual info (operation name and path
where relevant) in the log/dispatch so failures are discoverable.
In `@src/ui/commit_panel.rs`:
- Around line 1099-1104: The Discard All button currently calls
state.queue_action(CommitAction::DiscardAll) immediately; change it to require
confirmation before dispatching that action by showing a modal confirmation
dialog (or implementing a two-step click) and only calling
state.queue_action(CommitAction::DiscardAll) when the user explicitly confirms;
apply the same pattern for the other occurrence around the code that currently
queues CommitAction::DiscardAll (the block at 1131-1136) so both places open the
confirmation dialog and act only on explicit confirm.
---
Outside diff comments:
In `@Cargo.toml`:
- Around line 9-18: The Cargo.toml dependencies include git2 = "0.20.4" (unsound
per RUSTSEC-2026-0008) and serde = "1.0.219"; update git2 to "0.21.0" to address
the unsoundness advisory and consider bumping serde to a newer safe release
(e.g., "1.0.228") by editing the [dependencies] entries for git2 and serde in
Cargo.toml, then run cargo update and cargo build/test to ensure no breakage and
adjust any code if API changes surface in the git2 or serde crates.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d0add7a9-b806-40f4-8a62-ec6796200f67
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (7)
Cargo.tomlsrc/git/models.rssrc/git/repo.rssrc/main.rssrc/state/mod.rssrc/ui/body.rssrc/ui/commit_panel.rs
… confirmation - Bump git2 to 0.21.0 and serde to 1.0.228 (RUSTSEC-2026-0008) - Adapt to git2 API changes: remotes/tags iterators, entry.path() Result type, IndexEntry struct fields (path as Vec<u8>, mode as u32, explicit ctime/mtime) - Fix unstage_file to reset index from HEAD tree instead of remove_path - Fix stage_all to handle deleted files with remove_path - Add tracing::error! logging in handle_commit_action before returning - Add show_discard_confirm flag and inline confirmation overlay for Discard All
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/commit_panel.rs (1)
236-243:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMove the discard confirmation dialog outside the footer scope.
show_discard_confirm(...)is called from the footer-scopedUiin bothactionsandactions_cached, which clips the dialog rendering and interaction to the footer bounds. The confirmation dialog rect (derived frompanel_rect) extends well beyond the footer area and becomes invisible and non-clickable. Callshow_discard_confirm()from the parentUiafter the footer scope builder completes in bothrender_panelandrender_panel_cached.Suggested fix
fn render_panel( ui: &mut egui::Ui, panel_rect: egui::Rect, state: &mut State, header_text: &str, status: &Option<crate::git::models::RepoStatus>, ) { @@ ui.scope_builder( egui::UiBuilder::new() .id_salt("floating_commit_footer") .max_rect(footer_rect.shrink2(egui::vec2(CONTENT_PAD, 4.0))) .layout(egui::Layout::top_down(egui::Align::Min)), |ui| { - actions(ui, state, panel_rect); + actions(ui, state); }, ); + + if state.show_discard_confirm { + show_discard_confirm(ui, panel_rect, state); + } } @@ -fn actions(ui: &mut egui::Ui, state: &mut State, panel_rect: egui::Rect) { +fn actions(ui: &mut egui::Ui, state: &mut State) { @@ - if state.show_discard_confirm { - show_discard_confirm(ui, panel_rect, state); - } }Apply the same change to
render_panel_cached/actions_cached.🤖 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/ui/commit_panel.rs` around lines 236 - 243, The discard confirmation dialog (show_discard_confirm) is currently invoked from inside the footer-scoped Ui created by ui.scope_builder via the actions and actions_cached calls, which clips the dialog to the footer bounds; move the call to show_discard_confirm out of that scope so it runs on the parent/top-level Ui after the scope builder completes. Concretely: in both render_panel and render_panel_cached, keep the ui.scope_builder(...) block to render the footer and call actions/actions_cached there, then immediately after the scope_builder closure returns call show_discard_confirm(&mut parent_ui, state, panel_rect) (or the matching signature) so the dialog is rendered and interactive outside the footer; update any uses inside actions/actions_cached to no longer call show_discard_confirm.
🤖 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/git/repo.rs`:
- Around line 436-456: In unstage_file, get_path on head_tree can return
ErrorCode::NotFound for newly added files; change the logic in unstage_file to
match the result of head_tree.get_path(path) and handle the NotFound case by
simply removing the index entry (via index.remove_path) and writing the index,
while only constructing and adding the git2::IndexEntry when get_path succeeds;
update error propagation to return other errors as before and ensure
index.write() is still called after the removal or add operation.
---
Outside diff comments:
In `@src/ui/commit_panel.rs`:
- Around line 236-243: The discard confirmation dialog (show_discard_confirm) is
currently invoked from inside the footer-scoped Ui created by ui.scope_builder
via the actions and actions_cached calls, which clips the dialog to the footer
bounds; move the call to show_discard_confirm out of that scope so it runs on
the parent/top-level Ui after the scope builder completes. Concretely: in both
render_panel and render_panel_cached, keep the ui.scope_builder(...) block to
render the footer and call actions/actions_cached there, then immediately after
the scope_builder closure returns call show_discard_confirm(&mut parent_ui,
state, panel_rect) (or the matching signature) so the dialog is rendered and
interactive outside the footer; update any uses inside actions/actions_cached to
no longer call show_discard_confirm.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: abc24a98-7e30-4fdd-b205-fe34eb0e3771
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
Cargo.tomlsrc/git/repo.rssrc/main.rssrc/ui/commit_panel.rs
…ooter scope - unstage_file now handles ErrorCode::NotFound from head_tree.get_path for newly staged files, removing the index entry without re-adding - show_discard_confirm moved out of footer scope_builder to top-level Ui in render_panel and render_panel_cached so dialog is not clipped
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
src/ui/commit_panel.rs (3)
1092-1100:⚠️ Potential issue | 🟠 Major | ⚡ Quick winWire up or remove the dead footer CTAs.
Both
ReadyandCommitrender as enabled buttons, but theirResponses are ignored, so clicking them does nothing in either panel variant. That is a broken primary action path for the commit UI.Also applies to: 1124-1132
🤖 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/ui/commit_panel.rs` around lines 1092 - 1100, The footer buttons created with ui.add_enabled (using can_commit) for egui::Button (labels format!("{CHECK} Ready") and format!("{PAPER_PLANE} Commit")) are rendered but their Responses are ignored; either handle their clicks or remove them. Locate the two ui.add_enabled calls that create the "Ready" and "Commit" Buttons (and the similar pair around the other panel variant) and wire their returned Response values: check response.clicked() (or response.double_clicked() as appropriate) and call the existing commit flow/handler (the function or closure that performs validation and actual commit submission), or if that flow is not ready, remove the buttons to avoid dead CTAs. Ensure the click handling respects the can_commit condition already used when enabling the buttons.
1061-1082:⚠️ Potential issue | 🟠 Major | ⚡ Quick winAvoid byte slicing in
truncate_path.This helper truncates with
len()-based byte offsets, so any non-ASCII path can panic on an invalid UTF-8 boundary during rendering. Truncate onchar_indices()/chars()instead of raw byte indices.🤖 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/ui/commit_panel.rs` around lines 1061 - 1082, The truncate_path function uses byte-index slicing (e.g., &path[path.len() - max_chars + 3..], &file_name[file_name.len() - .....]) which can panic on non-ASCII paths; change the implementation to operate on characters rather than raw bytes: compute character counts with chars().count(), and when you need the last N characters of path or file_name use char-based extraction (e.g., collect the last N chars via chars().rev().take(N).collect::<Vec<_>>() then reverse and join, or use char_indices() to map char positions to safe byte offsets) instead of slicing by byte offsets; update all uses of path[..] and file_name[..] in truncate_path accordingly so truncation never slices inside a UTF-8 codepoint.
86-94:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t force the panel larger than the available viewport.
For body sizes below the 300px/200px floors plus margins, these calculations still produce a panel that is larger than
body_rect, which pushes controls off-screen. Clamp the minimum against the actual available width/height before positioning the rect.🔧 Minimal fix
fn panel_width(body_width: f32) -> f32 { - PANEL_WIDTH.min(body_width - PANEL_MARGIN * 2.0).max(300.0) + let available = (body_width - PANEL_MARGIN * 2.0).max(0.0); + PANEL_WIDTH.min(available).max(available.min(300.0)) } fn calc_panel_rect(body_rect: egui::Rect) -> egui::Rect { - let height = PANEL_HEIGHT - .min(body_rect.height() - PANEL_MARGIN * 2.0) - .max(200.0); + let available_h = (body_rect.height() - PANEL_MARGIN * 2.0).max(0.0); + let height = PANEL_HEIGHT.min(available_h).max(available_h.min(200.0)); let width = panel_width(body_rect.width());🤖 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/ui/commit_panel.rs` around lines 86 - 94, panel sizing can exceed the viewport when body_rect is smaller than the hard-coded floors; update panel_width and calc_panel_rect to clamp their minimums to the actual available space (body_width - PANEL_MARGIN*2.0 and body_rect.height() - PANEL_MARGIN*2.0) before applying PANEL_WIDTH/PANEL_HEIGHT floors. Specifically, in panel_width(body_width: f32) compute let available = (body_width - PANEL_MARGIN * 2.0).max(0.0) and then return available.min(PANEL_WIDTH).max( /* use a min floor that does not exceed available, e.g. 0 or computed available_min */ ); and in calc_panel_rect compute let available_height = (body_rect.height() - PANEL_MARGIN * 2.0).max(0.0) and clamp the height with available_height.min(PANEL_HEIGHT).max( /* same safe min */ ) so the panel never becomes larger than body_rect; adjust uses of PANEL_MARGIN/PANEL_WIDTH/PANEL_HEIGHT in panel_width and calc_panel_rect accordingly.src/git/repo.rs (4)
429-432:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStage per-file deletions the same way as
stage_all.This always calls
index.add_path(), so clicking the per-file stage button on a deleted file will fail instead of staging the deletion.stage_all()already has the right branch;stage_file()needs the same deleted-file handling.🤖 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/git/repo.rs` around lines 429 - 432, stage_file currently always calls index.add_path which fails for deleted files; update stage_file to mirror stage_all's logic by checking the file's status (or whether the path exists) and call index.remove_path(Path::new(path)) when the file is deleted, otherwise call index.add_path(Path::new(path)); keep the existing index.write() and error propagation so stage_file aligns with stage_all's deleted-file handling.
513-515:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon’t return
Ok(())after partial discard failures.
discard_all()suppresses filesystem and checkout errors with.ok(), so callers see success even when some paths were not actually restored or deleted. This is especially risky for a destructive action because the UI cannot tell the user that the discard only partially succeeded.Also applies to: 530-532
🤖 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/git/repo.rs` around lines 513 - 515, The discard_all() implementation currently swallows filesystem and checkout errors by calling .ok() on results (e.g., the std::fs::remove_file(&full_path) and std::fs::remove_dir_all(&full_path) calls and the similar block at 530-532), causing the function to return Ok(()) even on partial failures; change these to propagate or accumulate errors instead of silencing them: replace the .ok() calls with proper error handling inside discard_all() (for example, collect each failure into a Vec<Error> or return early with Err when a remove_file/remove_dir_all or checkout/reset operation fails), then return an Err describing partial/complete failure if any filesystem or checkout operation failed so callers can surface a failure to the UI.
105-107:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winUse
flatten(), notok(), ongit2::StringArrayiterators.
git2::StringArray::iter()yieldsOption<Option<&str>>, sor.ok()is a type error here—.ok()is a method onResult, notOption. Useflatten()to flatten the nestedOptiontypes.🔧 Minimal fix
- .filter_map(|r| r.ok().flatten()) + .flatten()Apply the same change in both
remotes()(line 107) andtags()(line 126).🤖 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/git/repo.rs` around lines 105 - 107, In src/git/repo.rs the iterator over git2::StringArray is being incorrectly handled with `.ok().flatten()`; change the flattening logic to use `.flatten()` directly on the iterator (e.g., in the remotes collection building where `remotes.iter().filter_map(|r| r.ok().flatten())` replace the chain with `.filter_map(|r| r.flatten())`) and apply the same fix in the tags handling (the `tags()` path) so both `remotes` and `tags` use `.flatten()` to collapse the Option<Option<&str>> produced by StringArray::iter().
485-495:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winMatch
StatusEntry::path()as anOption.
StatusEntry::path()returnsOption<&str>, notResult, so all threeif let Ok(path)blocks are compile errors. Match onSome(path)instead, or usepath_bytes()for non-UTF-8 paths.Occurs at lines 485, 509, and 524 in both
stage_all()anddiscard_all()methods.🔧 Minimal fix
- if let Ok(path) = entry.path() { + if let Some(path) = entry.path() {Apply to lines 485, 509, and 524.
🤖 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/git/repo.rs` around lines 485 - 495, The code incorrectly treats StatusEntry::path() as Result; in stage_all() and discard_all() replace the three occurrences of "if let Ok(path)" with "if let Some(path)" (or call path_bytes() when you need non-UTF-8 handling) and pass the resulting &str (or &[u8] converted appropriately) to index.add_path / index.remove_path; ensure you update the three sites referenced (around the index.add_path and index.remove_path calls) to match Option semantics and use std::path::Path::new(path) only when path is a &str, or convert bytes when using path_bytes().
🤖 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/git/repo.rs`:
- Around line 436-463: In unstage_file the call to
self.repo.head()?.peel_to_tree()? can fail for an unborn branch (fresh repo)
before you reach the NotFound handling; change the logic to first attempt to
obtain the head tree with explicit error handling (e.g. call
self.repo.head().and_then(|h| h.peel_to_tree()) and match the Result), treat
UnbornBranch and NotFound as “no head tree” (proceed by only removing the index
entry and writing the index), and return Err for any other git2 errors; keep the
existing behavior that when a head tree exists you construct the
git2::IndexEntry from tree_entry and add it to the index, then write the index.
This ensures unstage_file handles fresh/unborn repositories by simply removing
the index entry and writing the index.
---
Outside diff comments:
In `@src/git/repo.rs`:
- Around line 429-432: stage_file currently always calls index.add_path which
fails for deleted files; update stage_file to mirror stage_all's logic by
checking the file's status (or whether the path exists) and call
index.remove_path(Path::new(path)) when the file is deleted, otherwise call
index.add_path(Path::new(path)); keep the existing index.write() and error
propagation so stage_file aligns with stage_all's deleted-file handling.
- Around line 513-515: The discard_all() implementation currently swallows
filesystem and checkout errors by calling .ok() on results (e.g., the
std::fs::remove_file(&full_path) and std::fs::remove_dir_all(&full_path) calls
and the similar block at 530-532), causing the function to return Ok(()) even on
partial failures; change these to propagate or accumulate errors instead of
silencing them: replace the .ok() calls with proper error handling inside
discard_all() (for example, collect each failure into a Vec<Error> or return
early with Err when a remove_file/remove_dir_all or checkout/reset operation
fails), then return an Err describing partial/complete failure if any filesystem
or checkout operation failed so callers can surface a failure to the UI.
- Around line 105-107: In src/git/repo.rs the iterator over git2::StringArray is
being incorrectly handled with `.ok().flatten()`; change the flattening logic to
use `.flatten()` directly on the iterator (e.g., in the remotes collection
building where `remotes.iter().filter_map(|r| r.ok().flatten())` replace the
chain with `.filter_map(|r| r.flatten())`) and apply the same fix in the tags
handling (the `tags()` path) so both `remotes` and `tags` use `.flatten()` to
collapse the Option<Option<&str>> produced by StringArray::iter().
- Around line 485-495: The code incorrectly treats StatusEntry::path() as
Result; in stage_all() and discard_all() replace the three occurrences of "if
let Ok(path)" with "if let Some(path)" (or call path_bytes() when you need
non-UTF-8 handling) and pass the resulting &str (or &[u8] converted
appropriately) to index.add_path / index.remove_path; ensure you update the
three sites referenced (around the index.add_path and index.remove_path calls)
to match Option semantics and use std::path::Path::new(path) only when path is a
&str, or convert bytes when using path_bytes().
In `@src/ui/commit_panel.rs`:
- Around line 1092-1100: The footer buttons created with ui.add_enabled (using
can_commit) for egui::Button (labels format!("{CHECK} Ready") and
format!("{PAPER_PLANE} Commit")) are rendered but their Responses are ignored;
either handle their clicks or remove them. Locate the two ui.add_enabled calls
that create the "Ready" and "Commit" Buttons (and the similar pair around the
other panel variant) and wire their returned Response values: check
response.clicked() (or response.double_clicked() as appropriate) and call the
existing commit flow/handler (the function or closure that performs validation
and actual commit submission), or if that flow is not ready, remove the buttons
to avoid dead CTAs. Ensure the click handling respects the can_commit condition
already used when enabling the buttons.
- Around line 1061-1082: The truncate_path function uses byte-index slicing
(e.g., &path[path.len() - max_chars + 3..], &file_name[file_name.len() - .....])
which can panic on non-ASCII paths; change the implementation to operate on
characters rather than raw bytes: compute character counts with chars().count(),
and when you need the last N characters of path or file_name use char-based
extraction (e.g., collect the last N chars via
chars().rev().take(N).collect::<Vec<_>>() then reverse and join, or use
char_indices() to map char positions to safe byte offsets) instead of slicing by
byte offsets; update all uses of path[..] and file_name[..] in truncate_path
accordingly so truncation never slices inside a UTF-8 codepoint.
- Around line 86-94: panel sizing can exceed the viewport when body_rect is
smaller than the hard-coded floors; update panel_width and calc_panel_rect to
clamp their minimums to the actual available space (body_width -
PANEL_MARGIN*2.0 and body_rect.height() - PANEL_MARGIN*2.0) before applying
PANEL_WIDTH/PANEL_HEIGHT floors. Specifically, in panel_width(body_width: f32)
compute let available = (body_width - PANEL_MARGIN * 2.0).max(0.0) and then
return available.min(PANEL_WIDTH).max( /* use a min floor that does not exceed
available, e.g. 0 or computed available_min */ ); and in calc_panel_rect compute
let available_height = (body_rect.height() - PANEL_MARGIN * 2.0).max(0.0) and
clamp the height with available_height.min(PANEL_HEIGHT).max( /* same safe min
*/ ) so the panel never becomes larger than body_rect; adjust uses of
PANEL_MARGIN/PANEL_WIDTH/PANEL_HEIGHT in panel_width and calc_panel_rect
accordingly.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7ca7cd77-f00c-4564-b688-776f22b4186c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
Cargo.tomlsrc/git/repo.rssrc/ui/commit_panel.rs
- Handle unborn branch in unstage_file (repo.rs) - Support deleted files in stage_file (repo.rs) - Propagate errors in discard_all instead of swallowing (repo.rs) - Remove dead Ready/Commit footer buttons (commit_panel.rs) - Fix UTF-8 unsafe byte slicing in truncate_path (commit_panel.rs) - Clamp panel dimensions to available viewport space (commit_panel.rs)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces a foundational implementation of Git repository access and modeling, along with related infrastructure and dependency updates. The main focus is on enabling structured access to Git data using the
git2crate, providing error handling, and defining models for key Git concepts. It also removes the oldcommit_panelUI component and updates project metadata and dependencies.Git Integration and Modeling:
Introduced a new
gitmodule with submodules for error handling (error.rs), data models (models.rs), and repository access (repo.rs). TheGitRepostruct provides methods to open a repository, fetch commits, branches, remotes, tags, stashes, and repository status, using thegit2crate. Comprehensive error handling is provided via the newGitErrorenum, with conversion fromgit2and IO errors. Unit tests are included for both error handling and data models. [1] [2] [3] [4]Added the new
gitmodule to the crate root insrc/lib.rsfor public use, alongside newlogger,state, anduimodules.Dependency and Build Updates:
Updated
Cargo.tomlto bump the version to0.0.13and add new dependencies:git2,tracing,tracing-subscriber,rfd, andzed. These support Git access, logging, and UI/file dialog functionality. [1] [2]Added a
devtarget to theMakefilefor running the project in release mode.UI Cleanup:
src/components/commit_panel.rs, likely in preparation for a new or refactored UI that will leverage the new Git data models.References:
[1] [2] [3] [4] [5] [6] [7] [8] [9]
Summary by CodeRabbit
New Features
Improvements
Chores