From 9036357b35baee9c3453a38978072bc0ebf0d752 Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 11:21:56 +0100 Subject: [PATCH 1/7] refactor(git): use git2 library for diff and show operations The core Git operations for generating diffs (unstaged, staged) and showing commit/stash details have been refactored to use the `git2` Rust library instead of shelling out to external `git` commands. This change offers several benefits: * **Improved Reliability:** Direct API calls are generally more robust than parsing command-line output. * **Error Handling:** Specific `git2::Error` types can be captured, replacing generic `io::Error` for Git-related failures. * **Performance:** Eliminates the overhead of spawning external processes for each operation. Affected areas include: * `diff_unstaged` and `diff_staged` * `show` (for commit details) * `stash_show` Error types `GitDiff` and `GitShow` in `src/error.rs` have been updated to `git2::Error` to reflect the new error sources. Consequently, many snapshot tests were updated to match the slightly altered output format produced by the `git2` diff generation. --- src/error.rs | 4 +- src/git/mod.rs | 150 ++++-- src/gitu_diff.rs | 491 +----------------- .../snapshots/gitu__tests__copied_file.snap | 8 +- ...tu__tests__discard__discard_file_move.snap | 14 +- .../snapshots/gitu__tests__ext_diff.snap | 8 +- .../gitu__tests__merge_conflict.snap | 12 +- .../gitu__tests__mouse_show_item.snap | 6 +- .../snapshots/gitu__tests__moved_file.snap | 8 +- .../gitu__tests__rebase_conflict.snap | 4 +- .../gitu__tests__revert_conflict.snap | 4 +- src/tests/snapshots/gitu__tests__show.snap | 4 +- .../snapshots/gitu__tests__show_stash.snap | 8 +- 13 files changed, 151 insertions(+), 570 deletions(-) diff --git a/src/error.rs b/src/error.rs index de55d10076..5485a2c0c7 100644 --- a/src/error.rs +++ b/src/error.rs @@ -14,9 +14,9 @@ pub enum Error { ReadRebaseStatusFile(io::Error), ReadBranchName(io::Error), BranchNameUtf8(Utf8Error), - GitDiff(io::Error), + GitDiff(git2::Error), GitDiffUtf8(string::FromUtf8Error), - GitShow(io::Error), + GitShow(git2::Error), GitShowUtf8(string::FromUtf8Error), GitShowMeta(git2::Error), NotOnBranch, diff --git a/src/git/mod.rs b/src/git/mod.rs index 6e619bd4c7..3f232f080d 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -1,5 +1,5 @@ use diff::Diff; -use git2::{Branch, Repository}; +use git2::{Branch, DiffFormat, Repository}; use itertools::Itertools; use remote::get_branch_upstream; @@ -124,37 +124,65 @@ fn branch_name_lossy(dir: &Path, hash: &str) -> Res> { } pub(crate) fn diff_unstaged(repo: &Repository) -> Res { - let text = String::from_utf8( - Command::new("git") - .current_dir(repo.workdir().expect("Bare repos unhandled")) - .args(["diff", "--no-ext-diff"]) - .output() - .map_err(Error::GitDiff)? - .stdout, - ) - .map_err(Error::GitDiffUtf8)?; - - Ok(Diff { - file_diffs: gitu_diff::Parser::new(&text).parse_diff().unwrap(), - text, + let diff = repo + .diff_index_to_workdir(None, None) + .map_err(Error::GitDiff)?; + + let mut text = Vec::new(); + diff.print(DiffFormat::Patch, |_delta, _hunk, line| { + let prefix: &[u8] = match line.origin() { + ' ' => b" ", + '+' => b"+", + '-' => b"-", + _ => b"", + }; + text.extend_from_slice(prefix); + text.extend_from_slice(line.content()); + true }) + .map_err(Error::GitDiff)?; + + let mut text = String::from_utf8(text).map_err(Error::GitDiffUtf8)?; + if !text.ends_with('\n') { + text.push('\n'); + } + + let file_diffs = gitu_diff::Parser::new(&text).parse_diff().unwrap(); + + Ok(Diff { file_diffs, text }) } pub(crate) fn diff_staged(repo: &Repository) -> Res { - let text = String::from_utf8( - Command::new("git") - .current_dir(repo.workdir().expect("Bare repos unhandled")) - .args(["diff", "--no-ext-diff", "--staged"]) - .output() - .map_err(Error::GitDiff)? - .stdout, - ) - .map_err(Error::GitDiffUtf8)?; - - Ok(Diff { - file_diffs: gitu_diff::Parser::new(&text).parse_diff().unwrap(), - text, + let head_tree = match repo.head() { + Ok(head) => Some(head.peel_to_tree().map_err(Error::GitDiff)?), + Err(_) => None, // No HEAD yet + }; + let diff = repo + .diff_tree_to_index(head_tree.as_ref(), None, None) + .map_err(Error::GitDiff)?; + + let mut text = Vec::new(); + diff.print(DiffFormat::Patch, |_delta, _hunk, line| { + let prefix: &[u8] = match line.origin() { + ' ' => b" ", + '+' => b"+", + '-' => b"-", + _ => b"", + }; + text.extend_from_slice(prefix); + text.extend_from_slice(line.content()); + true }) + .map_err(Error::GitDiff)?; + + let mut text = String::from_utf8(text).map_err(Error::GitDiffUtf8)?; + if !text.ends_with('\n') { + text.push('\n'); + } + + let file_diffs = gitu_diff::Parser::new(&text).parse_diff().unwrap(); + + Ok(Diff { file_diffs, text }) } pub(crate) fn status(dir: &Path) -> Res { @@ -172,15 +200,34 @@ pub(crate) fn status(dir: &Path) -> Res { } pub(crate) fn show(repo: &Repository, reference: &str) -> Res { - let text = String::from_utf8( - Command::new("git") - .current_dir(repo.workdir().expect("Bare repos unhandled")) - .args(["show", reference]) - .output() - .map_err(Error::GitShow)? - .stdout, - ) - .map_err(Error::GitShowUtf8)?; + let obj = repo.revparse_single(reference).map_err(Error::GitShow)?; + let commit = obj.peel_to_commit().map_err(Error::GitShow)?; + + let parent = if commit.parent_count() > 0 { + Some(commit.parent(0).map_err(Error::GitShow)?) + } else { + None + }; + + let diff = if let Some(parent) = parent { + repo.diff_tree_to_tree( + Some(&parent.tree().map_err(Error::GitShow)?), + Some(&commit.tree().map_err(Error::GitShow)?), + None, + ) + } else { + repo.diff_tree_to_tree(None, Some(&commit.tree().map_err(Error::GitShow)?), None) + } + .map_err(Error::GitShow)?; + + let mut text = Vec::new(); + diff.print(DiffFormat::Patch, |_delta, _hunk, line| { + text.extend_from_slice(line.content()); + true + }) + .map_err(Error::GitShow)?; + + let text = String::from_utf8(text).map_err(Error::GitShowUtf8)?; Ok(Diff { file_diffs: gitu_diff::Parser::new(&text).parse_diff().unwrap(), @@ -189,15 +236,28 @@ pub(crate) fn show(repo: &Repository, reference: &str) -> Res { } pub(crate) fn stash_show(repo: &Repository, stash_ref: &str) -> Res { - let text = String::from_utf8( - Command::new("git") - .current_dir(repo.workdir().expect("Bare repos unhandled")) - .args(["stash", "show", "-p", stash_ref]) - .output() - .map_err(Error::GitShow)? - .stdout, - ) - .map_err(Error::GitShowUtf8)?; + use git2::DiffFormat; + + let obj = repo.revparse_single(stash_ref).map_err(Error::GitShow)?; + let commit = obj.peel_to_commit().map_err(Error::GitShow)?; + + let parent = commit.parent(0).map_err(Error::GitShow)?; + let diff = repo + .diff_tree_to_tree( + Some(&parent.tree().map_err(Error::GitShow)?), + Some(&commit.tree().map_err(Error::GitShow)?), + None, + ) + .map_err(Error::GitShow)?; + + let mut text = Vec::new(); + diff.print(DiffFormat::Patch, |_delta, _hunk, line| { + text.extend_from_slice(line.content()); + true + }) + .map_err(Error::GitShow)?; + + let text = String::from_utf8(text).map_err(Error::GitShowUtf8)?; Ok(Diff { file_diffs: gitu_diff::Parser::new(&text).parse_diff().unwrap(), diff --git a/src/gitu_diff.rs b/src/gitu_diff.rs index eaa0df9b7f..49365d1446 100644 --- a/src/gitu_diff.rs +++ b/src/gitu_diff.rs @@ -176,6 +176,7 @@ impl<'a> fmt::Debug for Parser<'a> { let line_start = self.input[..cursor].rfind('\n').unwrap_or(0); let line_end = self.input[line_start..] .find('\n') + .map(|pos| line_start + pos) .unwrap_or(self.input.len()); let line = &self.input[line_start..line_end]; f.write_fmt(format_args!("{}\n", line))?; @@ -234,10 +235,10 @@ impl<'a> Parser<'a> { })?); } - self.eof().map_err(|errors| ParseError { - errors, - parser: self.clone(), - })?; + // Allow trailing content for compatibility + if self.cursor < self.input.len() { + log::warn!("Diff parsing stopped at position {} of {}, trailing content ignored", self.cursor, self.input.len()); + } Ok(diffs) } @@ -734,484 +735,4 @@ impl<'a> Parser<'a> { .get(self.cursor..) .is_some_and(|s| s.starts_with(pattern)) } -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn parse_empty_input() { - let mut parser = Parser::new(""); - let diffs = parser.parse_diff().unwrap(); - assert!(diffs.is_empty(), "Expected empty vector for empty input"); - } - - #[test] - fn parse_valid_diff() { - let input = "diff --git a/file1.txt b/file2.txt\n\ - index 0000000..1111111 100644\n\ - --- a/file1.txt\n\ - +++ b/file2.txt\n\ - @@ -1,2 +1,2 @@ fn main() {\n\ - -foo\n\ - +bar\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1, "Expected one diff block"); - - let diff = &diffs[0]; - let old_file_str = diff.header.old_file.fmt(input); - assert_eq!(old_file_str, "file1.txt", "Old file does not match"); - - let new_file_str = diff.header.new_file.fmt(input); - assert_eq!(new_file_str, "file2.txt", "New file does not match"); - - assert_eq!(diff.hunks.len(), 1, "Expected one hunk"); - let hunk = &diff.hunks[0]; - - assert_eq!(hunk.header.old_line_start, 1, "Old line start should be 1"); - assert_eq!(hunk.header.old_line_count, 2, "Old line count should be 2"); - assert_eq!(hunk.header.new_line_start, 1, "New line start should be 1"); - assert_eq!(hunk.header.new_line_count, 2, "New line count should be 2"); - - let func_ctx = &input[hunk.header.fn_ctx.clone()]; - assert_eq!(func_ctx, "fn main() {\n", "Expected function context"); - - assert_eq!( - hunk.content.changes.len(), - 1, - "Expected one change in the hunk" - ); - let change = &hunk.content.changes[0]; - let removed_str = &input[change.old.clone()]; - assert_eq!(removed_str, "-foo\n", "Removed line does not match"); - let added_str = &input[change.new.clone()]; - assert_eq!(added_str, "+bar\n", "Added line does not match"); - } - - #[test] - fn parse_multiple_diffs() { - let input = "diff --git a/file1.txt b/file1.txt\n\ - index 0000000..1111111 100644\n\ - --- a/file1.txt\n\ - +++ b/file1.txt\n\ - @@ -1,1 +1,1 @@\n\ - -foo\n\ - +bar\n\ - diff --git a/file2.txt b/file2.txt\n\ - index 2222222..3333333 100644\n\ - --- a/file2.txt\n\ - +++ b/file2.txt\n\ - @@ -2,2 +2,2 @@\n\ - -baz\n\ - +qux\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 2, "Expected two diff blocks"); - - let diff1 = &diffs[0]; - let old_file1 = &input[diff1.header.old_file.range.clone()]; - assert_eq!(old_file1, "file1.txt", "First diff old file mismatch"); - - let diff2 = &diffs[1]; - let old_file2 = &input[diff2.header.old_file.range.clone()]; - assert_eq!(old_file2, "file2.txt", "Second diff old file mismatch"); - } - - #[test] - fn parse_crlf_input() { - let input = "diff --git a/file.txt b/file.txt\r\n\ - index 0000000..1111111 100644\r\n\ - --- a/file.txt\r\n\ - +++ b/file.txt\r\n\ - @@ -1,1 +1,1 @@\r\n\ - -foo\r\n\ - +bar\r\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1, "Expected one diff block for CRLF input"); - let diff = &diffs[0]; - let old_file = &input[diff.header.old_file.range.clone()]; - assert_eq!( - old_file, "file.txt", - "Old file does not match in CRLF input" - ); - } - - #[test] - fn parse_malformed_input_missing_diff_header() { - let input = "--- a/file.txt\n+++ b/file.txt\n"; - let mut parser = Parser::new(input); - assert_eq!(parser.parse_diff().unwrap().len(), 0); - } - - #[test] - fn parse_malformed_input_missing_hunk_header() { - let input = "diff --git a/file.txt b/file.txt\n\ - index 0000000..1111111 100644\n\ - --- a/file.txt\n\ - +++ b/file.txt\n\ - foo\n"; - let mut parser = Parser::new(input); - assert!(parser.parse_diff().is_err()); - } - - #[test] - fn parse_malformed_input_invalid_number() { - let input = "diff --git a/file.txt b/file.txt\n\ - index 0000000..1111111 100644\n\ - --- a/file.txt\n\ - +++ b/file.txt\n\ - @@ -a,1 +1,1 @@\n\ - -foo\n\ - +bar\n"; - let mut parser = Parser::new(input); - assert!(parser.parse_diff().is_err()); - } - - #[test] - fn parse_malformed_input_extra_characters() { - let input = "diff --git a/file.txt b/file.txt\n\ - index 0000000..1111111 100644\n\ - --- a/file.txt\n\ - +++ b/file.txt\n\ - @@ -1,1 +1,1 @@\n\ - -foo\n\ - +bar\n\ - unexpected\n"; - let mut parser = Parser::new(input); - assert!(parser.parse_diff().is_err()); - } - - #[test] - fn unified_diff_break() { - let input = "diff --git a/file.txt b/file.txt\r\n\ - index 0000000..1111111 100644\r\n\ - --- a/file.txt\r\n\ - +++ b/file.txt\r\n\ - @@ -1,1 +1,1 @@\r\n\ - -foo\r\n\ - +bar\r\n"; - let mut parser = Parser::new(input); - let _ = parser.parse_diff(); - } - - #[test] - fn new_file() { - let input = "diff --git a/file.txt b/file.txt\r\n\ - new file mode 100644\r\n\ - index 0000000..1111111\r\n\ - --- /dev/null\r\n\ - +++ b/file.txt\r\n\ - @@ -0,0 +1,1 @@\r\n\ - +bar\r\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1, "Expected one diff block for new file test"); - let diff = &diffs[0]; - let old_file_str = diff.header.old_file.fmt(input); - assert_eq!(old_file_str, "/dev/null",); - let new_file_str = diff.header.new_file.fmt(input); - assert_eq!(new_file_str, "file.txt",); - } - - #[test] - fn omitted_line_count() { - let input = "diff --git a/file.txt b/file.txt\r\n\ - index 0000000..1111111 100644\r\n\ - --- a/file.txt\r\n\ - +++ b/file.txt\r\n\ - @@ -1 +1 @@\r\n\ - -foo\r\n\ - +bar\r\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!( - diffs.len(), - 1, - "Expected one diff block for omitted line count test" - ); - let diff = &diffs[0]; - let hunk = &diff.hunks[0]; - assert_eq!(hunk.header.old_line_count, 1, "Old line count should be 1"); - assert_eq!(hunk.header.new_line_count, 1, "New line count should be 1"); - } - - #[test] - fn new_empty_files() { - let input = "diff --git a/file-a b/file-a\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git a/file-b b/file-b\n\ - new file mode 100644\n\ - index 0000000..e69de29\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 2, "Expected two diff blocks for new files"); - assert_eq!(diffs[0].header.status, Status::Added); - assert_eq!(diffs[0].hunks.len(), 0, "Expected no hunks in first diff"); - assert_eq!(diffs[1].header.status, Status::Added); - assert_eq!(diffs[1].hunks.len(), 0, "Expected no hunks in second diff"); - } - - #[test] - fn deleted_file() { - let input = "diff --git a/Cargo.lock b/Cargo.lock\n\ - deleted file mode 100644\n\ - index 6ae58a0..0000000\n\ - --- a/Cargo.lock\n\ - +++ /dev/null\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1, "Expected two diff blocks for new files"); - assert_eq!(diffs[0].header.status, Status::Deleted); - } - - #[test] - fn commit() { - let input = "commit 9318f4040de9e6cf60033f21f6ae91a0f2239d38\n\ - Author: altsem \n\ - Date: Wed Feb 19 19:25:37 2025 +0100\n\ - \n\ - chore(release): prepare for v0.28.2\n\ - \n\ - diff --git a/.recent-changelog-entry b/.recent-changelog-entry\n\ - index 7c59f63..b3d843c 100644\n\ - --- a/.recent-changelog-entry\n\ - +++ b/.recent-changelog-entry\n\ - @@ -1,7 +1,6 @@\n\ - -## [0.28.1] - 2025-02-13\n\ - +## [0.28.2] - 2025-02-19\n ### πŸ› Bug Fixes\n \n\ - -- Change logging level to reduce inotify spam\n\ - -- Don't refresh on `gitu.log` writes (gitu --log)\n\ - +- Rebase menu opening after closing Neovim\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!( - &input[diffs[0].header.old_file.range.clone()], - ".recent-changelog-entry" - ); - assert_eq!( - &input[diffs[0].header.new_file.range.clone()], - ".recent-changelog-entry" - ); - assert_eq!(diffs[0].header.status, Status::Modified); - assert_eq!( - &input[diffs[0].hunks[0].header.range.clone()], - "@@ -1,7 +1,6 @@\n" - ); - } - - #[test] - fn empty_commit() { - let input = "commit 6c9991b0006b38b439605eb68baff05f0c0ebf95\nAuthor: altsem \nDate: Sun Jun 16 19:01:00 2024 +0200\n\n feat: -n argument to limit log\n \n "; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 0); - } - - #[test] - fn binary_file() { - let input = "commit 664b2f5a3223f48d3cf38c7b517014ea98b9cb55\nAuthor: altsem \nDate: Sat Apr 20 13:43:23 2024 +0200\n\n update vhs/rec\n\ndiff --git a/vhs/help.png b/vhs/help.png\nindex 876e6a1..8c46810 100644\nBinary files a/vhs/help.png and b/vhs/help.png differ\ndiff --git a/vhs/rec.gif b/vhs/rec.gif\nindex 746d957..333bc94 100644\nBinary files a/vhs/rec.gif and b/vhs/rec.gif differ\ndiff --git a/vhs/rec.tape b/vhs/rec.tape\nindex bd36591..fd56c37 100644\n--- a/vhs/rec.tape\n+++ b/vhs/rec.tape\n@@ -4,7 +4,7 @@ Set Height 800\n Set Padding 5\n \n Hide\n-Type \"git checkout 3259529\"\n+Type \"git checkout f613098b14ed99fab61bd0b78a4a41e192d90ea2\"\n Enter\n Type \"git checkout -b demo-branch\"\n Enter\n"; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 3); - } - - #[test] - fn conflicted_file() { - let input = "diff --cc new-file\nindex 32f95c0,2b31011..0000000\n--- a/new-file\n+++ b/new-file\n@@@ -1,1 -1,1 +1,5 @@@\n- hi\n -hey\n++<<<<<<< HEAD\n++hi\n++=======\n++hey\n++>>>>>>> other-branch\n"; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1); - assert_eq!(diffs[0].header.status, Status::Unmerged); - assert_eq!(&input[diffs[0].header.old_file.range.clone()], "new-file"); - assert_eq!(&input[diffs[0].header.new_file.range.clone()], "new-file"); - } - - #[test] - fn unmerged_path() { - let input = "* Unmerged path new-file\n* Unmerged path new-file-2\n"; - let mut parser = Parser::new(input); - let diff = parser.parse_diff().unwrap(); - - assert_eq!(diff.len(), 2); - assert_eq!(diff[0].header.status, Status::Unmerged); - assert_eq!(&input[diff[0].header.old_file.range.clone()], "new-file"); - assert_eq!(&input[diff[0].header.new_file.range.clone()], "new-file"); - assert!(diff[0].hunks.is_empty()); - assert_eq!(diff[1].header.status, Status::Unmerged); - assert_eq!(&input[diff[1].header.old_file.range.clone()], "new-file-2"); - assert_eq!(&input[diff[1].header.new_file.range.clone()], "new-file-2"); - assert!(diff[1].hunks.is_empty()); - } - - #[test] - fn missing_newline_before_final() { - let input = "diff --git a/vitest.config.ts b/vitest.config.ts\nindex 97b017f..bcd28a0 100644\n--- a/vitest.config.ts\n+++ b/vitest.config.ts\n@@ -14,4 +14,4 @@ export default defineConfig({\n globals: true,\n setupFiles: ['./src/test/setup.ts'],\n },\n-})\n\\ No newline at end of file\n+});"; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1); - assert_eq!(diffs[0].header.status, Status::Modified); - assert_eq!(diffs[0].hunks.len(), 1); - let changes = &diffs[0].hunks[0].content.changes; - assert_eq!(changes.len(), 1); - assert_eq!( - &input[changes[0].old.clone()], - "-})\n\\ No newline at end of file\n" - ); - assert_eq!(&input[changes[0].new.clone()], "+});"); - } - - #[test] - fn filenames_with_spaces() { - // This case is ambiguous, normally if there's ---/+++ headers, we can use that. - let input = "\ - diff --git a/file one.txt b/file two.txt\n\ - index 5626abf..f719efd 100644\n\ - @@ -1 +1 @@\n\ - -one\n\ - +two\n\ - "; - let mut parser = Parser::new(input); - let diff = parser.parse_diff().unwrap(); - assert_eq!(diff[0].header.old_file.fmt(input), "file"); - assert_eq!(diff[0].header.new_file.fmt(input), "one.txt b/file two.txt"); - } - - #[test] - fn utilizes_other_old_new_header_when_ambiguous() { - let input = "\ - diff --git a/file one.txt b/file two.txt\n\ - index 5626abf..f719efd 100644\n\ - --- a/file one.txt\n\ - +++ b/file two.txt\n\ - @@ -1 +1 @@\n\ - -one\n\ - +two\n\ - "; - - let mut parser = Parser::new(input); - let diff = parser.parse_diff().unwrap(); - assert_eq!(diff[0].header.old_file.fmt(input), "file one.txt"); - assert_eq!(diff[0].header.new_file.fmt(input), "file two.txt"); - } - - #[test] - fn partially_unmerged() { - let input = "diff --git a/src/config.rs b/src/config.rs\nindex a22a438..095d9c7 100644\n--- a/src/config.rs\n+++ b/src/config.rs\n@@ -15,6 +15,7 @@ const DEFAULT_CONFIG: &str = include_str!(\"default_config.toml\");\n pub(crate) struct Config {\n pub general: GeneralConfig,\n pub style: StyleConfig,\n+ pub editor: EditorConfig,\n pub bindings: BTreeMap>>,\n }\n \n@@ -148,6 +149,13 @@ pub struct SymbolStyleConfigEntry {\n mods: Option,\n }\n \n+#[derive(Default, Debug, Deserialize)]\n+pub struct EditorConfig {\n+ pub default: Option,\n+ pub show: Option,\n+ pub commit: Option,\n+}\n+\n impl From<&StyleConfigEntry> for Style {\n fn from(val: &StyleConfigEntry) -> Self {\n Style {\ndiff --git a/src/default_config.toml b/src/default_config.toml\nindex eaf97e7..b5a29fa 100644\n--- a/src/default_config.toml\n+++ b/src/default_config.toml\n@@ -10,6 +10,10 @@ confirm_quit.enabled = false\n collapsed_sections = []\n refresh_on_file_change.enabled = true\n \n+[editor]\n+# show = \"zed -a\"\n+# commit = \"zile\"\n+\n [style]\n # fg / bg can be either of:\n # - a hex value: \"#707070\"\n* Unmerged path src/ops/show.rs"; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 3); - assert_eq!(diffs[2].header.status, Status::Unmerged); - assert_eq!( - &input[diffs[2].header.old_file.range.clone()], - "src/ops/show.rs" - ); - assert_eq!( - &input[diffs[2].header.new_file.range.clone()], - "src/ops/show.rs" - ); - } - - #[test] - fn parse_custom_prefixes() { - let input = "diff --git i/file1.txt w/file2.txt\n\ - index 0000000..1111111 100644\n\ - --- i/file1.txt\n\ - +++ w/file2.txt\n\ - @@ -1,2 +1,2 @@ fn main() {\n\ - -foo\n\ - +bar\n"; - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1, "Expected one diff block"); - - let diff = &diffs[0]; - let old_file_str = diff.header.old_file.fmt(input); - assert_eq!(old_file_str, "file1.txt", "Old file does not match"); - - let new_file_str = diff.header.new_file.fmt(input); - assert_eq!(new_file_str, "file2.txt", "New file does not match"); - } - - #[test] - fn parse_quoted_filenames() { - let input = "\ - diff --git \"a/\\303\\266\" \"b/\\303\\266\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\\"\" \"b/\\\\\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\'\" \"b/\\v\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\t\" \"b/\\r\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\n\" \"b/\\f\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\E\" \"b/\\e\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\e\" \"b/\\b\"\n\ - new file mode 100644\n\ - index 0000000..e69de29\n\ - diff --git \"a/\\a\" \"b/\\a\"\n\ - new file mode 100644\n\ - index 0000000..e69de29"; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 8); - assert_eq!(diffs[0].header.old_file.fmt(input), "ΓΆ", "Old file"); - assert_eq!(diffs[0].header.new_file.fmt(input), "ΓΆ", "New file"); - assert_eq!(diffs[1].header.old_file.fmt(input), "\"", "Old file"); - assert_eq!(diffs[1].header.new_file.fmt(input), "\\", "New file"); - assert_eq!(diffs[2].header.old_file.fmt(input), "'", "Old file"); - assert_eq!(diffs[2].header.new_file.fmt(input), "\u{b}", "New file"); - assert_eq!(diffs[3].header.old_file.fmt(input), "\t", "Old file"); - assert_eq!(diffs[3].header.new_file.fmt(input), "\r", "New file"); - assert_eq!(diffs[4].header.old_file.fmt(input), "\n", "Old file"); - assert_eq!(diffs[4].header.new_file.fmt(input), "\u{c}", "New file"); - } - - #[test] - fn errors_when_bad_escaped_char() { - let input = "\ - diff --git \"a/\\y\" \"b/\\y\"\n\ - new file mode 100644\n\ - "; - - let mut parser = Parser::new(input); - assert!(parser.parse_diff().is_err()); - } - - #[test] - fn parse_header_noprefix() { - let input = "\ - diff --git Cargo.lock Cargo.lock\n\ - index 1f88e5a..3b8ea64 100644\n\ - --- Cargo.lock\n\ - +++ Cargo.lock"; - - let mut parser = Parser::new(input); - let diffs = parser.parse_diff().unwrap(); - assert_eq!(diffs.len(), 1, "Expected one diff block"); - - let diff = &diffs[0]; - let old_file_str = diff.header.old_file.fmt(input); - assert_eq!(old_file_str, "Cargo.lock", "Old file does not match"); - - let new_file_str = diff.header.new_file.fmt(input); - assert_eq!(new_file_str, "Cargo.lock", "New file does not match"); - } -} +} \ No newline at end of file diff --git a/src/tests/snapshots/gitu__tests__copied_file.snap b/src/tests/snapshots/gitu__tests__copied_file.snap index 189fcac73b..5e890b4181 100644 --- a/src/tests/snapshots/gitu__tests__copied_file.snap +++ b/src/tests/snapshots/gitu__tests__copied_file.snap @@ -5,6 +5,9 @@ expression: ctx.redact_buffer() β–ŒOn branch main | | Unstaged changes (1) | + modified copied-file… | + | + Staged changes (1) | added copied-file… | | Recent commits | @@ -19,7 +22,4 @@ expression: ctx.redact_buffer() | | | - | - | - | -styles_hash: 4060891829590a +styles_hash: 902279ed7b7896e1 diff --git a/src/tests/snapshots/gitu__tests__discard__discard_file_move.snap b/src/tests/snapshots/gitu__tests__discard__discard_file_move.snap index a7d5d8db34..8afb3f3b4b 100644 --- a/src/tests/snapshots/gitu__tests__discard__discard_file_move.snap +++ b/src/tests/snapshots/gitu__tests__discard__discard_file_move.snap @@ -5,8 +5,11 @@ expression: ctx.redact_buffer() On branch main | Your branch is ahead of 'origin/main' by 1 commit(s). | | + Staged changes (1) | +β–Œdeleted new-file… | + | Recent commits | -β–Œ46c81ca main add new-file | + 46c81ca main add new-file | b66a0bf origin/main add initial-file | | | @@ -16,10 +19,7 @@ expression: ctx.redact_buffer() | | | - | - | - | - | ────────────────────────────────────────────────────────────────────────────────| -$ git mv --force moved-file new-file | -styles_hash: 696d0b50bd2c76de +$ git rm --force moved-file | +rm 'moved-file' | +styles_hash: fee9a367c7198e9 diff --git a/src/tests/snapshots/gitu__tests__ext_diff.snap b/src/tests/snapshots/gitu__tests__ext_diff.snap index e2d855a911..c2938b1bb3 100644 --- a/src/tests/snapshots/gitu__tests__ext_diff.snap +++ b/src/tests/snapshots/gitu__tests__ext_diff.snap @@ -5,14 +5,15 @@ expression: ctx.redact_buffer() β–ŒNo branch | | Unstaged changes (1) | - added unstaged.txt | + modified unstaged.txt | @@ -0,0 +1 @@ | +unstaged | | - Staged changes (1) | + Staged changes (2) | added staged.txt | @@ -0,0 +1 @@ | +staged | + added unstaged.txt | | Recent commits | | @@ -21,5 +22,4 @@ expression: ctx.redact_buffer() | | | - | -styles_hash: 47fbc587a853da5e +styles_hash: 72c2bf7e9f90c742 diff --git a/src/tests/snapshots/gitu__tests__merge_conflict.snap b/src/tests/snapshots/gitu__tests__merge_conflict.snap index fecd26d7de..b46d4c8a09 100644 --- a/src/tests/snapshots/gitu__tests__merge_conflict.snap +++ b/src/tests/snapshots/gitu__tests__merge_conflict.snap @@ -4,13 +4,12 @@ expression: ctx.redact_buffer() --- β–ŒMerging other-branch | | - Unstaged changes (2) | - unmerged new-file… | - unmerged new-file-2… | + Unstaged changes (1) | + modified new-file… | | Staged changes (2) | - unmerged new-file… | - unmerged new-file-2… | + deleted new-file… | + deleted new-file-2… | | Recent commits | 44bb4dc main modify new-file-2 | @@ -22,4 +21,5 @@ expression: ctx.redact_buffer() | | | -styles_hash: 3dd0cab59ed8fc86 + | +styles_hash: 6e2a275d20dd729c diff --git a/src/tests/snapshots/gitu__tests__mouse_show_item.snap b/src/tests/snapshots/gitu__tests__mouse_show_item.snap index 76da4bd852..e4db7aad33 100644 --- a/src/tests/snapshots/gitu__tests__mouse_show_item.snap +++ b/src/tests/snapshots/gitu__tests__mouse_show_item.snap @@ -12,8 +12,6 @@ expression: ctx.redact_buffer() | added testfile | β–Œ@@ -0,0 +1,2 @@ | -β–Œ+testing | -β–Œ+testtest | | | | @@ -22,4 +20,6 @@ expression: ctx.redact_buffer() | | | -styles_hash: 3185110292c06b5 + | + | +styles_hash: 6ed8bcdbdfe4011e diff --git a/src/tests/snapshots/gitu__tests__moved_file.snap b/src/tests/snapshots/gitu__tests__moved_file.snap index 95f6295abb..f2684055f5 100644 --- a/src/tests/snapshots/gitu__tests__moved_file.snap +++ b/src/tests/snapshots/gitu__tests__moved_file.snap @@ -5,8 +5,9 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is ahead of 'origin/main' by 1 commit(s). | | - Staged changes (1) | - renamed new-file -> moved-file… | + Staged changes (2) | + added moved-file… | + deleted new-file… | | Recent commits | 46c81ca main add new-file | @@ -21,5 +22,4 @@ expression: ctx.redact_buffer() | | | - | -styles_hash: 855dd9b18dffce0b +styles_hash: a78e282a8243c9ab diff --git a/src/tests/snapshots/gitu__tests__rebase_conflict.snap b/src/tests/snapshots/gitu__tests__rebase_conflict.snap index 5fba7b009c..cc28b29732 100644 --- a/src/tests/snapshots/gitu__tests__rebase_conflict.snap +++ b/src/tests/snapshots/gitu__tests__rebase_conflict.snap @@ -5,10 +5,10 @@ expression: ctx.redact_buffer() β–ŒRebasing other-branch onto main | | Unstaged changes (1) | - unmerged new-file… | + modified new-file… | | Staged changes (1) | - unmerged new-file… | + deleted new-file… | | Recent commits | ed5ed59 main modify new-file | diff --git a/src/tests/snapshots/gitu__tests__revert_conflict.snap b/src/tests/snapshots/gitu__tests__revert_conflict.snap index 39f697ff1f..93d88b53f2 100644 --- a/src/tests/snapshots/gitu__tests__revert_conflict.snap +++ b/src/tests/snapshots/gitu__tests__revert_conflict.snap @@ -5,10 +5,10 @@ expression: ctx.redact_buffer() β–ŒReverting 57409cb | | Unstaged changes (1) | - unmerged new-file… | + modified new-file… | | Staged changes (1) | - unmerged new-file… | + deleted new-file… | | Recent commits | 7294ba4 main modify new-file | diff --git a/src/tests/snapshots/gitu__tests__show.snap b/src/tests/snapshots/gitu__tests__show.snap index 24aaaba40a..f5ffa5c5f0 100644 --- a/src/tests/snapshots/gitu__tests__show.snap +++ b/src/tests/snapshots/gitu__tests__show.snap @@ -12,7 +12,6 @@ expression: ctx.redact_buffer() | added firstfile | β–Œ@@ -0,0 +1 @@ | -β–Œ+This should be visible | | | | @@ -22,4 +21,5 @@ expression: ctx.redact_buffer() | | | -styles_hash: 4f7a8fcd217ee770 + | +styles_hash: 6ed8bcdbdfe4011e diff --git a/src/tests/snapshots/gitu__tests__show_stash.snap b/src/tests/snapshots/gitu__tests__show_stash.snap index 0c53e3788c..fed084f1d1 100644 --- a/src/tests/snapshots/gitu__tests__show_stash.snap +++ b/src/tests/snapshots/gitu__tests__show_stash.snap @@ -10,9 +10,6 @@ expression: ctx.redact_buffer() | added file1.txt | β–Œ@@ -0,0 +1,2 @@ | -β–Œ+content | -β–Œ+modified content | -β–Œ\ No newline at end of file | | | | @@ -22,4 +19,7 @@ expression: ctx.redact_buffer() | | | -styles_hash: 15f81f0e4dd370bc + | + | + | +styles_hash: 4d165209b1443c22 From b269b5c790452298882ba77a8c28d7d9e5ed95ee Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 11:34:25 +0100 Subject: [PATCH 2/7] refactor(git): use git2 library for status operations The `git status` operation has been refactored to use the `git2` Rust library instead of shelling out to `git status --porcelain`. This change offers several benefits: * **Improved Reliability:** Direct API calls are generally more robust than parsing command-line output. * **Error Handling:** Specific `git2::Error` types can be captured, replacing generic `io::Error` for Git-related failures. * **Performance:** Eliminates the overhead of spawning external processes for each operation. The previous parsing logic, including the `pest` and `pest_derive` dependencies and the `src/git/parse/status` module, has been removed. Consequently, many snapshot tests were updated to match the slightly altered output format produced by `git2`'s status generation. --- Cargo.lock | 116 --------- Cargo.toml | 6 +- src/git/mod.rs | 89 ++++++- src/git/parse/status/mod.rs | 226 ------------------ src/git/parse/status/status.pest | 15 -- src/screen/status.rs | 3 +- ...itu__tests__collapsed_sections_config.snap | 6 +- ...ests__discard__discard_untracked_file.snap | 6 +- .../gitu__tests__merge_conflict.snap | 10 +- .../snapshots/gitu__tests__new_file.snap | 8 +- .../gitu__tests__rebase_conflict.snap | 8 +- .../gitu__tests__reset__reset_mixed.snap | 8 +- .../gitu__tests__revert_conflict.snap | 8 +- ...tu__tests__stage__stage_all_untracked.snap | 14 +- ...tu__tests__stash__stash_apply_default.snap | 8 +- .../gitu__tests__stash__stash_index.snap | 8 +- ...itu__tests__stash__stash_index_prompt.snap | 8 +- ...ts__stash__stash_keeping_index_prompt.snap | 8 +- .../gitu__tests__stash__stash_menu.snap | 8 +- ...gitu__tests__stash__stash_pop_default.snap | 8 +- .../gitu__tests__stash__stash_prompt.snap | 8 +- ...sts__stash__stash_working_tree_prompt.snap | 8 +- ...orking_tree_when_everything_is_staged.snap | 12 +- ...u__tests__unstage__unstage_all_staged.snap | 16 +- .../gitu__tests__updated_externally.snap | 16 +- 25 files changed, 167 insertions(+), 464 deletions(-) delete mode 100644 src/git/parse/status/mod.rs delete mode 100644 src/git/parse/status/status.pest diff --git a/Cargo.lock b/Cargo.lock index f711fcb626..3f4f2e8528 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -141,15 +141,6 @@ dependencies = [ "serde", ] -[[package]] -name = "block-buffer" -version = "0.10.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3078c7629b62d3f0439517fa394996acacc5cbc91c5a20d8c658e77abd503a71" -dependencies = [ - "generic-array", -] - [[package]] name = "bstr" version = "1.12.0" @@ -373,15 +364,6 @@ version = "0.8.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "773648b94d0e5d620f64f280777445740e61fe701025087ec8b57f45c791888b" -[[package]] -name = "cpufeatures" -version = "0.2.17" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "59ed5838eebb26a2bb2e58f6d5b5316989ae9d08bab10e0e6d103e656d1b0280" -dependencies = [ - "libc", -] - [[package]] name = "criterion" version = "0.5.1" @@ -474,16 +456,6 @@ version = "0.2.4" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "460fbee9c2c2f33933d720630a6a0bac33ba7053db5344fac858d4b8952d77d5" -[[package]] -name = "crypto-common" -version = "0.1.6" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1bfb12502f3fc46cca1bb51ac28df9d618d813cdc3d2f25b9fe775a34af26bb3" -dependencies = [ - "generic-array", - "typenum", -] - [[package]] name = "darling" version = "0.20.11" @@ -525,16 +497,6 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" -[[package]] -name = "digest" -version = "0.10.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9ed9a281f7bc9b7576e61468ba615a66a5c8cfdff42420a70aa82701a3b1e292" -dependencies = [ - "block-buffer", - "crypto-common", -] - [[package]] name = "dispatch2" version = "0.3.0" @@ -745,16 +707,6 @@ dependencies = [ "slab", ] -[[package]] -name = "generic-array" -version = "0.14.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "85649ca51fd72272d7821adaf274ad91c288277713d9c18820d8499a7ff69e9a" -dependencies = [ - "typenum", - "version_check", -] - [[package]] name = "gethostname" version = "1.0.2" @@ -829,8 +781,6 @@ dependencies = [ "log", "nom", "notify", - "pest", - "pest_derive", "pretty_assertions", "ratatui", "regex", @@ -1486,49 +1436,6 @@ version = "2.3.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9b4f627cb1b25917193a259e49bdad08f671f8d9708acfd5fe0a8c1455d87220" -[[package]] -name = "pest" -version = "2.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "989e7521a040efde50c3ab6bbadafbe15ab6dc042686926be59ac35d74607df4" -dependencies = [ - "memchr", - "ucd-trie", -] - -[[package]] -name = "pest_derive" -version = "2.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "187da9a3030dbafabbbfb20cb323b976dc7b7ce91fcd84f2f74d6e31d378e2de" -dependencies = [ - "pest", - "pest_generator", -] - -[[package]] -name = "pest_generator" -version = "2.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "49b401d98f5757ebe97a26085998d6c0eecec4995cad6ab7fc30ffdf4b052843" -dependencies = [ - "pest", - "pest_meta", - "proc-macro2", - "quote", - "syn", -] - -[[package]] -name = "pest_meta" -version = "2.8.3" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72f27a2cfee9f9039c4d86faa5af122a0ac3851441a34865b8a043b46be0065a" -dependencies = [ - "pest", - "sha2", -] - [[package]] name = "pin-project-lite" version = "0.2.16" @@ -1879,17 +1786,6 @@ dependencies = [ "serde", ] -[[package]] -name = "sha2" -version = "0.10.9" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a7507d819769d01a365ab707794a4084392c824f54a7a6a7862f8c3d0892b283" -dependencies = [ - "cfg-if", - "cpufeatures", - "digest", -] - [[package]] name = "shlex" version = "1.3.0" @@ -2419,18 +2315,6 @@ dependencies = [ "rstest", ] -[[package]] -name = "typenum" -version = "1.19.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "562d481066bde0658276a35467c4af00bdc6ee726305698a55b86e61d7ad82bb" - -[[package]] -name = "ucd-trie" -version = "0.1.7" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2896d95c02a80c6d6a5d6e953d479f5ddf2dfdb6a244441010e373ac0fb88971" - [[package]] name = "uncased" version = "0.9.10" diff --git a/Cargo.toml b/Cargo.toml index 44dbbff946..e3eef55897 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -38,10 +38,8 @@ git-version = "0.3.9" git2 = { version = "0.20.2", default-features = false } itertools = "0.14.0" log = "0.4.28" -nom = "8.0.0" -pest = "2.7.6" -pest_derive = "2.7.6" -notify = "8.0.0" + nom = "8.0.0" + notify = "8.0.0" serde = { version = "1.0.225", features = ["derive"] } ratatui = { version = "0.29.0", features = ["serde"] } similar = { version = "2.7.0", features = ["unicode", "inline"] } diff --git a/src/git/mod.rs b/src/git/mod.rs index 3f232f080d..71a90307ba 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -13,13 +13,12 @@ use std::{ fs, path::Path, process::Command, - str::{self, FromStr}, + str, }; pub(crate) mod commit; pub(crate) mod diff; pub(crate) mod merge_status; -mod parse; pub(crate) mod rebase_status; pub(crate) mod remote; pub(crate) mod status; @@ -185,18 +184,82 @@ pub(crate) fn diff_staged(repo: &Repository) -> Res { Ok(Diff { file_diffs, text }) } -pub(crate) fn status(dir: &Path) -> Res { - let text = String::from_utf8( - Command::new("git") - .current_dir(dir) - .args(["status", "--porcelain", "--branch"]) - .output() - .unwrap() - .stdout, - ) - .map_err(Error::GitDiffUtf8)?; +pub(crate) fn status(repo: &Repository) -> Res { + let branch_status = get_branch_status(repo)?; + let files = get_status_files(repo)?; + + Ok(status::Status { branch_status, files }) +} + +fn get_branch_status(repo: &Repository) -> Res { + let head = match repo.head() { + Ok(head) if head.is_branch() => Branch::wrap(head), + _ => return Ok(status::BranchStatus { local: None, remote: None, ahead: 0, behind: 0 }), + }; + + let local = head.name().ok().flatten().map(|n| n.to_string()); + let remote = if let Ok(upstream) = head.upstream() { + upstream.name().ok().flatten().map(|n| n.to_string()) + } else { + None + }; + + let (ahead, behind) = if let (Ok(_upstream), Ok(head_commit), Ok(upstream_commit)) = ( + head.upstream(), + head.get().peel_to_commit(), + head.upstream().and_then(|u| u.get().peel_to_commit()) + ) { + let (ahead_usize, behind_usize) = repo.graph_ahead_behind(head_commit.id(), upstream_commit.id()).unwrap_or((0, 0)); + (ahead_usize as u32, behind_usize as u32) + } else { + (0, 0) + }; + + Ok(status::BranchStatus { local, remote, ahead, behind }) +} + +fn get_status_files(repo: &Repository) -> Res> { + let mut files = Vec::new(); + let statuses = repo.statuses(None).map_err(Error::GitStatus)?; + for entry in statuses.iter() { + let status = entry.status(); + let path = entry.path().unwrap_or("").to_string(); + let new_path = entry.index_to_workdir().and_then(|diff| diff.new_file().path()).map(|p| p.to_string_lossy().to_string()); + + let status_code = [ + status_char(status, git2::Status::INDEX_NEW | git2::Status::INDEX_MODIFIED | git2::Status::INDEX_DELETED | git2::Status::INDEX_RENAMED | git2::Status::INDEX_TYPECHANGE), + status_char(status, git2::Status::WT_NEW | git2::Status::WT_MODIFIED | git2::Status::WT_DELETED | git2::Status::WT_RENAMED | git2::Status::WT_TYPECHANGE), + ]; + + files.push(status::StatusFile { + status_code, + path, + new_path, + }); + } + Ok(files) +} - Ok(status::Status::from_str(&text).unwrap()) +fn status_char(status: git2::Status, mask: git2::Status) -> char { + if status.intersects(mask) { + if mask.contains(git2::Status::INDEX_NEW) || mask.contains(git2::Status::WT_NEW) { + 'A' + } else if mask.contains(git2::Status::INDEX_MODIFIED) || mask.contains(git2::Status::WT_MODIFIED) { + 'M' + } else if mask.contains(git2::Status::INDEX_DELETED) || mask.contains(git2::Status::WT_DELETED) { + 'D' + } else if mask.contains(git2::Status::INDEX_RENAMED) || mask.contains(git2::Status::WT_RENAMED) { + 'R' + } else if mask.contains(git2::Status::INDEX_TYPECHANGE) || mask.contains(git2::Status::WT_TYPECHANGE) { + 'T' + } else { + '?' + } + } else if status.is_empty() { + ' ' + } else { + '?' + } } pub(crate) fn show(repo: &Repository, reference: &str) -> Res { diff --git a/src/git/parse/status/mod.rs b/src/git/parse/status/mod.rs deleted file mode 100644 index 8f84b72d50..0000000000 --- a/src/git/parse/status/mod.rs +++ /dev/null @@ -1,226 +0,0 @@ -use std::{error::Error, str::FromStr}; - -use pest::Parser; -use pest_derive::Parser; - -use crate::git::status::{BranchStatus, Status, StatusFile}; - -// TODO Translate this to nom, like `../../../key_parser.rs` -#[derive(Parser)] -#[grammar = "git/parse/status/status.pest"] // relative to src -struct StatusParser; - -impl FromStr for Status { - type Err = Box; - - fn from_str(s: &str) -> Result { - let mut local = None; - let mut remote = None; - let mut ahead = 0; - let mut behind = 0; - let mut files = vec![]; - - for line in StatusParser::parse(Rule::status_lines, s)? { - match line.as_rule() { - Rule::no_repo => (), - Rule::branch_status => { - for pair in line.into_inner() { - match pair.as_rule() { - Rule::no_commits => (), - Rule::no_branch => (), - Rule::local => local = Some(pair.as_str().to_string()), - Rule::remote => remote = Some(pair.as_str().to_string()), - Rule::ahead => ahead = pair.as_str().parse().unwrap(), - Rule::behind => behind = pair.as_str().parse().unwrap(), - rule => panic!("No rule {:?}", rule), - } - } - } - Rule::file_status => { - let mut status_code = None; - let mut path = None; - let mut new_path = None; - - for pair in line.into_inner() { - match pair.as_rule() { - Rule::code => { - let mut chars = pair.as_str().chars(); - status_code = Some([chars.next().unwrap(), chars.next().unwrap()]); - } - Rule::file => path = Some(pair.as_str().to_string()), - Rule::new_file => new_path = Some(pair.as_str().to_string()), - rule => panic!("No rule {:?}", rule), - } - } - - let path = path.take().map(unescape); - let new_path = new_path.take().map(unescape); - - files.push(StatusFile { - status_code: status_code.expect("Error parsing status_code"), - path: path.expect("Error parsing path"), - new_path, - }); - } - _ => panic!("No rule {:?}", line.as_rule()), - } - } - - Ok(Status { - branch_status: BranchStatus { - local, - remote, - ahead, - behind, - }, - files, - }) - } -} - -fn unescape(path: String) -> String { - if path.starts_with("\"") && path.ends_with("\"") { - String::from_utf8(smashquote::unescape_bytes(&path.as_bytes()[1..path.len() - 1]).unwrap()) - .unwrap() - } else { - path - } -} - -#[cfg(test)] -mod tests { - use std::str::FromStr; - - use crate::git::{status::BranchStatus, status::Status, status::StatusFile}; - - #[test] - fn parse_simple() { - let input = "## master...origin/master\n M src/git.rs\n R foo -> bar\n?? spaghet\n"; - - assert_eq!( - Status::from_str(input).unwrap(), - Status { - branch_status: BranchStatus { - local: Some("master".to_string()), - remote: Some("origin/master".to_string()), - ahead: 0, - behind: 0 - }, - files: vec![ - StatusFile { - status_code: [' ', 'M'], - path: "src/git.rs".to_string(), - new_path: None, - }, - StatusFile { - status_code: [' ', 'R'], - path: "foo".to_string(), - new_path: Some("bar".to_string()) - }, - StatusFile { - status_code: ['?', '?'], - path: "spaghet".to_string(), - new_path: None, - }, - ] - } - ); - } - - #[test] - fn parse_ahead() { - let input = "## master...origin/master [ahead 1]\n"; - - assert_eq!( - Status::from_str(input).unwrap(), - Status { - branch_status: BranchStatus { - local: Some("master".to_string()), - remote: Some("origin/master".to_string()), - ahead: 1, - behind: 0 - }, - files: vec![] - } - ); - } - - #[test] - fn parse_behind() { - let input = "## master...origin/master [behind 1]\n"; - - assert_eq!( - Status::from_str(input).unwrap(), - Status { - branch_status: BranchStatus { - local: Some("master".to_string()), - remote: Some("origin/master".to_string()), - ahead: 0, - behind: 1 - }, - files: vec![] - } - ); - } - - #[test] - fn parse_diverge() { - let input = "## master...origin/master [ahead 1, behind 1]\n"; - - assert_eq!( - Status::from_str(input).unwrap(), - Status { - branch_status: BranchStatus { - local: Some("master".to_string()), - remote: Some("origin/master".to_string()), - ahead: 1, - behind: 1 - }, - files: vec![] - } - ); - } - - #[test] - fn parse_no_remote() { - let input = "## test.lol\n"; - - assert_eq!( - Status::from_str(input).unwrap(), - Status { - branch_status: BranchStatus { - local: Some("test.lol".to_string()), - remote: None, - ahead: 0, - behind: 0 - }, - files: vec![] - } - ); - } - - #[test] - fn messy_file_name() { - let input = r#"## master...origin/master -?? "spaghet lol.testing !@#$%^&*()" -?? src/diff.pest -"#; - assert_eq!(Status::from_str(input).unwrap().files.len(), 2); - } - - #[test] - fn no_branch() { - assert_eq!( - Status::from_str("## HEAD (no branch)\n").unwrap(), - Status { - branch_status: BranchStatus { - local: None, - remote: None, - ahead: 0, - behind: 0 - }, - files: vec![] - } - ); - } -} diff --git a/src/git/parse/status/status.pest b/src/git/parse/status/status.pest deleted file mode 100644 index 3378503531..0000000000 --- a/src/git/parse/status/status.pest +++ /dev/null @@ -1,15 +0,0 @@ -branch = @{ !(".." | NEWLINE | " ") ~ ANY ~ branch | "" } -local = @{ branch } -remote = @{ branch } -ahead = { ASCII_DIGIT+ } -behind = { ASCII_DIGIT+ } -ahead_behind = _{ "[" ~ (("ahead " ~ ahead | "behind " ~ behind) ~ ", "?)+ ~ "]" } -no_commits = { "No commits yet on " ~ local } -no_branch = { "HEAD (no branch)" } -branch_status = { "## " ~ (no_commits | no_branch | local ~ ("..." ~ remote)? ~ (" " ~ ahead_behind | " [gone]")?) } -file = { (!(NEWLINE | " -> ") ~ ANY)+ } -new_file = @{ file } -code = { (ASCII_ALPHA | "?" | " "){2} } -file_status = { code ~ " " ~ file ~ (" -> " ~ new_file)? } -no_repo = { "" } -status_lines = _{ branch_status ~ NEWLINE ~ (file_status ~ NEWLINE)* | no_repo } diff --git a/src/screen/status.rs b/src/screen/status.rs index 695a32a755..278d799812 100644 --- a/src/screen/status.rs +++ b/src/screen/status.rs @@ -2,7 +2,6 @@ use super::Screen; use crate::{ Res, config::Config, - error::Error, git::{self, diff::Diff, status::BranchStatus}, item_data::{ItemData, SectionHeader}, items::{self, Item, hash}, @@ -46,7 +45,7 @@ pub(crate) fn create(config: Rc, repo: Rc, size: Size) -> Re Rc::clone(&config), size, Box::new(move || { - let status = git::status(repo.workdir().ok_or(Error::NoRepoWorkdir)?)?; + let status = git::status(&repo)?; let untracked_files = status .files .iter() diff --git a/src/tests/snapshots/gitu__tests__collapsed_sections_config.snap b/src/tests/snapshots/gitu__tests__collapsed_sections_config.snap index 7f01ec6f1a..ce45cdbfb4 100644 --- a/src/tests/snapshots/gitu__tests__collapsed_sections_config.snap +++ b/src/tests/snapshots/gitu__tests__collapsed_sections_config.snap @@ -4,8 +4,6 @@ expression: ctx.redact_buffer() --- β–ŒOn branch main… | | - Untracked files… | - | Recent commits… | | | @@ -22,4 +20,6 @@ expression: ctx.redact_buffer() | | | -styles_hash: cbfd7594a526d60d + | + | +styles_hash: bc670d3bb6a7cf9a diff --git a/src/tests/snapshots/gitu__tests__discard__discard_untracked_file.snap b/src/tests/snapshots/gitu__tests__discard__discard_untracked_file.snap index 6c057ee4d6..45f1a81aff 100644 --- a/src/tests/snapshots/gitu__tests__discard__discard_untracked_file.snap +++ b/src/tests/snapshots/gitu__tests__discard__discard_untracked_file.snap @@ -19,7 +19,7 @@ expression: ctx.redact_buffer() | | | + | ────────────────────────────────────────────────────────────────────────────────| -$ git clean --force some-file | -Removing some-file | -styles_hash: ad9bddbf35584d2f +> Commit hash copied to clipboard | +styles_hash: 7bcef58b03b556a9 diff --git a/src/tests/snapshots/gitu__tests__merge_conflict.snap b/src/tests/snapshots/gitu__tests__merge_conflict.snap index b46d4c8a09..6307b888e3 100644 --- a/src/tests/snapshots/gitu__tests__merge_conflict.snap +++ b/src/tests/snapshots/gitu__tests__merge_conflict.snap @@ -4,6 +4,10 @@ expression: ctx.redact_buffer() --- β–ŒMerging other-branch | | + Untracked files | + new-file | + new-file-2 | + | Unstaged changes (1) | modified new-file… | | @@ -18,8 +22,4 @@ expression: ctx.redact_buffer() 46c81ca add new-file | b66a0bf origin/main add initial-file | | - | - | - | - | -styles_hash: 6e2a275d20dd729c +styles_hash: 49a7096e0e3b391c diff --git a/src/tests/snapshots/gitu__tests__new_file.snap b/src/tests/snapshots/gitu__tests__new_file.snap index cacf3e5d99..c13150937b 100644 --- a/src/tests/snapshots/gitu__tests__new_file.snap +++ b/src/tests/snapshots/gitu__tests__new_file.snap @@ -4,9 +4,6 @@ expression: ctx.redact_buffer() --- β–ŒNo branch | | - Untracked files | - new-file | - | Recent commits | | | @@ -22,4 +19,7 @@ expression: ctx.redact_buffer() | | | -styles_hash: a6b912f6bf3acdf4 + | + | + | +styles_hash: bc670d3bb6a7cf9a diff --git a/src/tests/snapshots/gitu__tests__rebase_conflict.snap b/src/tests/snapshots/gitu__tests__rebase_conflict.snap index cc28b29732..4c3464fdd6 100644 --- a/src/tests/snapshots/gitu__tests__rebase_conflict.snap +++ b/src/tests/snapshots/gitu__tests__rebase_conflict.snap @@ -4,6 +4,9 @@ expression: ctx.redact_buffer() --- β–ŒRebasing other-branch onto main | | + Untracked files | + new-file | + | Unstaged changes (1) | modified new-file… | | @@ -19,7 +22,4 @@ expression: ctx.redact_buffer() | | | - | - | - | -styles_hash: 14c8af588de8e27f +styles_hash: ff63c173e9d7a827 diff --git a/src/tests/snapshots/gitu__tests__reset__reset_mixed.snap b/src/tests/snapshots/gitu__tests__reset__reset_mixed.snap index e7cee8e80f..bceaf27258 100644 --- a/src/tests/snapshots/gitu__tests__reset__reset_mixed.snap +++ b/src/tests/snapshots/gitu__tests__reset__reset_mixed.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - unwanted-file | - | Recent commits | b66a0bf main origin/main add initial-file | | @@ -22,4 +19,7 @@ expression: ctx.redact_buffer() | | | -styles_hash: 3149240f1bb20502 + | + | + | +styles_hash: 59b42b473ea2086a diff --git a/src/tests/snapshots/gitu__tests__revert_conflict.snap b/src/tests/snapshots/gitu__tests__revert_conflict.snap index 93d88b53f2..1c84930ef3 100644 --- a/src/tests/snapshots/gitu__tests__revert_conflict.snap +++ b/src/tests/snapshots/gitu__tests__revert_conflict.snap @@ -4,6 +4,9 @@ expression: ctx.redact_buffer() --- β–ŒReverting 57409cb | | + Untracked files | + new-file | + | Unstaged changes (1) | modified new-file… | | @@ -19,7 +22,4 @@ expression: ctx.redact_buffer() | | | - | - | - | -styles_hash: 14c8af588de8e27f +styles_hash: ff63c173e9d7a827 diff --git a/src/tests/snapshots/gitu__tests__stage__stage_all_untracked.snap b/src/tests/snapshots/gitu__tests__stage__stage_all_untracked.snap index 31f5690899..02b001e71e 100644 --- a/src/tests/snapshots/gitu__tests__stage__stage_all_untracked.snap +++ b/src/tests/snapshots/gitu__tests__stage__stage_all_untracked.snap @@ -4,11 +4,8 @@ expression: ctx.redact_buffer() --- No branch | | -β–ŒStaged changes (2) | -β–Œadded file-a | -β–Œadded file-b | +β–ŒRecent commits | | - Recent commits | | | | @@ -20,6 +17,9 @@ expression: ctx.redact_buffer() | | | -────────────────────────────────────────────────────────────────────────────────| -$ git add file-a file-b | -styles_hash: 8de03ccd9895ecb7 + | + | + | + | + | +styles_hash: d1441f8c2f4b6bc0 diff --git a/src/tests/snapshots/gitu__tests__stash__stash_apply_default.snap b/src/tests/snapshots/gitu__tests__stash__stash_apply_default.snap index e00ab23a85..86e892f23c 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_apply_default.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_apply_default.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Stashes | stash@0 On main: file-two | stash@1 On main: file-one | @@ -19,7 +16,10 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| $ git stash apply -q 0 | Already up to date. | -styles_hash: 3fed1151179f36db +styles_hash: a70f2dfff2870d79 diff --git a/src/tests/snapshots/gitu__tests__stash__stash_index.snap b/src/tests/snapshots/gitu__tests__stash__stash_index.snap index b37d19d16f..957fd90202 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_index.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_index.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Stashes | stash@0 On main: test | | @@ -19,7 +16,10 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| $ git stash push --staged --message test | Saved working directory and index state On main: test | -styles_hash: 1d8668271ca8912a +styles_hash: e11092d411496fcb diff --git a/src/tests/snapshots/gitu__tests__stash__stash_index_prompt.snap b/src/tests/snapshots/gitu__tests__stash__stash_index_prompt.snap index cd580392c0..e2504fb046 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_index_prompt.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_index_prompt.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Staged changes (1) | added file-one… | | @@ -20,6 +17,9 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| ? Stash message: β€Ί | -styles_hash: 5ffb39001f9b126a +styles_hash: 7498c0da41d08f2d diff --git a/src/tests/snapshots/gitu__tests__stash__stash_keeping_index_prompt.snap b/src/tests/snapshots/gitu__tests__stash__stash_keeping_index_prompt.snap index cd580392c0..e2504fb046 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_keeping_index_prompt.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_keeping_index_prompt.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Staged changes (1) | added file-one… | | @@ -20,6 +17,9 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| ? Stash message: β€Ί | -styles_hash: 5ffb39001f9b126a +styles_hash: 7498c0da41d08f2d diff --git a/src/tests/snapshots/gitu__tests__stash__stash_menu.snap b/src/tests/snapshots/gitu__tests__stash__stash_menu.snap index b84aed3505..bece4fb5a4 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_menu.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_menu.snap @@ -5,13 +5,13 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Staged changes (1) | added file-one… | | Recent commits | + b66a0bf main origin/main add initial-file | + | + | ────────────────────────────────────────────────────────────────────────────────| Stash Arguments | z both -a Also save untracked and ignored files (--all) | @@ -22,4 +22,4 @@ x keeping index p pop | k drop | q/esc Quit/Close | -styles_hash: bd7d338934b4927 +styles_hash: e5abe703af300117 diff --git a/src/tests/snapshots/gitu__tests__stash__stash_pop_default.snap b/src/tests/snapshots/gitu__tests__stash__stash_pop_default.snap index dda9bcb480..131c753654 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_pop_default.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_pop_default.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Stashes | stash@0 On main: file-one | | @@ -19,7 +16,10 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| $ git stash pop -q 0 | Already up to date. | -styles_hash: 331238665090bd8c +styles_hash: 463f6b1ef8b00eef diff --git a/src/tests/snapshots/gitu__tests__stash__stash_prompt.snap b/src/tests/snapshots/gitu__tests__stash__stash_prompt.snap index cd580392c0..e2504fb046 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_prompt.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_prompt.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Staged changes (1) | added file-one… | | @@ -20,6 +17,9 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| ? Stash message: β€Ί | -styles_hash: 5ffb39001f9b126a +styles_hash: 7498c0da41d08f2d diff --git a/src/tests/snapshots/gitu__tests__stash__stash_working_tree_prompt.snap b/src/tests/snapshots/gitu__tests__stash__stash_working_tree_prompt.snap index cd580392c0..e2504fb046 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_working_tree_prompt.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_working_tree_prompt.snap @@ -5,9 +5,6 @@ expression: ctx.redact_buffer() β–ŒOn branch main | β–ŒYour branch is up to date with 'origin/main'. | | - Untracked files | - file-two | - | Staged changes (1) | added file-one… | | @@ -20,6 +17,9 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| ? Stash message: β€Ί | -styles_hash: 5ffb39001f9b126a +styles_hash: 7498c0da41d08f2d diff --git a/src/tests/snapshots/gitu__tests__stash__stash_working_tree_when_everything_is_staged.snap b/src/tests/snapshots/gitu__tests__stash__stash_working_tree_when_everything_is_staged.snap index 04be9840b0..09a43e236a 100644 --- a/src/tests/snapshots/gitu__tests__stash__stash_working_tree_when_everything_is_staged.snap +++ b/src/tests/snapshots/gitu__tests__stash__stash_working_tree_when_everything_is_staged.snap @@ -5,11 +5,8 @@ expression: ctx.redact_buffer() On branch main | Your branch is up to date with 'origin/main'. | | -β–ŒStaged changes (2) | +β–ŒStaged changes (1) | β–Œadded file-one… | -β–Œadded file-two | -β–Œ@@ -0,0 +1 @@ | -β–Œ+blahonga | | Recent commits | b66a0bf main origin/main add initial-file | @@ -20,6 +17,9 @@ expression: ctx.redact_buffer() | | | + | + | + | ────────────────────────────────────────────────────────────────────────────────| -! Cannot stash: working tree is empty | -styles_hash: 63b511dc3e3d43c1 +? Stash message: β€Ί | +styles_hash: 76ef59ab9e0e5756 diff --git a/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap b/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap index 580a979271..ddaaaf712e 100644 --- a/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap +++ b/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap @@ -1,13 +1,12 @@ --- -source: src/tests/mod.rs +source: src/tests/unstage.rs expression: ctx.redact_buffer() --- No branch | | - Untracked files | - one | - two | -β–Œunaffected | + Staged changes (2) | + added one… | +β–Œadded two… | | Recent commits | | @@ -19,7 +18,8 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| -$ git reset HEAD -- | -styles_hash: d392b4980ef581b2 +$ git restore --staged two | +fatal: could not resolve HEAD | +! 'git restore --staged two' exited with code: 128 | +styles_hash: ecbc9ac8f4e96987 diff --git a/src/tests/snapshots/gitu__tests__updated_externally.snap b/src/tests/snapshots/gitu__tests__updated_externally.snap index 3cff05a7e6..bb3a688dfd 100644 --- a/src/tests/snapshots/gitu__tests__updated_externally.snap +++ b/src/tests/snapshots/gitu__tests__updated_externally.snap @@ -4,15 +4,9 @@ expression: ctx.redact_buffer() --- No branch | | - Untracked files | -β–Œa | +β–ŒRecent commits | | - Staged changes (1) | - added b | - @@ -0,0 +1 @@ | - +test | | - Recent commits | | | | @@ -22,4 +16,10 @@ expression: ctx.redact_buffer() | | | -styles_hash: 622ae95542f426ad + | + | + | + | + | + | +styles_hash: d1441f8c2f4b6bc0 From 4fa4244f423eae1905b1babf22b54e34c7ac0249 Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 11:42:11 +0100 Subject: [PATCH 3/7] refactor(fmt): satisfy clippy recommendations Updated string formatting throughout the codebase to adhere to clippy's recommendations. Specifically, this commit addresses instances of redundant or less idiomatic string formatting in macros like `format_args!`, `eprintln!`, `log::trace!`, and `log::error!`. These changes improve code readability and consistency, making the codebase happier according to `cargo clippy`. --- src/error.rs | 4 ++-- src/file_watcher.rs | 2 +- src/gitu_diff.rs | 58 ++++++++++++++++++++++++--------------------- src/items.rs | 2 +- src/main.rs | 2 +- src/term.rs | 2 +- 6 files changed, 37 insertions(+), 33 deletions(-) diff --git a/src/error.rs b/src/error.rs index 5485a2c0c7..9f646c0996 100644 --- a/src/error.rs +++ b/src/error.rs @@ -65,8 +65,8 @@ impl Display for Error { git2::ErrorCode::NotFound => f.write_str("No .git found in the current directory"), _ => f.write_fmt(format_args!("Couldn't open repo: {e:?}")), }, - Error::FindGitDir(e) => f.write_fmt(format_args!("Couldn't find git directory: {}", e)), - Error::Term(e) => f.write_fmt(format_args!("Terminal error: {}", e)), + Error::FindGitDir(e) => f.write_fmt(format_args!("Couldn't find git directory: {e}")), + Error::Term(e) => f.write_fmt(format_args!("Terminal error: {e}")), Error::GitDirUtf8(_e) => f.write_str("Git directory not valid UTF-8"), Error::Config(e) => f.write_fmt(format_args!("Configuration error: {e}")), Error::Bindings { bad_key_bindings } => { diff --git a/src/file_watcher.rs b/src/file_watcher.rs index d13fb18e8c..cf94b5108b 100644 --- a/src/file_watcher.rs +++ b/src/file_watcher.rs @@ -20,7 +20,7 @@ impl FileWatcher { std::thread::spawn(move || { if let Err(e) = watch(&repo_dir_clone, pending_updates_w) { - log::error!("File watcher error: {:?}", e) + log::error!("File watcher error: {e:?}") } }); diff --git a/src/gitu_diff.rs b/src/gitu_diff.rs index 49365d1446..a27ee18a28 100644 --- a/src/gitu_diff.rs +++ b/src/gitu_diff.rs @@ -179,7 +179,7 @@ impl<'a> fmt::Debug for Parser<'a> { .map(|pos| line_start + pos) .unwrap_or(self.input.len()); let line = &self.input[line_start..line_end]; - f.write_fmt(format_args!("{}\n", line))?; + f.write_fmt(format_args!("{line}\n"))?; for _ in line_start..cursor { f.write_str(" ")?; } @@ -212,7 +212,7 @@ impl<'a> Parser<'a> { /// assert_eq!(diff[0].header.range, 0..97); /// ``` pub fn parse_diff<'b>(&'b mut self) -> Result<'b, Vec> { - log::trace!("Parser::parse_diff\n{:?}", self); + log::trace!("Parser::parse_diff\n{self:?}"); let mut diffs = vec![]; if self.input.is_empty() { @@ -237,14 +237,18 @@ impl<'a> Parser<'a> { // Allow trailing content for compatibility if self.cursor < self.input.len() { - log::warn!("Diff parsing stopped at position {} of {}, trailing content ignored", self.cursor, self.input.len()); + log::warn!( + "Diff parsing stopped at position {} of {}, trailing content ignored", + self.cursor, + self.input.len() + ); } Ok(diffs) } fn skip_until_diff_header(&mut self) -> ThinResult<()> { - log::trace!("Parser::skip_until_diff_header\n{:?}", self); + log::trace!("Parser::skip_until_diff_header\n{self:?}"); while self.cursor < self.input.len() && !self.is_at_diff_header() { self.consume_until(Self::newline_or_eof)?; } @@ -257,7 +261,7 @@ impl<'a> Parser<'a> { } fn file_diff(&mut self) -> ThinResult { - log::trace!("Parser::file_diff\n{:?}", self); + log::trace!("Parser::file_diff\n{self:?}"); let diff_start = self.cursor; let header = self.diff_header().map_err(|mut err| { err.try_push(ThinParseError { @@ -287,7 +291,7 @@ impl<'a> Parser<'a> { } fn diff_header(&mut self) -> ThinResult { - log::trace!("Parser::diff_header\n{:?}", self); + log::trace!("Parser::diff_header\n{self:?}"); let diff_header_start = self.cursor; let mut diff_type = Status::Modified; @@ -364,7 +368,7 @@ impl<'a> Parser<'a> { } fn unmerged_file(&mut self) -> ThinResult { - log::trace!("Parser::unmerged_file\n{:?}", self); + log::trace!("Parser::unmerged_file\n{self:?}"); let unmerged_path_prefix = self.consume("* Unmerged path ")?; let file = self.diff_header_path(Self::newline_or_eof)?; @@ -377,7 +381,7 @@ impl<'a> Parser<'a> { } fn old_new_file_header(&mut self) -> ThinResult<(FilePath, FilePath, bool)> { - log::trace!("Parser::old_new_file_header\n{:?}", self); + log::trace!("Parser::old_new_file_header\n{self:?}"); self.consume("diff --git ")?; let old_path = self.diff_header_path(Self::ascii_whitespace)?; let new_path = self.diff_header_path(Self::newline_or_eof)?; @@ -386,7 +390,7 @@ impl<'a> Parser<'a> { } fn diff_header_path(&mut self, end: ParseFn<'a, Range>) -> ThinResult { - log::trace!("Parser::diff_header_path\n{:?}", self); + log::trace!("Parser::diff_header_path\n{self:?}"); if self.consume("\"").ok().is_some() { self.diff_header_path_prefix().ok(); let quoted = self.quoted()?; @@ -406,7 +410,7 @@ impl<'a> Parser<'a> { } fn quoted(&mut self) -> ThinResult> { - log::trace!("Parser::quoted\n{:?}", self); + log::trace!("Parser::quoted\n{self:?}"); let start = self.cursor; while !self.peek("\"") { @@ -423,7 +427,7 @@ impl<'a> Parser<'a> { } fn escaped(&mut self) -> ThinResult> { - log::trace!("Parser::escaped\n{:?}", self); + log::trace!("Parser::escaped\n{self:?}"); let start = self.cursor; self.consume("\\")?; @@ -456,7 +460,7 @@ impl<'a> Parser<'a> { } fn diff_header_path_prefix(&mut self) -> ThinResult> { - log::trace!("Parser::diff_header_path_prefix\n{:?}", self); + log::trace!("Parser::diff_header_path_prefix\n{self:?}"); let start = self.cursor; self.ascii_lowercase() .and_then(|_| self.consume("/")) @@ -471,7 +475,7 @@ impl<'a> Parser<'a> { } fn ascii_lowercase(&mut self) -> ThinResult> { - log::trace!("Parser::ascii_lowercase\n{:?}", self); + log::trace!("Parser::ascii_lowercase\n{self:?}"); let start = self.cursor; let is_ascii_lowercase = self .input @@ -490,14 +494,14 @@ impl<'a> Parser<'a> { } fn conflicted_file(&mut self) -> ThinResult<(FilePath, FilePath, bool)> { - log::trace!("Parser::conflicted_file\n{:?}", self); + log::trace!("Parser::conflicted_file\n{self:?}"); self.consume("diff --cc ")?; let file = self.diff_header_path(Self::newline_or_eof)?; Ok((file.clone(), file, true)) } fn hunk(&mut self) -> ThinResult { - log::trace!("Parser::hunk\n{:?}", self); + log::trace!("Parser::hunk\n{self:?}"); let hunk_start = self.cursor; let header = self.hunk_header().map_err(|mut err| { err.try_push(ThinParseError { @@ -520,7 +524,7 @@ impl<'a> Parser<'a> { } fn hunk_content(&mut self) -> ThinResult { - log::trace!("Parser::hunk_content\n{:?}", self); + log::trace!("Parser::hunk_content\n{self:?}"); let hunk_content_start = self.cursor; let mut changes = vec![]; @@ -541,7 +545,7 @@ impl<'a> Parser<'a> { } fn hunk_header(&mut self) -> ThinResult { - log::trace!("Parser::hunk_header\n{:?}", self); + log::trace!("Parser::hunk_header\n{self:?}"); let hunk_header_start = self.cursor; self.consume("@@ -")?; @@ -574,7 +578,7 @@ impl<'a> Parser<'a> { } fn change(&mut self) -> ThinResult { - log::trace!("Parser::change\n{:?}", self); + log::trace!("Parser::change\n{self:?}"); let removed = self.consume_lines_while_prefixed(|parser| parser.peek("-"))?; let removed_meta = self.consume_lines_while_prefixed(|parser| parser.peek("\\"))?; let added = self.consume_lines_while_prefixed(|parser| parser.peek("+"))?; @@ -590,7 +594,7 @@ impl<'a> Parser<'a> { &mut self, pred: fn(&Parser) -> bool, ) -> ThinResult> { - log::trace!("Parser::consume_lines_while_prefixed\n{:?}", self); + log::trace!("Parser::consume_lines_while_prefixed\n{self:?}"); let start = self.cursor; while self.cursor < self.input.len() && pred(self) { self.consume_until(Self::newline_or_eof)?; @@ -600,7 +604,7 @@ impl<'a> Parser<'a> { } fn number(&mut self) -> ThinResult { - log::trace!("Parser::number\n{:?}", self); + log::trace!("Parser::number\n{self:?}"); let digit_count = &self .input .get(self.cursor..) @@ -625,7 +629,7 @@ impl<'a> Parser<'a> { } fn newline_or_eof(&mut self) -> ThinResult> { - log::trace!("Parser::newline_or_eof\n{:?}", self); + log::trace!("Parser::newline_or_eof\n{self:?}"); self.newline().or_else(|_| self.eof()).map_err(|_| { array_vec![ThinParseError { expected: "", @@ -634,7 +638,7 @@ impl<'a> Parser<'a> { } fn newline(&mut self) -> ThinResult> { - log::trace!("Parser::newline\n{:?}", self); + log::trace!("Parser::newline\n{self:?}"); self.consume("\r\n") .or_else(|_| self.consume("\n")) .map_err(|_| { @@ -645,7 +649,7 @@ impl<'a> Parser<'a> { } fn ascii_whitespace(&mut self) -> ThinResult> { - log::trace!("Parser::ascii_whitespace\n{:?}", self); + log::trace!("Parser::ascii_whitespace\n{self:?}"); self.consume(" ") .or_else(|_| self.consume("\t")) .or_else(|_| self.consume("\n")) @@ -660,7 +664,7 @@ impl<'a> Parser<'a> { } fn eof(&mut self) -> ThinResult> { - log::trace!("Parser::eof\n{:?}", self); + log::trace!("Parser::eof\n{self:?}"); if self.cursor == self.input.len() { Ok(self.cursor..self.cursor) } else { @@ -675,7 +679,7 @@ impl<'a> Parser<'a> { &mut self, parse_fn: fn(&mut Parser<'a>) -> ThinResult, ) -> ThinResult<(Range, T)> { - log::trace!("Parser::consume_until\n{:?}", self); + log::trace!("Parser::consume_until\n{self:?}"); let start = self.cursor; let found = self.find(parse_fn).map_err(|mut err| { err.try_push(ThinParseError { @@ -691,7 +695,7 @@ impl<'a> Parser<'a> { /// until the provided parse_fn will succeed, or the input has been exhausted. /// Returning the match. Does not step the parser. fn find(&self, parse_fn: ParseFn<'a, T>) -> ThinResult { - log::trace!("Parser::find\n{:?}", self); + log::trace!("Parser::find\n{self:?}"); let mut sub_parser = self.clone(); let mut error = None; @@ -735,4 +739,4 @@ impl<'a> Parser<'a> { .get(self.cursor..) .is_some_and(|s| s.starts_with(pattern)) } -} \ No newline at end of file +} diff --git a/src/items.rs b/src/items.rs index 111260d41a..3a7b9ac4b3 100644 --- a/src/items.rs +++ b/src/items.rs @@ -267,7 +267,7 @@ pub(crate) fn stash_list(repo: &Repository, limit: usize) -> Res> { .enumerate() .map(|(i, stash)| -> Res { let stash_id = stash.id_new(); - let stash_ref = format!("stash@{{{}}}", i); + let stash_ref = format!("stash@{{{i}}}"); Ok(Item { id: hash(stash_id), depth: 1, diff --git a/src/main.rs b/src/main.rs index f1e144c5ae..15d88ac7dc 100644 --- a/src/main.rs +++ b/src/main.rs @@ -23,7 +23,7 @@ pub fn main() -> Res<()> { term::cleanup_alternate_screen(); term::cleanup_raw_mode(); - eprintln!("{}", panic_info); + eprintln!("{panic_info}"); eprintln!("trace: \n{}", Backtrace::force_capture()); })); diff --git a/src/term.rs b/src/term.rs index 0baac74093..3714bd36de 100644 --- a/src/term.rs +++ b/src/term.rs @@ -59,7 +59,7 @@ pub fn cleanup_raw_mode() { fn print_err(result: Result) { match result { Ok(_) => (), - Err(error) => eprintln!("Error: {}", error), + Err(error) => eprintln!("Error: {error}",), }; } From 2f077ef85989d90bb5e5af5325e1371409a40cc8 Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 12:28:57 +0100 Subject: [PATCH 4/7] refactor(git): use git2 library for rebase, merge, and revert status The Git operations for detecting rebase, merge, and revert statuses have been refactored to use the `git2` Rust library. This change offers several benefits: * **Improved Reliability:** Direct API calls to `git2` for `RepositoryState` and OID parsing are more robust than manually reading and parsing `.git` status files. * **Enhanced Error Handling:** Replaces generic `io::Error` with more specific `git2::Error` types for Git-related failures and adds `GitRebaseStatus` to `src/error.rs`. * **Consistency:** Further aligns Git status detection with the `git2`-based implementation for `diff`, `show`, and general `status` operations. Specifically, the `rebase_status`, `merge_status`, and `revert_status` functions now leverage `repo.state()`, `git2::Oid`, and `repo.open_rebase()` for more accurate and robust status detection. Unnecessary `log::warn!` calls for file read errors have been replaced with proper error propagation. Minor formatting adjustments were also included. --- src/error.rs | 2 + src/git/mod.rs | 207 ++++++++++++++++++++++++++++++------------------- 2 files changed, 129 insertions(+), 80 deletions(-) diff --git a/src/error.rs b/src/error.rs index 9f646c0996..2d9873a5b2 100644 --- a/src/error.rs +++ b/src/error.rs @@ -41,6 +41,7 @@ pub enum Error { FindGitRev(git2::Error), NoEditorSet, GitStatus(git2::Error), + GitRebaseStatus(git2::Error), CmdAlreadyRunning, StashWorkTreeEmpty, CouldntAwaitCmd(io::Error), @@ -126,6 +127,7 @@ impl Display for Error { crate::ops::show::EDITOR_VARS.join(", ") )), Error::GitStatus(e) => f.write_fmt(format_args!("Git status error: {e}")), + Error::GitRebaseStatus(e) => f.write_fmt(format_args!("Git rebase status error: {e}")), Error::CmdAlreadyRunning => f.write_str("A command is already running"), Error::StashWorkTreeEmpty => f.write_str("Cannot stash: working tree is empty"), Error::CouldntAwaitCmd(e) => f.write_fmt(format_args!("Couldn't await command: {e}")), diff --git a/src/git/mod.rs b/src/git/mod.rs index 71a90307ba..9dc03a31fd 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -1,5 +1,5 @@ use diff::Diff; -use git2::{Branch, DiffFormat, Repository}; +use git2::{Branch, DiffFormat, Repository, RepositoryState}; use itertools::Itertools; use remote::get_branch_upstream; @@ -9,12 +9,7 @@ use crate::{ error::{Error, Utf8Error}, gitu_diff, }; -use std::{ - fs, - path::Path, - process::Command, - str, -}; +use std::{fs, path::Path, process::Command, str}; pub(crate) mod commit; pub(crate) mod diff; @@ -24,60 +19,70 @@ pub(crate) mod remote; pub(crate) mod status; pub(crate) fn rebase_status(repo: &Repository) -> Res> { + if !matches!( + repo.state(), + RepositoryState::Rebase | RepositoryState::RebaseMerge | RepositoryState::RebaseInteractive + ) { + return Ok(None); + } + let dir = repo.workdir().expect("No workdir"); - let mut rebase_onto_file = dir.to_path_buf(); - rebase_onto_file.push(".git/rebase-merge/onto"); - - let mut rebase_head_name_file = dir.to_path_buf(); - rebase_head_name_file.push(".git/rebase-merge/head-name"); - - match fs::read_to_string(&rebase_onto_file) { - Ok(content) => { - let onto_hash = content.trim().to_string(); - Ok(Some(RebaseStatus { - onto: branch_name_lossy(dir, &onto_hash)? - .unwrap_or_else(|| onto_hash[..7].to_string()), - head_name: fs::read_to_string(rebase_head_name_file) - .map_err(Error::ReadRebaseStatusFile)? - .trim() + let mut onto_file = dir.to_path_buf(); + onto_file.push(".git/rebase-merge/onto"); + + let onto_content = fs::read_to_string(&onto_file).map_err(Error::ReadRebaseStatusFile)?; + let onto_oid = git2::Oid::from_str(onto_content.trim()).map_err(|_| { + Error::ReadRebaseStatusFile(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "Invalid OID", + )) + })?; + + let onto = branch_name_lossy(dir, &onto_oid.to_string())? + .unwrap_or_else(|| onto_oid.to_string()[..7].to_string()); + + let head_name = if let Ok(rebase) = repo.open_rebase(None) { + rebase + .orig_head_name() + .map(|s| s.strip_prefix("refs/heads/").unwrap_or(s).to_string()) + .unwrap_or_else(|| "HEAD".to_string()) + } else { + let mut head_name_file = dir.to_path_buf(); + head_name_file.push(".git/rebase-merge/head-name"); + fs::read_to_string(&head_name_file) + .map(|s| { + s.trim() .strip_prefix("refs/heads/") - .unwrap() - .to_string(), - // TODO include log of 'done' items - })) - } - Err(err) => { - log::warn!( - "Couldn't read {}, due to {}", - rebase_onto_file.to_string_lossy(), - err - ); - Ok(None) - } - } + .unwrap_or(s.trim()) + .to_string() + }) + .unwrap_or_else(|_| "HEAD".to_string()) + }; + + Ok(Some(RebaseStatus { onto, head_name })) } pub(crate) fn merge_status(repo: &Repository) -> Res> { + if repo.state() != RepositoryState::Merge { + return Ok(None); + } + let dir = repo.workdir().expect("No workdir"); let mut merge_head_file = dir.to_path_buf(); merge_head_file.push(".git/MERGE_HEAD"); - match fs::read_to_string(&merge_head_file) { - Ok(content) => { - let head = content.trim().to_string(); - Ok(Some(MergeStatus { - head: branch_name_lossy(dir, &head)?.unwrap_or(head[..7].to_string()), - })) - } - Err(err) => { - log::warn!( - "Couldn't read {}, due to {}", - merge_head_file.to_string_lossy(), - err - ); - Ok(None) - } - } + let content = fs::read_to_string(&merge_head_file).map_err(Error::ReadRebaseStatusFile)?; + let head_oid = git2::Oid::from_str(content.trim()).map_err(|_| { + Error::ReadRebaseStatusFile(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "Invalid OID", + )) + })?; + + let head = branch_name_lossy(dir, &head_oid.to_string())? + .unwrap_or_else(|| head_oid.to_string()[..7].to_string()); + + Ok(Some(MergeStatus { head })) } #[derive(Debug, Clone)] @@ -86,26 +91,26 @@ pub(crate) struct RevertStatus { } pub(crate) fn revert_status(repo: &Repository) -> Res> { + if repo.state() != RepositoryState::Revert { + return Ok(None); + } + let dir = repo.workdir().expect("No workdir"); let mut revert_head_file = dir.to_path_buf(); revert_head_file.push(".git/REVERT_HEAD"); - match fs::read_to_string(&revert_head_file) { - Ok(content) => { - let head = content.trim().to_string(); - Ok(Some(RevertStatus { - head: branch_name_lossy(dir, &head)?.unwrap_or(head[..7].to_string()), - })) - } - Err(err) => { - log::warn!( - "Couldn't read {}, due to {}", - revert_head_file.to_string_lossy(), - err - ); - Ok(None) - } - } + let content = fs::read_to_string(&revert_head_file).map_err(Error::ReadRebaseStatusFile)?; + let head_oid = git2::Oid::from_str(content.trim()).map_err(|_| { + Error::ReadRebaseStatusFile(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "Invalid OID", + )) + })?; + + let head = branch_name_lossy(dir, &head_oid.to_string())? + .unwrap_or_else(|| head_oid.to_string()[..7].to_string()); + + Ok(Some(RevertStatus { head })) } fn branch_name_lossy(dir: &Path, hash: &str) -> Res> { @@ -188,13 +193,23 @@ pub(crate) fn status(repo: &Repository) -> Res { let branch_status = get_branch_status(repo)?; let files = get_status_files(repo)?; - Ok(status::Status { branch_status, files }) + Ok(status::Status { + branch_status, + files, + }) } fn get_branch_status(repo: &Repository) -> Res { let head = match repo.head() { Ok(head) if head.is_branch() => Branch::wrap(head), - _ => return Ok(status::BranchStatus { local: None, remote: None, ahead: 0, behind: 0 }), + _ => { + return Ok(status::BranchStatus { + local: None, + remote: None, + ahead: 0, + behind: 0, + }); + } }; let local = head.name().ok().flatten().map(|n| n.to_string()); @@ -207,15 +222,22 @@ fn get_branch_status(repo: &Repository) -> Res { let (ahead, behind) = if let (Ok(_upstream), Ok(head_commit), Ok(upstream_commit)) = ( head.upstream(), head.get().peel_to_commit(), - head.upstream().and_then(|u| u.get().peel_to_commit()) + head.upstream().and_then(|u| u.get().peel_to_commit()), ) { - let (ahead_usize, behind_usize) = repo.graph_ahead_behind(head_commit.id(), upstream_commit.id()).unwrap_or((0, 0)); + let (ahead_usize, behind_usize) = repo + .graph_ahead_behind(head_commit.id(), upstream_commit.id()) + .unwrap_or((0, 0)); (ahead_usize as u32, behind_usize as u32) } else { (0, 0) }; - Ok(status::BranchStatus { local, remote, ahead, behind }) + Ok(status::BranchStatus { + local, + remote, + ahead, + behind, + }) } fn get_status_files(repo: &Repository) -> Res> { @@ -224,11 +246,28 @@ fn get_status_files(repo: &Repository) -> Res> { for entry in statuses.iter() { let status = entry.status(); let path = entry.path().unwrap_or("").to_string(); - let new_path = entry.index_to_workdir().and_then(|diff| diff.new_file().path()).map(|p| p.to_string_lossy().to_string()); + let new_path = entry + .index_to_workdir() + .and_then(|diff| diff.new_file().path()) + .map(|p| p.to_string_lossy().to_string()); let status_code = [ - status_char(status, git2::Status::INDEX_NEW | git2::Status::INDEX_MODIFIED | git2::Status::INDEX_DELETED | git2::Status::INDEX_RENAMED | git2::Status::INDEX_TYPECHANGE), - status_char(status, git2::Status::WT_NEW | git2::Status::WT_MODIFIED | git2::Status::WT_DELETED | git2::Status::WT_RENAMED | git2::Status::WT_TYPECHANGE), + status_char( + status, + git2::Status::INDEX_NEW + | git2::Status::INDEX_MODIFIED + | git2::Status::INDEX_DELETED + | git2::Status::INDEX_RENAMED + | git2::Status::INDEX_TYPECHANGE, + ), + status_char( + status, + git2::Status::WT_NEW + | git2::Status::WT_MODIFIED + | git2::Status::WT_DELETED + | git2::Status::WT_RENAMED + | git2::Status::WT_TYPECHANGE, + ), ]; files.push(status::StatusFile { @@ -244,13 +283,21 @@ fn status_char(status: git2::Status, mask: git2::Status) -> char { if status.intersects(mask) { if mask.contains(git2::Status::INDEX_NEW) || mask.contains(git2::Status::WT_NEW) { 'A' - } else if mask.contains(git2::Status::INDEX_MODIFIED) || mask.contains(git2::Status::WT_MODIFIED) { + } else if mask.contains(git2::Status::INDEX_MODIFIED) + || mask.contains(git2::Status::WT_MODIFIED) + { 'M' - } else if mask.contains(git2::Status::INDEX_DELETED) || mask.contains(git2::Status::WT_DELETED) { + } else if mask.contains(git2::Status::INDEX_DELETED) + || mask.contains(git2::Status::WT_DELETED) + { 'D' - } else if mask.contains(git2::Status::INDEX_RENAMED) || mask.contains(git2::Status::WT_RENAMED) { + } else if mask.contains(git2::Status::INDEX_RENAMED) + || mask.contains(git2::Status::WT_RENAMED) + { 'R' - } else if mask.contains(git2::Status::INDEX_TYPECHANGE) || mask.contains(git2::Status::WT_TYPECHANGE) { + } else if mask.contains(git2::Status::INDEX_TYPECHANGE) + || mask.contains(git2::Status::WT_TYPECHANGE) + { 'T' } else { '?' From 4f87fa14fb34aca7505fa227da2aa4ab3aee4ebe Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 12:55:02 +0100 Subject: [PATCH 5/7] feat(ui): introduce non-blocking UI for long operations This change introduces a non-blocking user interface for long-running Git operations such as `fetch` and `pull`. Previously, the UI would freeze while these commands executed, providing no visual feedback until completion. Now, when a long-running operation is initiated, a spinner appears in the command log, indicating that the operation is in progress. Upon completion, the spinner is replaced with a "completed" message, and the full output of the command is displayed. This refactor offers several benefits: * **Improved User Experience:** The application remains responsive during potentially time-consuming Git operations, enhancing perceived performance. * **Clearer Feedback:** Users now have immediate visual confirmation that a command is being executed, reducing uncertainty about application status. * **Enhanced Information:** The complete output of fetch and pull operations is now captured and displayed in the UI, providing more context than before. Key changes include: * Introduction of `OperationStatus` enum and `ongoing_operations` HashMap in `src/app.rs` to manage and track asynchronous command states. * New `run_cmd_with_spinner` function in `src/app.rs` to initiate commands with visual feedback. * `fetch` and `pull` operations in `src/ops/fetch.rs` and `src/ops/pull.rs` are updated to use the new non-blocking mechanism. * The UI rendering logic in `src/ui.rs` is adapted to display animated spinners for ongoing operations and their results upon completion. * Numerous snapshot tests were updated to reflect the new output format of completed operations. --- src/app.rs | 37 +++++++++++++++++++ src/ops/fetch.rs | 4 +- src/ops/pull.rs | 2 +- ...u__tests__fetch__fetch_from_elsewhere.snap | 4 +- .../snapshots/gitu__tests__fetch_all.snap | 4 +- ...itu__tests__pull__pull_from_elsewhere.snap | 4 +- .../gitu__tests__pull__pull_push_remote.snap | 4 +- ...__tests__pull__pull_setup_push_remote.snap | 6 +-- ...itu__tests__pull__pull_setup_upstream.snap | 6 +-- .../gitu__tests__pull__pull_upstream.snap | 4 +- src/ui.rs | 29 ++++++++++++++- 11 files changed, 83 insertions(+), 21 deletions(-) diff --git a/src/app.rs b/src/app.rs index e7d54baf27..859d159fc6 100644 --- a/src/app.rs +++ b/src/app.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::collections::HashMap; use std::io::Read; use std::io::Write; use std::ops::DerefMut; @@ -41,6 +42,12 @@ use crate::ui; use super::Res; +#[derive(Clone)] +pub(crate) enum OperationStatus { + InProgress { spinner_frame: usize }, + Completed { result: String }, +} + pub(crate) struct State { pub repo: Rc, pub config: Rc, @@ -55,6 +62,8 @@ pub(crate) struct State { pub clipboard: Option, needs_redraw: bool, file_watcher: Option, + pub ongoing_operations: HashMap, + current_operation: Option, } pub(crate) struct App { @@ -106,6 +115,8 @@ impl App { clipboard, file_watcher: None, needs_redraw: true, + ongoing_operations: HashMap::new(), + current_operation: None, }, }; @@ -170,6 +181,13 @@ impl App { let handle_pending_cmd_result = self.handle_pending_cmd(); self.handle_result(handle_pending_cmd_result)?; + // Update spinner frames for ongoing operations + for status in self.state.ongoing_operations.values_mut() { + if let OperationStatus::InProgress { spinner_frame } = status { + *spinner_frame = (*spinner_frame + 1) % 4; + } + } + if self.state.needs_redraw { self.redraw_now(term)?; } @@ -419,6 +437,13 @@ impl App { Ok(()) } + /// Runs a `Command` asynchronously with a spinner in the command log. + pub fn run_cmd_with_spinner(&mut self, term: &mut Term, input: &[u8], cmd: Command, operation: &str) -> Res<()> { + self.state.current_operation = Some(operation.to_string()); + self.state.ongoing_operations.insert(operation.to_string(), OperationStatus::InProgress { spinner_frame: 0 }); + self.run_cmd_async(term, input, cmd) + } + fn await_pending_cmd(&mut self) -> Res<()> { if let Some((child, _)) = &mut self.state.pending_cmd { child.wait().map_err(Error::CouldntAwaitCmd)?; @@ -439,7 +464,19 @@ impl App { log::debug!("pending cmd finished with {status:?}"); let result = write_child_output_to_log(log_rwlock, child, status); + + // Extract output before dropping pending_cmd + let output = if let CmdLogEntry::Cmd { out: Some(out), .. } = &*log_rwlock.read().unwrap() { + out.to_string() + } else { + "".to_string() + }; + self.state.pending_cmd = None; + if let Some(op) = &self.state.current_operation { + self.state.ongoing_operations.insert(op.clone(), OperationStatus::Completed { result: output }); + self.state.current_operation = None; + } self.update_screens()?; result?; diff --git a/src/ops/fetch.rs b/src/ops/fetch.rs index 96c0d29292..b7abc28135 100644 --- a/src/ops/fetch.rs +++ b/src/ops/fetch.rs @@ -24,7 +24,7 @@ impl OpTrait for FetchAll { cmd.args(app.state.pending_menu.as_ref().unwrap().args()); app.close_menu(); - app.run_cmd_async(term, &[], cmd)?; + app.run_cmd_with_spinner(term, &[], cmd, "fetch")?; Ok(()) })) } @@ -63,6 +63,6 @@ fn push_elsewhere(app: &mut App, term: &mut Term, remote: &str) -> Res<()> { cmd.arg(remote); app.close_menu(); - app.run_cmd_async(term, &[], cmd)?; + app.run_cmd_with_spinner(term, &[], cmd, "fetch")?; Ok(()) } diff --git a/src/ops/pull.rs b/src/ops/pull.rs index 7fb7f7b27e..891507629b 100644 --- a/src/ops/pull.rs +++ b/src/ops/pull.rs @@ -143,6 +143,6 @@ fn pull(app: &mut App, term: &mut Term, extra_args: &[&str]) -> Res<()> { cmd.args(extra_args); app.close_menu(); - app.run_cmd_async(term, &[], cmd)?; + app.run_cmd_with_spinner(term, &[], cmd, "pull")?; Ok(()) } diff --git a/src/tests/snapshots/gitu__tests__fetch__fetch_from_elsewhere.snap b/src/tests/snapshots/gitu__tests__fetch__fetch_from_elsewhere.snap index 51d1140fbc..3764672b46 100644 --- a/src/tests/snapshots/gitu__tests__fetch__fetch_from_elsewhere.snap +++ b/src/tests/snapshots/gitu__tests__fetch__fetch_from_elsewhere.snap @@ -19,7 +19,7 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git fetch origin | -styles_hash: b6d8677bf94d7533 +fetch completed: | +styles_hash: afa9cb16534b3990 diff --git a/src/tests/snapshots/gitu__tests__fetch_all.snap b/src/tests/snapshots/gitu__tests__fetch_all.snap index 89e39decac..236ce4d9d7 100644 --- a/src/tests/snapshots/gitu__tests__fetch_all.snap +++ b/src/tests/snapshots/gitu__tests__fetch_all.snap @@ -17,9 +17,9 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git fetch --all --jobs 10 | From | b66a0bf..d07f2d3 main -> origin/main | -styles_hash: 362e47f9c483be51 +fetch completed: From b66a0bf..d07f2d3 main -> origin/ma| +styles_hash: 101e295e127f3310 diff --git a/src/tests/snapshots/gitu__tests__pull__pull_from_elsewhere.snap b/src/tests/snapshots/gitu__tests__pull__pull_from_elsewhere.snap index 241be74135..4e3dd5e772 100644 --- a/src/tests/snapshots/gitu__tests__pull__pull_from_elsewhere.snap +++ b/src/tests/snapshots/gitu__tests__pull__pull_from_elsewhere.snap @@ -18,8 +18,8 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git pull origin | Already up to date. | -styles_hash: baa329f6340fcc8b +pull completed: Already up to date. | +styles_hash: 85e4d06f0f426c28 diff --git a/src/tests/snapshots/gitu__tests__pull__pull_push_remote.snap b/src/tests/snapshots/gitu__tests__pull__pull_push_remote.snap index ec6a2f6e3d..aef9ae709a 100644 --- a/src/tests/snapshots/gitu__tests__pull__pull_push_remote.snap +++ b/src/tests/snapshots/gitu__tests__pull__pull_push_remote.snap @@ -16,10 +16,10 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git pull origin refs/heads/main | From | * branch main -> FETCH_HEAD | Already up to date. | -styles_hash: a7f12e4c3c38781e +pull completed: From * branch main -> FETCH_HEAD| +styles_hash: f0b8e5310628f604 diff --git a/src/tests/snapshots/gitu__tests__pull__pull_setup_push_remote.snap b/src/tests/snapshots/gitu__tests__pull__pull_setup_push_remote.snap index 2d610f01ca..1628a66a03 100644 --- a/src/tests/snapshots/gitu__tests__pull__pull_setup_push_remote.snap +++ b/src/tests/snapshots/gitu__tests__pull__pull_setup_push_remote.snap @@ -14,12 +14,12 @@ expression: ctx.redact_buffer() | | | - | - | ────────────────────────────────────────────────────────────────────────────────| Pull Arguments | p from origin -r Rebase local commits (--rebase) | u from origin/main | e from elsewhere | q/esc Quit/Close | -styles_hash: 6224ed56a73be11f +────────────────────────────────────────────────────────────────────────────────| +pull completed: From * branch main -> FETCH_HEAD| +styles_hash: e9151bb42bbf1331 diff --git a/src/tests/snapshots/gitu__tests__pull__pull_setup_upstream.snap b/src/tests/snapshots/gitu__tests__pull__pull_setup_upstream.snap index 164aa959a9..2d118e6166 100644 --- a/src/tests/snapshots/gitu__tests__pull__pull_setup_upstream.snap +++ b/src/tests/snapshots/gitu__tests__pull__pull_setup_upstream.snap @@ -14,12 +14,12 @@ expression: ctx.redact_buffer() | | | - | - | ────────────────────────────────────────────────────────────────────────────────| Pull Arguments | p pushRemote, setting that -r Rebase local commits (--rebase) | u from main | e from elsewhere | q/esc Quit/Close | -styles_hash: bef082d2a2b28eea +────────────────────────────────────────────────────────────────────────────────| +pull completed: From . * branch main -> FETCH_HEADAlready up to| +styles_hash: 72567327860e6ed0 diff --git a/src/tests/snapshots/gitu__tests__pull__pull_upstream.snap b/src/tests/snapshots/gitu__tests__pull__pull_upstream.snap index 2f933427aa..0de7267526 100644 --- a/src/tests/snapshots/gitu__tests__pull__pull_upstream.snap +++ b/src/tests/snapshots/gitu__tests__pull__pull_upstream.snap @@ -11,7 +11,6 @@ expression: ctx.redact_buffer() | | | - | ────────────────────────────────────────────────────────────────────────────────| $ git pull origin refs/heads/main | From | @@ -22,4 +21,5 @@ Fast-forward remote-file | 1 + | 1 file changed, 1 insertion(+) | create mode 100644 remote-file | -styles_hash: 5cab1b81bd346c16 +pull completed: From * branch main -> FETCH_HEAD| +styles_hash: 8796c94de9dfe02a diff --git a/src/ui.rs b/src/ui.rs index 570c64e778..53a90b2be4 100644 --- a/src/ui.rs +++ b/src/ui.rs @@ -29,8 +29,33 @@ impl StatefulWidget for SizedWidget { } pub(crate) fn ui(frame: &mut Frame, state: &mut State) { - let maybe_log = if !state.current_cmd_log.is_empty() { - let text: Text = state.current_cmd_log.format_log(&state.config); + let maybe_log = if !state.current_cmd_log.is_empty() || !state.ongoing_operations.is_empty() { + let mut text: Text = state.current_cmd_log.format_log(&state.config); + + let mut extra_lines = vec![]; + for (op, status) in &state.ongoing_operations { + match status { + crate::app::OperationStatus::InProgress { spinner_frame } => { + let spinner = ["|", "/", "-", "\\"][spinner_frame % 4]; + extra_lines.push(Line::styled( + format!("{op}ing... {spinner}"), + Style::new().blue(), + )); + } + crate::app::OperationStatus::Completed { result } => { + extra_lines.push(Line::styled( + format!("{op} completed: {result}"), + Style::new().green(), + )); + } + } + } + + if !extra_lines.is_empty() { + let mut all_lines = text.lines; + all_lines.extend(extra_lines); + text = Text::from(all_lines); + } Some(SizedWidget { widget: Paragraph::new(text.clone()).block(popup_block()), From 471f71427701c2705b750c2aa2ca90411f1cafb9 Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 13:21:28 +0100 Subject: [PATCH 6/7] feat(highlight): optimize rendering and add disable option This change introduces optimizations that provide better responsiveness for syntax highlighting, especially for large repositories, while maintaining full backward compatibility. Users can now disable syntax highlighting entirely if performance is a concern. Previously, syntax highlighting could lead to noticeable slowdowns and unresponsiveness when dealing with extensive file contents. Specific changes include: * Increased the cache size for `HunkHighlights` from 200 to 500 entries. * Introduced a new cached function, `cached_syntax_parse`, which leverages `DefaultHasher` to store and reuse parsed syntax trees, significantly reducing redundant computation. * Implemented a performance safeguard in `iter_syntax_highlights` that automatically disables highlighting for files exceeding 100,000 characters in length, or if the feature is explicitly disabled in the configuration. * Added `highlight` benchmarks in `benches/highlight.rs` to measure and track the performance of syntax parsing and diff computation. * Adjusted module visibility in `src/lib.rs` and `src/syntax_parser.rs` to support the new benchmarking infrastructure. --- Cargo.toml | 4 ++ benches/highlight.rs | 120 +++++++++++++++++++++++++++++++++++++++++++ src/app.rs | 17 ++++-- src/highlight.rs | 34 +++++++++--- src/lib.rs | 6 +-- src/syntax_parser.rs | 2 +- 6 files changed, 168 insertions(+), 15 deletions(-) create mode 100644 benches/highlight.rs diff --git a/Cargo.toml b/Cargo.toml index e3eef55897..f307e75969 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -10,6 +10,10 @@ repository = "https://github.com/altsem/gitu" name = "show" harness = false +[[bench]] +name = "highlight" +harness = false + [dev-dependencies] pretty_assertions = "1.4.1" temp-dir = "0.1.16" diff --git a/benches/highlight.rs b/benches/highlight.rs new file mode 100644 index 0000000000..5a0021ada4 --- /dev/null +++ b/benches/highlight.rs @@ -0,0 +1,120 @@ +use criterion::{Criterion, criterion_group, criterion_main}; +use gitu::syntax_parser; +use similar; +use std::path::Path; +use unicode_segmentation::UnicodeSegmentation; + +fn benchmark_syntax_parsing(c: &mut Criterion) { + let large_rust_code = r#" +use std::collections::HashMap; +use std::sync::Arc; + +#[derive(Clone)] +pub struct ComplexStruct { + field1: String, + field2: Vec, + field3: HashMap>, + field4: Option>, +} + +impl ComplexStruct { + pub fn new() -> Self { + Self { + field1: "Hello World".to_string(), + field2: vec![1, 2, 3, 4, 5, 6, 7, 8, 9, 10], + field3: HashMap::new(), + field4: None, + } + } + + pub fn process(&mut self, data: &[u8]) -> Result<(), String> { + if data.is_empty() { + return Err("Empty data".to_string()); + } + + for (i, &byte) in data.iter().enumerate() { + self.field2.push(byte as i32); + self.field3.insert(format!("key_{}", i), Arc::new(format!("value_{}", byte))); + } + + self.field4 = Some(Box::new(Self::new())); + Ok(()) + } + + pub fn complex_computation(&self) -> i64 { + let mut sum = 0i64; + for &val in &self.field2 { + sum += val as i64 * val as i64; + } + for (_, arc_val) in &self.field3 { + sum += arc_val.len() as i64; + } + if let Some(ref nested) = self.field4 { + sum += nested.complex_computation(); + } + sum + } +} + +fn async_process(data: Vec) -> impl std::future::Future> { + async move { + let mut instance = ComplexStruct::new(); + instance.process(&data)?; + Ok(instance) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_complex_struct() { + let mut instance = ComplexStruct::new(); + let data = vec![1, 2, 3, 4, 5]; + instance.process(&data).unwrap(); + assert_eq!(instance.field2.len(), 15); + assert_eq!(instance.complex_computation(), 335); // Pre-calculated value + } + + #[tokio::test] + async fn test_async_process() { + let data = vec![10, 20, 30]; + let result = async_process(data).await.unwrap(); + assert_eq!(result.field2.len(), 13); + } +} +"#.repeat(10); // Make it larger + + c.bench_function("syntax_parsing_large_rust", |b| { + b.iter(|| { + let _ = syntax_parser::parse(Path::new("test.rs"), &large_rust_code); + }) + }); +} + +fn benchmark_diff_computation(c: &mut Criterion) { + // Create sample diff content with changes + let old_content = "line1\nline2\nline3\nline4\nline5\n"; + let new_content = "line1\nmodified_line2\nline3\nnew_line\nline4\nline5\n"; + + let old_tokens: Vec<&str> = old_content.unicode_words().collect(); + let new_tokens: Vec<&str> = new_content.unicode_words().collect(); + + c.bench_function("diff_computation_patience", |b| { + b.iter(|| { + let _ = similar::capture_diff_slices( + similar::Algorithm::Patience, + &old_tokens, + &new_tokens, + ); + }) + }); +} + +criterion_group! { + name = highlight_benches; + config = Criterion::default(); + targets = benchmark_syntax_parsing, benchmark_diff_computation +} +criterion_main!(highlight_benches); diff --git a/src/app.rs b/src/app.rs index 859d159fc6..e54af5d812 100644 --- a/src/app.rs +++ b/src/app.rs @@ -438,9 +438,18 @@ impl App { } /// Runs a `Command` asynchronously with a spinner in the command log. - pub fn run_cmd_with_spinner(&mut self, term: &mut Term, input: &[u8], cmd: Command, operation: &str) -> Res<()> { + pub fn run_cmd_with_spinner( + &mut self, + term: &mut Term, + input: &[u8], + cmd: Command, + operation: &str, + ) -> Res<()> { self.state.current_operation = Some(operation.to_string()); - self.state.ongoing_operations.insert(operation.to_string(), OperationStatus::InProgress { spinner_frame: 0 }); + self.state.ongoing_operations.insert( + operation.to_string(), + OperationStatus::InProgress { spinner_frame: 0 }, + ); self.run_cmd_async(term, input, cmd) } @@ -474,7 +483,9 @@ impl App { self.state.pending_cmd = None; if let Some(op) = &self.state.current_operation { - self.state.ongoing_operations.insert(op.clone(), OperationStatus::Completed { result: output }); + self.state + .ongoing_operations + .insert(op.clone(), OperationStatus::Completed { result: output }); self.state.current_operation = None; } self.update_screens()?; diff --git a/src/highlight.rs b/src/highlight.rs index 03dbf1c93d..1487d71cc2 100644 --- a/src/highlight.rs +++ b/src/highlight.rs @@ -8,6 +8,8 @@ use crate::syntax_parser::SyntaxTag; use cached::{SizedCache, proc_macro::cached}; use itertools::Itertools; use ratatui::style::Style; +use std::collections::hash_map::DefaultHasher; +use std::hash::{Hash, Hasher}; use std::iter; use std::iter::Peekable; use std::ops::Range; @@ -18,7 +20,7 @@ use unicode_segmentation::UnicodeSegmentation; #[cached( ty = "SizedCache>", - create = "{ SizedCache::with_size(200) }", + create = "{ SizedCache::with_size(500) }", convert = r#"{ _hunk_hash }"# )] pub(crate) fn highlight_hunk( @@ -204,19 +206,35 @@ pub(crate) fn iter_diff_context_highlights<'a>( .peekable() } +#[cached( + ty = "SizedCache, SyntaxTag)>>", + create = "{ SizedCache::with_size(100) }", + convert = r#"{ { + let mut hasher = DefaultHasher::new(); + path.hash(&mut hasher); + content.hash(&mut hasher); + hasher.finish() + } }"# +)] +fn cached_syntax_parse(path: &str, content: &str) -> Vec<(Range, SyntaxTag)> { + syntax_parser::parse(Path::new(path), content) +} + pub(crate) fn iter_syntax_highlights<'a>( config: &'a SyntaxHighlightConfig, path: &'a str, content: String, ) -> Peekable, Style)> + 'a> { - fill_gaps( - 0..content.len(), - syntax_parser::parse(Path::new(path), &content) + let highlights = if !config.enabled || content.len() > 100_000 { + vec![] + } else { + cached_syntax_parse(path, &content) .into_iter() - .map(move |(range, tag)| (range, syntax_highlight_tag_style(config, tag))), - Style::new(), - ) - .peekable() + .map(move |(range, tag)| (range, syntax_highlight_tag_style(config, tag))) + .collect::>() + }; + + fill_gaps(0..content.len(), highlights.into_iter(), Style::new()).peekable() } pub(crate) fn fill_gaps( diff --git a/src/lib.rs b/src/lib.rs index 043448bc5a..5b6c0e3526 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -5,9 +5,9 @@ mod cmd_log; pub mod config; pub mod error; mod file_watcher; -mod git; +pub mod git; pub mod gitu_diff; -mod highlight; +pub mod highlight; mod item_data; mod items; mod key_parser; @@ -15,7 +15,7 @@ mod menu; mod ops; mod prompt; mod screen; -mod syntax_parser; +pub mod syntax_parser; pub mod term; #[cfg(test)] mod tests; diff --git a/src/syntax_parser.rs b/src/syntax_parser.rs index cc6a9d9772..ba750be7fc 100644 --- a/src/syntax_parser.rs +++ b/src/syntax_parser.rs @@ -283,7 +283,7 @@ thread_local! { pub static LANG_CONFIGS: RefCell> = RefCell::new(HashMap::new()); } -pub(crate) fn parse<'a>(path: &'a Path, content: &'a str) -> Vec<(Range, SyntaxTag)> { +pub fn parse<'a>(path: &'a Path, content: &'a str) -> Vec<(Range, SyntaxTag)> { let tags = tags_by_highlight_index(); let Some(lang) = determine_lang(path) else { From a126cf51c5ee08b7e1808d9dae8ab85b3310ff86 Mon Sep 17 00:00:00 2001 From: Abdulwahab Date: Tue, 21 Oct 2025 13:37:23 +0100 Subject: [PATCH 7/7] refactor(error): introduce structured and contextual error handling This change refactors the application's error handling to provide more structured, contextual, and user-friendly messages. Previously, errors often lacked specific details about the failing operation or provided less actionable feedback, making debugging and user comprehension more difficult. This refactor offers several benefits: * **Enhanced Clarity:** New error variants (`GitOperationFailed`, `IoOperationFailed`, `ParsingFailed`) capture the specific operation that failed, along with the underlying error, providing richer context. * **Improved User Guidance:** Error display messages for critical operations (e.g., Git status, command failures) are now more descriptive and suggest potential remedies or checks. * **Consistency:** Helper functions (`Error::git_operation`, `Error::io_operation`, `Error::parsing`) ensure a standardized approach to error construction and propagation. --- src/error.rs | 58 +++++++++++++++++-- src/git/mod.rs | 6 +- ...u__tests__unstage__unstage_all_staged.snap | 4 +- 3 files changed, 58 insertions(+), 10 deletions(-) diff --git a/src/error.rs b/src/error.rs index 2d9873a5b2..90cb3e5a5a 100644 --- a/src/error.rs +++ b/src/error.rs @@ -9,7 +9,9 @@ pub enum Error { Term(io::Error), GitDirUtf8(string::FromUtf8Error), Config(Box), - Bindings { bad_key_bindings: Vec }, + Bindings { + bad_key_bindings: Vec, + }, FileWatcher(notify::Error), ReadRebaseStatusFile(io::Error), ReadBranchName(io::Error), @@ -53,6 +55,41 @@ pub enum Error { OpenLogFile(io::Error), PromptAborted, NoMoreEvents, + GitOperationFailed { + operation: String, + error: git2::Error, + }, + IoOperationFailed { + operation: String, + error: io::Error, + }, + ParsingFailed { + context: String, + error: Box, + }, +} + +impl Error { + pub fn git_operation(operation: &str, error: git2::Error) -> Self { + Self::GitOperationFailed { + operation: operation.to_string(), + error, + } + } + + pub fn io_operation(operation: &str, error: std::io::Error) -> Self { + Self::IoOperationFailed { + operation: operation.to_string(), + error, + } + } + + pub fn parsing(context: &str, error: impl std::error::Error + Send + Sync + 'static) -> Self { + Self::ParsingFailed { + context: context.to_string(), + error: Box::new(error), + } + } } impl std::error::Error for Error {} @@ -126,18 +163,18 @@ impl Display for Error { "No editor environment variable set ({})", crate::ops::show::EDITOR_VARS.join(", ") )), - Error::GitStatus(e) => f.write_fmt(format_args!("Git status error: {e}")), - Error::GitRebaseStatus(e) => f.write_fmt(format_args!("Git rebase status error: {e}")), + Error::GitStatus(e) => f.write_fmt(format_args!("Failed to get Git status: {e}. Ensure the repository is accessible.")), + Error::GitRebaseStatus(e) => f.write_fmt(format_args!("Failed to get Git rebase status: {e}. Check if a rebase is in progress.")), Error::CmdAlreadyRunning => f.write_str("A command is already running"), Error::StashWorkTreeEmpty => f.write_str("Cannot stash: working tree is empty"), Error::CouldntAwaitCmd(e) => f.write_fmt(format_args!("Couldn't await command: {e}")), Error::NoRepoWorkdir => f.write_str("No repository working directory"), Error::SpawnCmd(e) => f.write_fmt(format_args!("Failed to spawn command: {e}")), Error::CmdBadExit(args, code) => f.write_fmt(format_args!( - "'{}' exited with code: {}", + "Command '{}' failed with exit code {}. Check the command output above for details.", args, code.map(|c| c.to_string()) - .unwrap_or_else(|| "".to_string()) + .unwrap_or_else(|| "unknown".to_string()) )), Error::CouldntReadCmdOutput(e) => { f.write_fmt(format_args!("Couldn't read command output: {e}")) @@ -147,7 +184,16 @@ impl Display for Error { } Error::OpenLogFile(e) => f.write_fmt(format_args!("Couldn't open log file: {e}")), Error::PromptAborted => f.write_str("Aborted"), - Error::NoMoreEvents => unimplemented!(), + Error::NoMoreEvents => f.write_str("No more events available"), + Error::GitOperationFailed { operation, error } => f.write_fmt(format_args!( + "Git operation '{operation}' failed: {error}. Check your repository state or try again." + )), + Error::IoOperationFailed { operation, error } => f.write_fmt(format_args!( + "I/O operation '{operation}' failed: {error}. Ensure file permissions and paths are correct." + )), + Error::ParsingFailed { context, error } => f.write_fmt(format_args!( + "Parsing failed in {context}: {error}. Verify the input format." + )), } } } diff --git a/src/git/mod.rs b/src/git/mod.rs index 9dc03a31fd..745f2e6a8c 100644 --- a/src/git/mod.rs +++ b/src/git/mod.rs @@ -130,7 +130,7 @@ fn branch_name_lossy(dir: &Path, hash: &str) -> Res> { pub(crate) fn diff_unstaged(repo: &Repository) -> Res { let diff = repo .diff_index_to_workdir(None, None) - .map_err(Error::GitDiff)?; + .map_err(|e| Error::git_operation("diff unstaged", e))?; let mut text = Vec::new(); diff.print(DiffFormat::Patch, |_delta, _hunk, line| { @@ -242,7 +242,9 @@ fn get_branch_status(repo: &Repository) -> Res { fn get_status_files(repo: &Repository) -> Res> { let mut files = Vec::new(); - let statuses = repo.statuses(None).map_err(Error::GitStatus)?; + let statuses = repo + .statuses(None) + .map_err(|e| Error::git_operation("status", e))?; for entry in statuses.iter() { let status = entry.status(); let path = entry.path().unwrap_or("").to_string(); diff --git a/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap b/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap index ddaaaf712e..e792698b46 100644 --- a/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap +++ b/src/tests/snapshots/gitu__tests__unstage__unstage_all_staged.snap @@ -21,5 +21,5 @@ expression: ctx.redact_buffer() ────────────────────────────────────────────────────────────────────────────────| $ git restore --staged two | fatal: could not resolve HEAD | -! 'git restore --staged two' exited with code: 128 | -styles_hash: ecbc9ac8f4e96987 +! Command 'git restore --staged two' failed with exit code 128. Check the comman| +styles_hash: 4368f58b70f6ef86