From 03505b9eb69c0b258d165e6301f4cac8efb2af9c Mon Sep 17 00:00:00 2001 From: Mehran Kordi Date: Fri, 15 May 2020 15:15:26 +0200 Subject: [PATCH] Replace unwrap calls in asyncgit with error handling - closes #53 --- .gitignore | 1 + Cargo.lock | 56 ++++++++++++++ asyncgit/Cargo.toml | 3 +- asyncgit/src/diff.rs | 99 ++++++++++++++---------- asyncgit/src/error.rs | 21 +++++ asyncgit/src/lib.rs | 1 + asyncgit/src/revlog.rs | 77 ++++++++++--------- asyncgit/src/status.rs | 71 ++++++++++------- asyncgit/src/sync/commits_info.rs | 26 ++++--- asyncgit/src/sync/diff.rs | 109 ++++++++++++++++---------- asyncgit/src/sync/hooks.rs | 70 +++++++++-------- asyncgit/src/sync/hunks.rs | 28 +++---- asyncgit/src/sync/logwalker.rs | 27 +++---- asyncgit/src/sync/mod.rs | 56 +++++++------- asyncgit/src/sync/reset.rs | 122 ++++++++++++++++-------------- asyncgit/src/sync/status.rs | 49 +++++++----- asyncgit/src/sync/tags.rs | 6 +- asyncgit/src/sync/utils.rs | 120 +++++++++++++++++------------ src/app.rs | 16 +++- src/components/changes.rs | 10 ++- src/components/commit.rs | 6 +- src/tabs/revlog/mod.rs | 7 +- src/tabs/status.rs | 13 ++-- 23 files changed, 615 insertions(+), 379 deletions(-) create mode 100644 asyncgit/src/error.rs diff --git a/.gitignore b/.gitignore index 1c60cede..47c1a2ea 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,4 @@ /target /release .DS_Store +/.idea/ diff --git a/Cargo.lock b/Cargo.lock index 674688ef..cebc6a9f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -38,6 +38,7 @@ dependencies = [ "rayon-core", "scopetime", "tempfile", + "thiserror", ] [[package]] @@ -565,6 +566,24 @@ version = "0.2.6" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "74490b50b9fbe561ac330df47c08f3f33073d2d00c150f719147d7c54522fa1b" +[[package]] +name = "proc-macro2" +version = "1.0.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "8872cf6f48eee44265156c111456a700ab3483686b3f96df4cf5481c89157319" +dependencies = [ + "unicode-xid", +] + +[[package]] +name = "quote" +version = "1.0.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "42934bc9c8ab0d3b273a16d8551c8f0fcff46be73276ca083ec2414c15c4ba5e" +dependencies = [ + "proc-macro2", +] + [[package]] name = "rand" version = "0.7.3" @@ -719,6 +738,17 @@ version = "1.4.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "c7cb5678e1615754284ec264d9bb5b4c27d2018577fd90ac0ceb578591ed5ee4" +[[package]] +name = "syn" +version = "1.0.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4696caa4048ac7ce2bcd2e484b3cef88c1004e41b8e945a277e2c25dc0b72060" +dependencies = [ + "proc-macro2", + "quote", + "unicode-xid", +] + [[package]] name = "tempfile" version = "3.1.0" @@ -733,6 +763,26 @@ dependencies = [ "winapi 0.3.8", ] +[[package]] +name = "thiserror" +version = "1.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "467e5ff447618a916519a4e0d62772ab14f434897f3d63f05d8700ef1e9b22c1" +dependencies = [ + "thiserror-impl", +] + +[[package]] +name = "thiserror-impl" +version = "1.0.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e63c1091225b9834089b429bc4a2e01223470e3183e891582909e9d1c4cb55d9" +dependencies = [ + "proc-macro2", + "quote", + "syn", +] + [[package]] name = "time" version = "0.1.43" @@ -788,6 +838,12 @@ version = "0.1.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "caaa9d531767d1ff2150b9332433f32a24622147e5ebb1f26409d5da67afd479" +[[package]] +name = "unicode-xid" +version = "0.2.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "826e7639553986605ec5979c7dd957c7895e93eabed50ab2ffa7f6128a75097c" + [[package]] name = "url" version = "2.1.1" diff --git a/asyncgit/Cargo.toml b/asyncgit/Cargo.toml index cc22f380..e84d24f0 100644 --- a/asyncgit/Cargo.toml +++ b/asyncgit/Cargo.toml @@ -17,4 +17,5 @@ crossbeam-channel = "0.4" log = "0.4" is_executable = "0.1" scopetime = { path = "../scopetime", version = "0.1" } -tempfile = "3.1" \ No newline at end of file +tempfile = "3.1" +thiserror = "1.0" \ No newline at end of file diff --git a/asyncgit/src/diff.rs b/asyncgit/src/diff.rs index aad272b8..8bd0ee7c 100644 --- a/asyncgit/src/diff.rs +++ b/asyncgit/src/diff.rs @@ -1,3 +1,4 @@ +use crate::error::Result; use crate::{hash, sync, AsyncNotification, FileDiff, CWD}; use crossbeam_channel::Sender; use log::trace; @@ -42,21 +43,22 @@ impl AsyncDiff { } /// - pub fn last(&mut self) -> Option<(DiffParams, FileDiff)> { - let last = self.last.lock().unwrap(); - if let Some(res) = last.clone() { - Some((res.params, res.result)) - } else { - None - } + pub fn last(&mut self) -> Result> { + let last = self.last.lock()?; + + Ok(match last.clone() { + Some(res) => Some((res.params, res.result)), + None => None, + }) } /// - pub fn refresh(&mut self) { - if let Some(param) = self.get_last_param() { - self.clear_current(); - self.request(param); + pub fn refresh(&mut self) -> Result<()> { + if let Ok(Some(param)) = self.get_last_param() { + self.clear_current()?; + self.request(param)?; } + Ok(()) } /// @@ -68,16 +70,16 @@ impl AsyncDiff { pub fn request( &mut self, params: DiffParams, - ) -> Option { + ) -> Result> { trace!("request"); let hash = hash(¶ms); { - let mut current = self.current.lock().unwrap(); + let mut current = self.current.lock()?; if current.0 == hash { - return current.1.clone(); + return Ok(current.1.clone()); } current.0 = hash; @@ -91,25 +93,13 @@ impl AsyncDiff { rayon_core::spawn(move || { arc_pending.fetch_add(1, Ordering::Relaxed); - let res = - sync::diff::get_diff(CWD, params.0.clone(), params.1); - let mut notify = false; - { - let mut current = arc_current.lock().unwrap(); - if current.0 == hash { - current.1 = Some(res.clone()); - notify = true; - } - } - - { - let mut last = arc_last.lock().unwrap(); - *last = Some(LastResult { - result: res, - hash, - params, - }); - } + let notify = AsyncDiff::get_diff_helper( + params, + arc_last, + arc_current, + hash, + ) + .expect("error getting diff"); arc_pending.fetch_sub(1, Ordering::Relaxed); @@ -120,16 +110,49 @@ impl AsyncDiff { } }); - None + Ok(None) } - fn get_last_param(&self) -> Option { - self.last.lock().unwrap().clone().map(|e| e.params) + fn get_diff_helper( + params: DiffParams, + arc_last: Arc< + Mutex>>, + >, + arc_current: Arc>>, + hash: u64, + ) -> Result { + let res = + sync::diff::get_diff(CWD, params.0.clone(), params.1)?; + + let mut notify = false; + { + let mut current = arc_current.lock()?; + if current.0 == hash { + current.1 = Some(res.clone()); + notify = true; + } + } + + { + let mut last = arc_last.lock()?; + *last = Some(LastResult { + result: res, + hash, + params, + }); + } + + Ok(notify) } - fn clear_current(&mut self) { - let mut current = self.current.lock().unwrap(); + fn get_last_param(&self) -> Result> { + Ok(self.last.lock()?.clone().map(|e| e.params)) + } + + fn clear_current(&mut self) -> Result<()> { + let mut current = self.current.lock()?; current.0 = 0; current.1 = None; + Ok(()) } } diff --git a/asyncgit/src/error.rs b/asyncgit/src/error.rs new file mode 100644 index 00000000..7312932a --- /dev/null +++ b/asyncgit/src/error.rs @@ -0,0 +1,21 @@ +use thiserror::Error; + +#[derive(Error, Debug)] +pub enum Error { + #[error("`{0}`")] + Generic(String), + + #[error("io error:{0}")] + Io(#[from] std::io::Error), + + #[error("git error:{0}")] + Git(#[from] git2::Error), +} + +pub type Result = std::result::Result; + +impl From> for Error { + fn from(error: std::sync::PoisonError) -> Self { + Error::Generic(format!("poison error: {}", error)) + } +} diff --git a/asyncgit/src/lib.rs b/asyncgit/src/lib.rs index 74eb4199..96214f5b 100644 --- a/asyncgit/src/lib.rs +++ b/asyncgit/src/lib.rs @@ -5,6 +5,7 @@ #![deny(clippy::all)] mod diff; +mod error; mod revlog; mod status; pub mod sync; diff --git a/asyncgit/src/revlog.rs b/asyncgit/src/revlog.rs index 8df8e451..f76ba5d6 100644 --- a/asyncgit/src/revlog.rs +++ b/asyncgit/src/revlog.rs @@ -1,3 +1,4 @@ +use crate::error::Result; use crate::{sync, AsyncNotification, CWD}; use crossbeam_channel::Sender; use git2::Oid; @@ -31,8 +32,8 @@ impl AsyncLog { } /// - pub fn count(&mut self) -> usize { - self.current.lock().unwrap().len() + pub fn count(&mut self) -> Result { + Ok(self.current.lock()?.len()) } /// @@ -40,13 +41,13 @@ impl AsyncLog { &self, start_index: usize, amount: usize, - ) -> Vec { - let list = self.current.lock().unwrap(); + ) -> Result> { + let list = self.current.lock()?; let list_len = list.len(); let min = start_index.min(list_len); let max = min + amount; let max = max.min(list_len); - Vec::from_iter(list[min..max].iter().cloned()) + Ok(Vec::from_iter(list[min..max].iter().cloned())) } /// @@ -55,48 +56,56 @@ impl AsyncLog { } /// - pub fn fetch(&mut self) { + pub fn fetch(&mut self) -> Result<()> { if !self.is_pending() { - self.clear(); + self.clear()?; let arc_current = Arc::clone(&self.current); let sender = self.sender.clone(); let arc_pending = Arc::clone(&self.pending); + rayon_core::spawn(move || { - arc_pending.store(true, Ordering::Relaxed); - scope_time!("async::revlog"); - - let mut entries = Vec::with_capacity(LIMIT_COUNT); - let r = repo(CWD); - let mut walker = LogWalker::new(&r); - loop { - entries.clear(); - let res_is_err = walker - .read(&mut entries, LIMIT_COUNT) - .is_err(); - - if !res_is_err { - let mut current = arc_current.lock().unwrap(); - current.extend(entries.iter()); - } - - if res_is_err || entries.len() <= 1 { - break; - } else { - Self::notify(&sender); - } - } - + arc_pending.store(true, Ordering::Relaxed); + AsyncLog::fetch_helper(arc_current, &sender) + .expect("failed to fetch"); arc_pending.store(false, Ordering::Relaxed); - Self::notify(&sender); }); } + Ok(()) } - fn clear(&mut self) { - self.current.lock().unwrap().clear(); + fn fetch_helper( + arc_current: Arc>>, + sender: &Sender, + ) -> Result<()> { + let mut entries = Vec::with_capacity(LIMIT_COUNT); + let r = repo(CWD)?; + let mut walker = LogWalker::new(&r); + loop { + entries.clear(); + let res_is_err = + walker.read(&mut entries, LIMIT_COUNT).is_err(); + + if !res_is_err { + let mut current = arc_current.lock()?; + current.extend(entries.iter()); + } + + if res_is_err || entries.len() <= 1 { + break; + } else { + Self::notify(&sender); + } + } + + Ok(()) + } + + fn clear(&mut self) -> Result<()> { + self.current.lock()?.clear(); + Ok(()) } fn notify(sender: &Sender) { diff --git a/asyncgit/src/status.rs b/asyncgit/src/status.rs index f29792ac..1ce62864 100644 --- a/asyncgit/src/status.rs +++ b/asyncgit/src/status.rs @@ -1,4 +1,6 @@ -use crate::{hash, sync, AsyncNotification, StatusItem, CWD}; +use crate::{ + error::Result, hash, sync, AsyncNotification, StatusItem, CWD, +}; use crossbeam_channel::Sender; use log::trace; use std::{ @@ -38,9 +40,9 @@ impl AsyncStatus { } /// - pub fn last(&mut self) -> Status { - let last = self.last.lock().unwrap(); - last.clone() + pub fn last(&mut self) -> Result { + let last = self.last.lock()?; + Ok(last.clone()) } /// @@ -49,16 +51,16 @@ impl AsyncStatus { } /// - pub fn fetch(&mut self, request: u64) -> Option { + pub fn fetch(&mut self, request: u64) -> Result> { let hash_request = hash(&request); trace!("request: {} [hash: {}]", request, hash_request); { - let mut current = self.current.lock().unwrap(); + let mut current = self.current.lock()?; if current.0 == hash_request { - return current.1.clone(); + return Ok(current.1.clone()); } current.0 = hash_request; @@ -72,20 +74,12 @@ impl AsyncStatus { rayon_core::spawn(move || { arc_pending.fetch_add(1, Ordering::Relaxed); - let res = Self::get_status(); - trace!("status fetched: {}", hash(&res)); - - { - let mut current = arc_current.lock().unwrap(); - if current.0 == hash_request { - current.1 = Some(res.clone()); - } - } - - { - let mut last = arc_last.lock().unwrap(); - *last = res; - } + AsyncStatus::fetch_helper( + hash_request, + arc_current, + arc_last, + ) + .expect("failed to fetch status"); arc_pending.fetch_sub(1, Ordering::Relaxed); @@ -94,14 +88,37 @@ impl AsyncStatus { .expect("error sending status"); }); - None + Ok(None) } - fn get_status() -> Status { - let work_dir = - sync::status::get_status(CWD, StatusType::WorkingDir); - let stage = sync::status::get_status(CWD, StatusType::Stage); + fn fetch_helper( + hash_request: u64, + arc_current: Arc>>, + arc_last: Arc>, + ) -> Result<()> { + let res = Self::get_status()?; + trace!("status fetched: {}", hash(&res)); - Status { stage, work_dir } + { + let mut current = arc_current.lock()?; + if current.0 == hash_request { + current.1 = Some(res.clone()); + } + } + + { + let mut last = arc_last.lock()?; + *last = res; + } + + Ok(()) + } + + fn get_status() -> Result { + let work_dir = + sync::status::get_status(CWD, StatusType::WorkingDir)?; + let stage = sync::status::get_status(CWD, StatusType::Stage)?; + + Ok(Status { stage, work_dir }) } } diff --git a/asyncgit/src/sync/commits_info.rs b/asyncgit/src/sync/commits_info.rs index d8f5ea3c..be734f06 100644 --- a/asyncgit/src/sync/commits_info.rs +++ b/asyncgit/src/sync/commits_info.rs @@ -1,4 +1,5 @@ use super::utils::repo; +use crate::error::Result; use git2::{Commit, Error, Oid}; use scopetime::scope_time; @@ -19,12 +20,16 @@ pub struct CommitInfo { pub fn get_commits_info( repo_path: &str, ids: &[Oid], -) -> Result, Error> { +) -> Result> { scope_time!("get_commits_info"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; - let commits = ids.iter().map(|id| repo.find_commit(*id).unwrap()); + let commits = ids + .iter() + .map(|id| repo.find_commit(*id)) + .collect::, Error>>()? + .into_iter(); let res = commits .map(|c: Commit| { @@ -66,27 +71,24 @@ fn limit_str(s: &str, limit: usize) -> String { mod tests { use super::get_commits_info; + use crate::error::Result; use crate::sync::{ commit, stage_add_file, tests::repo_init_empty, }; - use std::{ - fs::File, - io::{Error, Write}, - path::Path, - }; + use std::{fs::File, io::Write, path::Path}; #[test] - fn test_log() -> Result<(), Error> { + fn test_log() -> Result<()> { let file_path = Path::new("foo"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); File::create(&root.join(file_path))?.write_all(b"a")?; - stage_add_file(repo_path, file_path); + stage_add_file(repo_path, file_path).unwrap(); let c1 = commit(repo_path, "commit1").unwrap(); File::create(&root.join(file_path))?.write_all(b"a")?; - stage_add_file(repo_path, file_path); + stage_add_file(repo_path, file_path).unwrap(); let c2 = commit(repo_path, "commit2").unwrap(); let res = get_commits_info(repo_path, &vec![c2, c1]).unwrap(); diff --git a/asyncgit/src/sync/diff.rs b/asyncgit/src/sync/diff.rs index 86eac44d..7b65bfeb 100644 --- a/asyncgit/src/sync/diff.rs +++ b/asyncgit/src/sync/diff.rs @@ -1,10 +1,11 @@ //! sync git api for fetching a diff use super::utils; -use crate::hash; +use crate::error::Result; +use crate::{error::Error, hash}; use git2::{ - Delta, Diff, DiffDelta, DiffFormat, DiffHunk, DiffOptions, Error, - Patch, Repository, + Delta, Diff, DiffDelta, DiffFormat, DiffHunk, DiffOptions, Patch, + Repository, }; use scopetime::scope_time; use std::{fs, path::Path}; @@ -79,7 +80,7 @@ pub(crate) fn get_diff_raw<'a>( p: &str, stage: bool, reverse: bool, -) -> Result, Error> { +) -> Result> { let mut opt = DiffOptions::new(); opt.pathspec(p); opt.reverse(reverse); @@ -87,8 +88,15 @@ pub(crate) fn get_diff_raw<'a>( let diff = if stage { // diff against head if let Ok(ref_head) = repo.head() { - let parent = - repo.find_commit(ref_head.target().unwrap())?; + let parent = repo.find_commit( + ref_head.target().ok_or_else(|| { + let name = ref_head.name().unwrap_or("??"); + Error::Generic( + format!("can not find the target of symbolic references: {}", name) + ) + })?, + )?; + let tree = parent.tree()?; repo.diff_tree_to_index( Some(&tree), @@ -111,15 +119,22 @@ pub(crate) fn get_diff_raw<'a>( Ok(diff) } -//TODO: return Option /// -pub fn get_diff(repo_path: &str, p: String, stage: bool) -> FileDiff { +pub fn get_diff( + repo_path: &str, + p: String, + stage: bool, +) -> Result { scope_time!("get_diff"); - let repo = utils::repo(repo_path); - let repo_path = repo.path().parent().unwrap(); - - let diff = get_diff_raw(&repo, &p, stage, false).unwrap(); + let repo = utils::repo(repo_path)?; + let repo_path = repo.path().parent().ok_or_else(|| { + Error::Generic( + "repositories located at root are not supported." + .to_string(), + ) + })?; + let diff = get_diff_raw(&repo, &p, stage, false)?; let mut res: FileDiff = FileDiff::default(); let mut current_lines = Vec::new(); @@ -165,11 +180,18 @@ pub fn get_diff(repo_path: &str, p: String, stage: bool) -> FileDiff { }; let new_file_diff = if diff.deltas().len() == 1 { + // it's safe to unwrap here because we check first that diff.deltas has a single element. let delta: DiffDelta = diff.deltas().next().unwrap(); if delta.status() == Delta::Untracked { - let newfile_path = - repo_path.join(delta.new_file().path().unwrap()); + let relative_path = + delta.new_file().path().ok_or_else(|| { + Error::Generic( + "new file path is unspecified.".to_string(), + ) + })?; + + let newfile_path = repo_path.join(relative_path); if let Some(newfile_content) = new_file_content(&newfile_path) @@ -180,15 +202,13 @@ pub fn get_diff(repo_path: &str, p: String, stage: bool) -> FileDiff { newfile_content.as_bytes(), Some(&newfile_path), None, - ) - .unwrap(); + )?; patch .print(&mut |_delta, hunk:Option, line: git2::DiffLine| { put(hunk,line); true - }) - .unwrap(); + })?; true } else { @@ -208,15 +228,14 @@ pub fn get_diff(repo_path: &str, p: String, stage: bool) -> FileDiff { put(hunk, line); true }, - ) - .unwrap(); + )?; } if !current_lines.is_empty() { adder(¤t_hunk.unwrap(), ¤t_lines); } - res + Ok(res) } fn new_file_content(path: &Path) -> Option { @@ -238,6 +257,7 @@ fn new_file_content(path: &Path) -> Option { #[cfg(test)] mod tests { use super::get_diff; + use crate::error::Result; use crate::sync::{ stage_add_file, status::{get_status, StatusType}, @@ -245,17 +265,18 @@ mod tests { }; use std::{ fs::{self, File}, - io::{Error, Write}, + io::Write, path::Path, }; #[test] fn test_untracked_subfolder() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); - let res = get_status(repo_path, StatusType::WorkingDir); + let res = + get_status(repo_path, StatusType::WorkingDir).unwrap(); assert_eq!(res.len(), 0); fs::create_dir(&root.join("foo")).unwrap(); @@ -264,11 +285,13 @@ mod tests { .write_all(b"test\nfoo") .unwrap(); - let res = get_status(repo_path, StatusType::WorkingDir); + let res = + get_status(repo_path, StatusType::WorkingDir).unwrap(); assert_eq!(res.len(), 1); let diff = - get_diff(repo_path, "foo/bar.txt".to_string(), false); + get_diff(repo_path, "foo/bar.txt".to_string(), false) + .unwrap(); assert_eq!(diff.hunks.len(), 1); assert_eq!(diff.hunks[0].lines[1].content, "test\n"); @@ -277,7 +300,7 @@ mod tests { #[test] fn test_empty_repo() { let file_path = Path::new("foo.txt"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -290,7 +313,10 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - assert_eq!(stage_add_file(repo_path, file_path), true); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + true + ); assert_eq!(get_statuses(repo_path), (0, 1)); @@ -298,7 +324,8 @@ mod tests { repo_path, String::from(file_path.to_str().unwrap()), true, - ); + ) + .unwrap(); assert_eq!(diff.hunks.len(), 1); } @@ -331,7 +358,7 @@ mod tests { #[test] fn test_hunks() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -346,11 +373,13 @@ mod tests { .unwrap(); } - let res = get_status(repo_path, StatusType::WorkingDir); + let res = + get_status(repo_path, StatusType::WorkingDir).unwrap(); assert_eq!(res.len(), 1); assert_eq!(res[0].path, "bar.txt"); - let res = stage_add_file(repo_path, Path::new("bar.txt")); + let res = + stage_add_file(repo_path, Path::new("bar.txt")).unwrap(); assert_eq!(res, true); assert_eq!(get_statuses(repo_path), (0, 1)); @@ -364,7 +393,8 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 1)); - let res = get_diff(repo_path, "bar.txt".to_string(), false); + let res = get_diff(repo_path, "bar.txt".to_string(), false) + .unwrap(); assert_eq!(res.hunks.len(), 2) } @@ -372,7 +402,7 @@ mod tests { #[test] fn test_diff_newfile_in_sub_dir_current_dir() { let file_path = Path::new("foo/foo.txt"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let sub_path = root.join("foo/"); @@ -387,16 +417,16 @@ mod tests { sub_path.to_str().unwrap(), String::from(file_path.to_str().unwrap()), false, - ); + ) + .unwrap(); assert_eq!(diff.hunks[0].lines[1].content, "test"); } #[test] - fn test_diff_new_binary_file_using_invalid_utf8( - ) -> Result<(), Error> { + fn test_diff_new_binary_file_using_invalid_utf8() -> Result<()> { let file_path = Path::new("bar"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -407,7 +437,8 @@ mod tests { repo_path, String::from(file_path.to_str().unwrap()), false, - ); + ) + .unwrap(); assert_eq!(diff.hunks.len(), 0); diff --git a/asyncgit/src/sync/hooks.rs b/asyncgit/src/sync/hooks.rs index 2ec38530..8566fb2e 100644 --- a/asyncgit/src/sync/hooks.rs +++ b/asyncgit/src/sync/hooks.rs @@ -1,3 +1,4 @@ +use crate::error::{Error, Result}; use is_executable::IsExecutable; use scopetime::scope_time; use std::{ @@ -14,37 +15,42 @@ const HOOK_COMMIT_MSG: &str = ".git/hooks/commit-msg"; pub fn hooks_commit_msg( repo_path: &str, msg: &mut String, -) -> HookResult { +) -> Result { scope_time!("hooks_commit_msg"); if hook_runable(repo_path, HOOK_COMMIT_MSG) { - let mut file = NamedTempFile::new().unwrap(); + let mut file = NamedTempFile::new()?; - write!(file, "{}", msg).unwrap(); + write!(file, "{}", msg)?; - let file_path = file.path().to_str().unwrap(); + let file_path = file.path().to_str().ok_or_else(|| { + Error::Generic( + "temp file path contains invalid unicode sequences." + .to_string(), + ) + })?; let res = run_hook(repo_path, HOOK_COMMIT_MSG, &[&file_path]); // load possibly altered msg - let mut file = file.reopen().unwrap(); + let mut file = file.reopen()?; msg.clear(); - file.read_to_string(msg).unwrap(); + file.read_to_string(msg)?; - res + Ok(res) } else { - HookResult::Ok + Ok(HookResult::Ok) } } /// -pub fn hooks_post_commit(repo_path: &str) -> HookResult { +pub fn hooks_post_commit(repo_path: &str) -> Result { scope_time!("hooks_post_commit"); if hook_runable(repo_path, HOOK_POST_COMMIT) { - run_hook(repo_path, HOOK_POST_COMMIT, &[]) + Ok(run_hook(repo_path, HOOK_POST_COMMIT, &[])) } else { - HookResult::Ok + Ok(HookResult::Ok) } } @@ -65,19 +71,19 @@ pub enum HookResult { } fn run_hook(path: &str, cmd: &str, args: &[&str]) -> HookResult { - let output = - Command::new(cmd).args(args).current_dir(path).output(); + match Command::new(cmd).args(args).current_dir(path).output() { + Ok(output) => { + if output.status.success() { + HookResult::Ok + } else { + let err = String::from_utf8_lossy(&output.stderr); + let out = String::from_utf8_lossy(&output.stdout); + let formatted = format!("{}{}", out, err); - let output = output.expect("general hook error"); - - if output.status.success() { - HookResult::Ok - } else { - let err = String::from_utf8(output.stderr).unwrap(); - let out = String::from_utf8(output.stdout).unwrap(); - let formatted = format!("{}{}", out, err); - - HookResult::NotOk(formatted) + HookResult::NotOk(formatted) + } + } + Err(e) => HookResult::NotOk(format!("{}", e)), } } @@ -89,16 +95,16 @@ mod tests { #[test] fn test_smoke() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); let mut msg = String::from("test"); - let res = hooks_commit_msg(repo_path, &mut msg); + let res = hooks_commit_msg(repo_path, &mut msg).unwrap(); assert_eq!(res, HookResult::Ok); - let res = hooks_post_commit(repo_path); + let res = hooks_post_commit(repo_path).unwrap(); assert_eq!(res, HookResult::Ok); } @@ -119,7 +125,7 @@ mod tests { #[test] #[cfg(not(windows))] fn test_hooks_commit_msg_ok() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -131,7 +137,7 @@ exit 0 create_hook(root, HOOK_COMMIT_MSG, hook); let mut msg = String::from("test"); - let res = hooks_commit_msg(repo_path, &mut msg); + let res = hooks_commit_msg(repo_path, &mut msg).unwrap(); assert_eq!(res, HookResult::Ok); @@ -141,7 +147,7 @@ exit 0 #[test] #[cfg(not(windows))] fn test_hooks_commit_msg() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -155,7 +161,7 @@ exit 1 create_hook(root, HOOK_COMMIT_MSG, hook); let mut msg = String::from("test"); - let res = hooks_commit_msg(repo_path, &mut msg); + let res = hooks_commit_msg(repo_path, &mut msg).unwrap(); assert_eq!( res, @@ -168,7 +174,7 @@ exit 1 #[test] #[cfg(not(windows))] fn test_commit_msg_no_block_but_alter() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -181,7 +187,7 @@ exit 0 create_hook(root, HOOK_COMMIT_MSG, hook); let mut msg = String::from("test"); - let res = hooks_commit_msg(repo_path, &mut msg); + let res = hooks_commit_msg(repo_path, &mut msg).unwrap(); assert_eq!(res, HookResult::Ok); assert_eq!(msg, String::from("msg\n")); diff --git a/asyncgit/src/sync/hunks.rs b/asyncgit/src/sync/hunks.rs index 1f72724b..3a727b7f 100644 --- a/asyncgit/src/sync/hunks.rs +++ b/asyncgit/src/sync/hunks.rs @@ -2,7 +2,8 @@ use super::{ diff::{get_diff_raw, HunkHeader}, utils::repo, }; -use crate::hash; +use crate::error::Result; +use crate::{error::Error, hash}; use git2::{ApplyLocation, ApplyOptions, Diff}; use log::error; use scopetime::scope_time; @@ -12,12 +13,12 @@ pub fn stage_hunk( repo_path: &str, file_path: String, hunk_hash: u64, -) -> bool { +) -> Result<()> { scope_time!("stage_hunk"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; - let diff = get_diff_raw(&repo, &file_path, false, false).unwrap(); + let diff = get_diff_raw(&repo, &file_path, false, false)?; let mut opt = ApplyOptions::new(); opt.hunk_callback(|hunk| { @@ -25,8 +26,9 @@ pub fn stage_hunk( hash(&header) == hunk_hash }); - repo.apply(&diff, ApplyLocation::Index, Some(&mut opt)) - .is_ok() + repo.apply(&diff, ApplyLocation::Index, Some(&mut opt))?; + + Ok(()) } fn find_hunk_index(diff: &Diff, hunk_hash: u64) -> Option { @@ -60,22 +62,22 @@ pub fn unstage_hunk( repo_path: &str, file_path: String, hunk_hash: u64, -) -> bool { +) -> Result { scope_time!("revert_hunk"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; - let diff = get_diff_raw(&repo, &file_path, true, false).unwrap(); + let diff = get_diff_raw(&repo, &file_path, true, false)?; let diff_count_positive = diff.deltas().len(); let hunk_index = find_hunk_index(&diff, hunk_hash); if hunk_index.is_none() { error!("hunk not found"); - return false; + return Err(Error::Generic("hunk not found".to_string())); } - let diff = get_diff_raw(&repo, &file_path, true, true).unwrap(); + let diff = get_diff_raw(&repo, &file_path, true, true)?; assert_eq!(diff.deltas().len(), diff_count_positive); @@ -100,9 +102,9 @@ pub fn unstage_hunk( .is_err() { error!("apply failed"); - return false; + return Err(Error::Generic("apply failed".to_string())); } } - count == 1 + Ok(count == 1) } diff --git a/asyncgit/src/sync/logwalker.rs b/asyncgit/src/sync/logwalker.rs index cd7c61b5..b364db4d 100644 --- a/asyncgit/src/sync/logwalker.rs +++ b/asyncgit/src/sync/logwalker.rs @@ -1,4 +1,5 @@ -use git2::{Error, Oid, Repository, Revwalk}; +use crate::error::Result; +use git2::{Oid, Repository, Revwalk}; /// pub struct LogWalker<'a> { @@ -20,7 +21,7 @@ impl<'a> LogWalker<'a> { &mut self, out: &mut Vec, limit: usize, - ) -> Result { + ) -> Result { let mut count = 0_usize; if self.revwalk.is_none() { @@ -53,24 +54,20 @@ mod tests { commit, get_commits_info, stage_add_file, tests::repo_init_empty, }; - use std::{ - fs::File, - io::{Error, Write}, - path::Path, - }; + use std::{fs::File, io::Write, path::Path}; #[test] - fn test_limit() -> Result<(), Error> { + fn test_limit() -> Result<()> { let file_path = Path::new("foo"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); File::create(&root.join(file_path))?.write_all(b"a")?; - stage_add_file(repo_path, file_path); + stage_add_file(repo_path, file_path).unwrap(); commit(repo_path, "commit1").unwrap(); File::create(&root.join(file_path))?.write_all(b"a")?; - stage_add_file(repo_path, file_path); + stage_add_file(repo_path, file_path).unwrap(); let oid2 = commit(repo_path, "commit2").unwrap(); let mut items = Vec::new(); @@ -84,17 +81,17 @@ mod tests { } #[test] - fn test_logwalker() -> Result<(), Error> { + fn test_logwalker() -> Result<()> { let file_path = Path::new("foo"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); File::create(&root.join(file_path))?.write_all(b"a")?; - stage_add_file(repo_path, file_path); + stage_add_file(repo_path, file_path).unwrap(); commit(repo_path, "commit1").unwrap(); File::create(&root.join(file_path))?.write_all(b"a")?; - stage_add_file(repo_path, file_path); + stage_add_file(repo_path, file_path).unwrap(); let oid2 = commit(repo_path, "commit2").unwrap(); let mut items = Vec::new(); diff --git a/asyncgit/src/sync/mod.rs b/asyncgit/src/sync/mod.rs index ba76509a..ab9ce899 100644 --- a/asyncgit/src/sync/mod.rs +++ b/asyncgit/src/sync/mod.rs @@ -25,36 +25,37 @@ pub use utils::{ #[cfg(test)] mod tests { use super::status::{get_status, StatusType}; + use crate::error::Result; use git2::Repository; use std::process::Command; use tempfile::TempDir; /// - pub fn repo_init_empty() -> (TempDir, Repository) { - let td = TempDir::new().unwrap(); - let repo = Repository::init(td.path()).unwrap(); + pub fn repo_init_empty() -> Result<(TempDir, Repository)> { + let td = TempDir::new()?; + let repo = Repository::init(td.path())?; { - let mut config = repo.config().unwrap(); - config.set_str("user.name", "name").unwrap(); - config.set_str("user.email", "email").unwrap(); + let mut config = repo.config()?; + config.set_str("user.name", "name")?; + config.set_str("user.email", "email")?; } - (td, repo) + Ok((td, repo)) } /// - pub fn repo_init() -> (TempDir, Repository) { - let td = TempDir::new().unwrap(); - let repo = Repository::init(td.path()).unwrap(); + pub fn repo_init() -> Result<(TempDir, Repository)> { + let td = TempDir::new()?; + let repo = Repository::init(td.path())?; { - let mut config = repo.config().unwrap(); - config.set_str("user.name", "name").unwrap(); - config.set_str("user.email", "email").unwrap(); + let mut config = repo.config()?; + config.set_str("user.name", "name")?; + config.set_str("user.email", "email")?; - let mut index = repo.index().unwrap(); - let id = index.write_tree().unwrap(); + let mut index = repo.index()?; + let id = index.write_tree()?; - let tree = repo.find_tree(id).unwrap(); - let sig = repo.signature().unwrap(); + let tree = repo.find_tree(id)?; + let sig = repo.signature()?; repo.commit( Some("HEAD"), &sig, @@ -62,23 +63,25 @@ mod tests { "initial", &tree, &[], - ) - .unwrap(); + )?; } - (td, repo) + Ok((td, repo)) } /// helper returning amount of files with changes in the (wd,stage) pub fn get_statuses(repo_path: &str) -> (usize, usize) { ( - get_status(repo_path, StatusType::WorkingDir).len(), - get_status(repo_path, StatusType::Stage).len(), + get_status(repo_path, StatusType::WorkingDir) + .unwrap() + .len(), + get_status(repo_path, StatusType::Stage).unwrap().len(), ) } /// pub fn debug_cmd_print(path: &str, cmd: &str) { - eprintln!("\n----\n{}", debug_cmd(path, cmd)) + let cmd = debug_cmd(path, cmd); + eprintln!("\n----\n{}", cmd); } fn debug_cmd(path: &str, cmd: &str) -> String { @@ -87,17 +90,18 @@ mod tests { .args(&["/C", cmd]) .current_dir(path) .output() + .unwrap() } else { Command::new("sh") .arg("-c") .arg(cmd) .current_dir(path) .output() + .unwrap() }; - let output = output.unwrap(); - let stdout = String::from_utf8(output.stdout).unwrap(); - let stderr = String::from_utf8(output.stderr).unwrap(); + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); format!( "{}{}", if stdout.is_empty() { diff --git a/asyncgit/src/sync/reset.rs b/asyncgit/src/sync/reset.rs index 11dfcf64..be8865e1 100644 --- a/asyncgit/src/sync/reset.rs +++ b/asyncgit/src/sync/reset.rs @@ -1,69 +1,75 @@ use super::utils::repo; +use crate::error::{Error, Result}; use git2::{build::CheckoutBuilder, ObjectType, Status}; use scopetime::scope_time; use std::{fs, path::Path}; /// -pub fn reset_stage(repo_path: &str, path: &Path) -> bool { +pub fn reset_stage(repo_path: &str, path: &Path) -> Result<()> { scope_time!("reset_stage"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; let head = repo.head(); if let Ok(reference) = head { - let obj = repo - .find_object( - reference.target().unwrap(), - Some(ObjectType::Commit), - ) - .unwrap(); + let obj = repo.find_object( + reference.target().ok_or_else(|| { + Error::Generic( + "can't get reference to symbolic reference," + .to_string(), + ) + })?, + Some(ObjectType::Commit), + )?; - repo.reset_default(Some(&obj), &[path]).is_ok() + repo.reset_default(Some(&obj), &[path])?; } else { - repo.reset_default(None, &[path]).is_ok() + repo.reset_default(None, &[path])?; } + + Ok(()) } /// -pub fn reset_workdir_file(repo_path: &str, path: &str) -> bool { +pub fn reset_workdir_file(repo_path: &str, path: &str) -> Result<()> { scope_time!("reset_workdir_file"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; // Note: early out for removing untracked files, due to bug in checkout_head code: // see https://github.com/libgit2/libgit2/issues/5089 - if let Ok(status) = repo.status_file(Path::new(path)) { - let removed_file_wd = if status == Status::WT_NEW - || (status == Status::WT_MODIFIED | Status::INDEX_NEW) - { - fs::remove_file(Path::new(repo_path).join(path)).is_ok() - } else { - false - }; + let status = repo.status_file(Path::new(path))?; - if status == Status::WT_NEW { - return removed_file_wd; - } + if status == Status::WT_NEW + || (status == Status::WT_MODIFIED | Status::INDEX_NEW) + { + fs::remove_file(Path::new(repo_path).join(path))?; + }; - let mut checkout_opts = CheckoutBuilder::new(); - checkout_opts - .update_index(true) // windows: needs this to be true WTF?! - .allow_conflicts(true) - .force() - .path(path); - - repo.checkout_index(None, Some(&mut checkout_opts)).is_ok() - } else { - false + if status == Status::WT_NEW { + return Ok(()); } + + let mut checkout_opts = CheckoutBuilder::new(); + checkout_opts + .update_index(true) // windows: needs this to be true WTF?! + .allow_conflicts(true) + .force() + .path(path); + + repo.checkout_index(None, Some(&mut checkout_opts))?; + Ok(()) } /// -pub fn reset_workdir_folder(repo_path: &str, path: &str) -> bool { +pub fn reset_workdir_folder( + repo_path: &str, + path: &str, +) -> Result<()> { scope_time!("reset_workdir_folder"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; let mut checkout_opts = CheckoutBuilder::new(); checkout_opts @@ -73,7 +79,8 @@ pub fn reset_workdir_folder(repo_path: &str, path: &str) -> bool { .force() .path(path); - repo.checkout_index(None, Some(&mut checkout_opts)).is_ok() + repo.checkout_index(None, Some(&mut checkout_opts))?; + Ok(()) } #[cfg(test)] @@ -81,6 +88,7 @@ mod tests { use super::{ reset_stage, reset_workdir_file, reset_workdir_folder, }; + use crate::error::Result; use crate::sync::{ status::{get_status, StatusType}, tests::{ @@ -90,7 +98,7 @@ mod tests { }; use std::{ fs::{self, File}, - io::{Error, Write}, + io::Write, path::Path, }; @@ -122,11 +130,12 @@ mod tests { #[test] fn test_reset_only_unstaged() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); - let res = get_status(repo_path, StatusType::WorkingDir); + let res = + get_status(repo_path, StatusType::WorkingDir).unwrap(); assert_eq!(res.len(), 0); let file_path = root.join("bar.txt"); @@ -140,7 +149,7 @@ mod tests { debug_cmd_print(repo_path, "git status"); - stage_add_file(repo_path, Path::new("bar.txt")); + stage_add_file(repo_path, Path::new("bar.txt")).unwrap(); debug_cmd_print(repo_path, "git status"); @@ -156,8 +165,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 1)); - let res = reset_workdir_file(repo_path, "bar.txt"); - assert_eq!(res, true); + reset_workdir_file(repo_path, "bar.txt").unwrap(); debug_cmd_print(repo_path, "git status"); @@ -166,7 +174,7 @@ mod tests { #[test] fn test_reset_untracked_in_subdir() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -182,8 +190,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - let res = reset_workdir_file(repo_path, "foo/bar.txt"); - assert_eq!(res, true); + reset_workdir_file(repo_path, "foo/bar.txt").unwrap(); debug_cmd_print(repo_path, "git status"); @@ -191,8 +198,8 @@ mod tests { } #[test] - fn test_reset_folder() -> Result<(), Error> { - let (_td, repo) = repo_init(); + fn test_reset_folder() -> Result<()> { + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -206,7 +213,7 @@ mod tests { .write_all(b"file3")?; } - assert!(stage_add_all(repo_path, "*")); + assert!(stage_add_all(repo_path, "*").unwrap()); commit(repo_path, "msg").unwrap(); { @@ -223,11 +230,12 @@ mod tests { assert_eq!(get_statuses(repo_path), (5, 0)); - stage_add_file(repo_path, Path::new("foo/file5.txt")); + stage_add_file(repo_path, Path::new("foo/file5.txt")) + .unwrap(); assert_eq!(get_statuses(repo_path), (4, 1)); - assert!(reset_workdir_folder(repo_path, "foo")); + reset_workdir_folder(repo_path, "foo").unwrap(); assert_eq!(get_statuses(repo_path), (1, 1)); @@ -236,7 +244,7 @@ mod tests { #[test] fn test_reset_untracked_in_subdir_and_index() { - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); let file = "foo/bar.txt"; @@ -266,8 +274,7 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 1)); - let res = reset_workdir_file(repo_path, file); - assert_eq!(res, true); + reset_workdir_file(repo_path, file).unwrap(); debug_cmd_print(repo_path, "git status"); @@ -277,7 +284,7 @@ mod tests { #[test] fn unstage_in_empty_repo() { let file_path = Path::new("foo.txt"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -288,11 +295,14 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - assert_eq!(stage_add_file(repo_path, file_path), true); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + true + ); assert_eq!(get_statuses(repo_path), (0, 1)); - assert_eq!(reset_stage(repo_path, file_path), true); + reset_stage(repo_path, file_path).unwrap(); assert_eq!(get_statuses(repo_path), (1, 0)); } diff --git a/asyncgit/src/sync/status.rs b/asyncgit/src/sync/status.rs index a3cfb547..41b52fcc 100644 --- a/asyncgit/src/sync/status.rs +++ b/asyncgit/src/sync/status.rs @@ -1,6 +1,7 @@ //! sync git api for fetching a status -use crate::sync::utils; +use crate::error::Result; +use crate::{error::Error, sync::utils}; use git2::{Status, StatusOptions, StatusShow}; use scopetime::scope_time; use std::path::Path; @@ -67,32 +68,42 @@ impl Into for StatusType { pub fn get_status( repo_path: &str, status_type: StatusType, -) -> Vec { +) -> Result> { scope_time!("get_index"); - let repo = utils::repo(repo_path); + let repo = utils::repo(repo_path)?; - let statuses = repo - .statuses(Some( - StatusOptions::default() - .show(status_type.into()) - .include_untracked(true) - .renames_head_to_index(true) - .recurse_untracked_dirs(true), - )) - .unwrap(); + let statuses = repo.statuses(Some( + StatusOptions::default() + .show(status_type.into()) + .include_untracked(true) + .renames_head_to_index(true) + .recurse_untracked_dirs(true), + ))?; let mut res = Vec::with_capacity(statuses.len()); for e in statuses.iter() { let status: Status = e.status(); - let path = if let Some(diff) = e.head_to_index() { - String::from( - diff.new_file().path().unwrap().to_str().unwrap(), - ) - } else { - e.path().unwrap().to_string() + let path = match e.head_to_index() { + Some(diff) => diff + .new_file() + .path() + .and_then(|x| x.to_str()) + .map(String::from) + .ok_or_else(|| { + Error::Generic( + "failed to get path to diff's new file." + .to_string(), + ) + })?, + None => e.path().map(String::from).ok_or_else(|| { + Error::Generic( + "failed to get the path to indexed file." + .to_string(), + ) + })?, }; res.push(StatusItem { @@ -105,5 +116,5 @@ pub fn get_status( Path::new(a.path.as_str()).cmp(Path::new(b.path.as_str())) }); - res + Ok(res) } diff --git a/asyncgit/src/sync/tags.rs b/asyncgit/src/sync/tags.rs index e9a0f18b..3ef2dc80 100644 --- a/asyncgit/src/sync/tags.rs +++ b/asyncgit/src/sync/tags.rs @@ -1,5 +1,5 @@ use super::utils::repo; -use git2::Error; +use crate::error::Result; use scopetime::scope_time; use std::collections::HashMap; @@ -7,12 +7,12 @@ use std::collections::HashMap; pub type Tags = HashMap; /// returns `Tags` type filled with all tags found in repo -pub fn get_tags(repo_path: &str) -> Result { +pub fn get_tags(repo_path: &str) -> Result { scope_time!("get_tags"); let mut res = Tags::new(); - let repo = repo(repo_path); + let repo = repo(repo_path)?; for name in repo.tag_names(None)?.iter() { if let Some(name) = name { diff --git a/asyncgit/src/sync/utils.rs b/asyncgit/src/sync/utils.rs index 00989018..e891e967 100644 --- a/asyncgit/src/sync/utils.rs +++ b/asyncgit/src/sync/utils.rs @@ -1,8 +1,7 @@ //! sync git api (various methods) -use git2::{ - Error, IndexAddOption, Oid, Repository, RepositoryOpenFlags, -}; +use crate::error::{Error, Result}; +use git2::{IndexAddOption, Oid, Repository, RepositoryOpenFlags}; use scopetime::scope_time; use std::path::Path; @@ -17,26 +16,25 @@ pub fn is_repo(repo_path: &str) -> bool { } /// -pub fn repo(repo_path: &str) -> Repository { +pub fn repo(repo_path: &str) -> Result { let repo = Repository::open_ext( repo_path, RepositoryOpenFlags::empty(), Vec::<&Path>::new(), - ) - .unwrap(); + )?; if repo.is_bare() { panic!("bare repo") } - repo + Ok(repo) } /// this does not run any git hooks -pub fn commit(repo_path: &str, msg: &str) -> Result { +pub fn commit(repo_path: &str, msg: &str) -> Result { scope_time!("commit"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; let signature = repo.signature()?; let mut index = repo.index()?; @@ -44,7 +42,14 @@ pub fn commit(repo_path: &str, msg: &str) -> Result { let tree = repo.find_tree(tree_id)?; let parents = if let Ok(reference) = repo.head() { - let parent = repo.find_commit(reference.target().unwrap())?; + let parent = repo.find_commit( + reference.target().ok_or_else(|| { + Error::Generic( + "failed to get the target for reference" + .to_string(), + ) + })?, + )?; vec![parent] } else { Vec::new() @@ -52,65 +57,68 @@ pub fn commit(repo_path: &str, msg: &str) -> Result { let parents = parents.iter().collect::>(); - repo.commit( + Ok(repo.commit( Some("HEAD"), &signature, &signature, msg, &tree, parents.as_slice(), - ) + )?) } /// add a file diff from workingdir to stage (will not add removed files see `stage_addremoved`) -pub fn stage_add_file(repo_path: &str, path: &Path) -> bool { +pub fn stage_add_file(repo_path: &str, path: &Path) -> Result { scope_time!("stage_add_file"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; - let mut index = repo.index().unwrap(); + let mut index = repo.index()?; if index.add_path(path).is_ok() { - index.write().unwrap(); - return true; + index.write()?; + return Ok(true); } - false + Ok(false) } /// like `stage_add_file` but uses a pattern to match/glob multiple files/folders -pub fn stage_add_all(repo_path: &str, pattern: &str) -> bool { +pub fn stage_add_all(repo_path: &str, pattern: &str) -> Result { scope_time!("stage_add_all"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; - let mut index = repo.index().unwrap(); + let mut index = repo.index()?; if index .add_all(vec![pattern], IndexAddOption::DEFAULT, None) .is_ok() { - index.write().unwrap(); - return true; + index.write()?; + return Ok(true); } - false + Ok(false) } /// stage a removed file -pub fn stage_addremoved(repo_path: &str, path: &Path) -> bool { +pub fn stage_addremoved( + repo_path: &str, + path: &Path, +) -> Result { scope_time!("stage_addremoved"); - let repo = repo(repo_path); + let repo = repo(repo_path)?; - let mut index = repo.index().unwrap(); + let mut index = repo.index()?; if index.remove_path(path).is_ok() { - index.write().unwrap(); - return true; + index.write()?; + return Ok(true); } - false + Ok(false) } #[cfg(test)] @@ -122,14 +130,14 @@ mod tests { }; use std::{ fs::{self, remove_file, File}, - io::{Error, Write}, + io::Write, path::Path, }; #[test] fn test_commit() { let file_path = Path::new("foo"); - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -140,7 +148,10 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - assert_eq!(stage_add_file(repo_path, file_path), true); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + true + ); assert_eq!(get_statuses(repo_path), (0, 1)); @@ -152,7 +163,7 @@ mod tests { #[test] fn test_commit_in_empty_repo() { let file_path = Path::new("foo"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -165,7 +176,10 @@ mod tests { assert_eq!(get_statuses(repo_path), (1, 0)); - assert_eq!(stage_add_file(repo_path, file_path), true); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + true + ); assert_eq!(get_statuses(repo_path), (0, 1)); @@ -177,17 +191,20 @@ mod tests { #[test] fn test_stage_add_smoke() { let file_path = Path::new("foo"); - let (_td, repo) = repo_init_empty(); + let (_td, repo) = repo_init_empty().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); - assert_eq!(stage_add_file(repo_path, file_path), false); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + false + ); } #[test] fn test_staging_one_file() { let file_path = Path::new("file1.txt"); - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); @@ -203,19 +220,22 @@ mod tests { assert_eq!(get_statuses(repo_path), (2, 0)); - assert_eq!(stage_add_file(repo_path, file_path), true); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + true + ); assert_eq!(get_statuses(repo_path), (1, 1)); } #[test] - fn test_staging_folder() -> Result<(), Error> { - let (_td, repo) = repo_init(); + fn test_staging_folder() -> Result<()> { + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); let status_count = |s: StatusType| -> usize { - get_status(repo_path, s).len() + get_status(repo_path, s).unwrap().len() }; fs::create_dir_all(&root.join("a/d"))?; @@ -228,7 +248,7 @@ mod tests { assert_eq!(status_count(StatusType::WorkingDir), 3); - assert_eq!(stage_add_all(repo_path, "a/d"), true); + assert_eq!(stage_add_all(repo_path, "a/d").unwrap(), true); assert_eq!(status_count(StatusType::WorkingDir), 1); assert_eq!(status_count(StatusType::Stage), 2); @@ -239,12 +259,12 @@ mod tests { #[test] fn test_staging_deleted_file() { let file_path = Path::new("file1.txt"); - let (_td, repo) = repo_init(); + let (_td, repo) = repo_init().unwrap(); let root = repo.path().parent().unwrap(); let repo_path = root.as_os_str().to_str().unwrap(); let status_count = |s: StatusType| -> usize { - get_status(repo_path, s).len() + get_status(repo_path, s).unwrap().len() }; let full_path = &root.join(file_path); @@ -254,7 +274,10 @@ mod tests { .write_all(b"test file1 content") .unwrap(); - assert_eq!(stage_add_file(repo_path, file_path), true); + assert_eq!( + stage_add_file(repo_path, file_path).unwrap(), + true + ); commit(repo_path, "commit msg").unwrap(); @@ -264,7 +287,10 @@ mod tests { // deleted file in diff now assert_eq!(status_count(StatusType::WorkingDir), 1); - assert_eq!(stage_addremoved(repo_path, file_path), true); + assert_eq!( + stage_addremoved(repo_path, file_path).unwrap(), + true + ); assert_eq!(status_count(StatusType::WorkingDir), 0); assert_eq!(status_count(StatusType::Stage), 1); diff --git a/src/app.rs b/src/app.rs index 2367beb1..28a30d5f 100644 --- a/src/app.rs +++ b/src/app.rs @@ -220,13 +220,17 @@ impl App { if sync::reset_workdir_folder( CWD, reset_item.path.as_str(), - ) { + ) + .is_ok() + { flags.insert(NeedsUpdate::ALL); } } else if sync::reset_workdir_file( CWD, reset_item.path.as_str(), - ) { + ) + .is_ok() + { flags.insert(NeedsUpdate::ALL); } } @@ -239,10 +243,14 @@ impl App { self.status_tab.selected_path() { if is_stage { - if sync::unstage_hunk(CWD, path, hash) { + if sync::unstage_hunk(CWD, path, hash) + .unwrap() + { flags.insert(NeedsUpdate::ALL); } - } else if sync::stage_hunk(CWD, path, hash) { + } else if sync::stage_hunk(CWD, path, hash) + .is_ok() + { flags.insert(NeedsUpdate::ALL); } } diff --git a/src/components/changes.rs b/src/components/changes.rs index 6b79eaeb..a692ddd4 100644 --- a/src/components/changes.rs +++ b/src/components/changes.rs @@ -109,8 +109,10 @@ impl ChangesComponent { return match status { StatusItemType::Deleted => { sync::stage_addremoved(CWD, path) + .unwrap() } - _ => sync::stage_add_file(CWD, path), + _ => sync::stage_add_file(CWD, path) + .unwrap(), }; } } else { @@ -118,12 +120,14 @@ impl ChangesComponent { return sync::stage_add_all( CWD, tree_item.info.full_path.as_str(), - ); + ) + .unwrap(); } } else { let path = Path::new(tree_item.info.full_path.as_str()); - return sync::reset_stage(CWD, path); + sync::reset_stage(CWD, path).unwrap(); + return true; } } diff --git a/src/components/commit.rs b/src/components/commit.rs index 86f5205d..637fa3df 100644 --- a/src/components/commit.rs +++ b/src/components/commit.rs @@ -122,7 +122,7 @@ impl CommitComponent { fn commit(&mut self) { if let HookResult::NotOk(e) = - sync::hooks_commit_msg(CWD, &mut self.msg) + sync::hooks_commit_msg(CWD, &mut self.msg).unwrap() { error!("commit-msg hook error: {}", e); self.queue.borrow_mut().push_back( @@ -135,7 +135,9 @@ impl CommitComponent { } sync::commit(CWD, &self.msg).unwrap(); - if let HookResult::NotOk(e) = sync::hooks_post_commit(CWD) { + if let HookResult::NotOk(e) = + sync::hooks_post_commit(CWD).unwrap() + { error!("post-commit hook error: {}", e); self.queue.borrow_mut().push_back( InternalEvent::ShowMsg(format!( diff --git a/src/tabs/revlog/mod.rs b/src/tabs/revlog/mod.rs index e7ce0134..37b78e56 100644 --- a/src/tabs/revlog/mod.rs +++ b/src/tabs/revlog/mod.rs @@ -80,7 +80,8 @@ impl Revlog { /// pub fn update(&mut self) { - self.selection_max = self.git_log.count().saturating_sub(1); + self.selection_max = + self.git_log.count().unwrap().saturating_sub(1); if self.items.needs_data(self.selection, self.selection_max) { self.fetch_commits(); @@ -96,7 +97,7 @@ impl Revlog { let commits = sync::get_commits_info( CWD, - &self.git_log.get_slice(want_min, SLICE_SIZE), + &self.git_log.get_slice(want_min, SLICE_SIZE).unwrap(), ); if let Ok(commits) = commits { @@ -343,7 +344,7 @@ impl Component for Revlog { if !self.first_open_done { self.first_open_done = true; - self.git_log.fetch(); + self.git_log.fetch().unwrap(); } } } diff --git a/src/tabs/status.rs b/src/tabs/status.rs index 97bf3bb1..03b9c86b 100644 --- a/src/tabs/status.rs +++ b/src/tabs/status.rs @@ -183,8 +183,8 @@ impl Status { /// pub fn update(&mut self) { - self.git_diff.refresh(); - self.git_status.fetch(current_tick()); + self.git_diff.refresh().unwrap(); + self.git_status.fetch(current_tick()).unwrap(); } /// @@ -202,7 +202,7 @@ impl Status { } fn update_status(&mut self) { - let status = self.git_status.last(); + let status = self.git_status.last().unwrap(); self.index.update(&status.stage); self.index_wd.update(&status.work_dir); @@ -217,14 +217,17 @@ impl Status { if self.diff.current() == (path.clone(), is_stage) { // we are already showing a diff of the right file // maybe the diff changed (outside file change) - if let Some((params, last)) = self.git_diff.last() { + if let Some((params, last)) = + self.git_diff.last().unwrap() + { if params == diff_params { self.diff.update(path, is_stage, last); } } } else { // we dont show the right diff right now, so we need to request - if let Some(diff) = self.git_diff.request(diff_params) + if let Some(diff) = + self.git_diff.request(diff_params).unwrap() { self.diff.update(path, is_stage, diff); } else {