better error handling in comment and git

This replaces a bunch of `unwrap()` calls with error returns.
This commit is contained in:
Sebastian Kuzminsky 2025-07-19 10:38:15 -06:00
parent 97a575316e
commit c217434071
2 changed files with 96 additions and 60 deletions

View file

@ -48,19 +48,23 @@ impl Comment {
} }
} }
} }
if description == None { let Some(description) = description else {
return Err(CommentError::CommentParseError); return Err(CommentError::CommentParseError);
} };
let author = crate::git::git_log_oldest_author(comment_dir)?; let author = crate::git::git_log_oldest_author(comment_dir)?;
let creation_time = crate::git::git_log_oldest_timestamp(comment_dir)?; let creation_time = crate::git::git_log_oldest_timestamp(comment_dir)?;
let dir = std::path::PathBuf::from(comment_dir); let dir = std::path::PathBuf::from(comment_dir);
Ok(Self { Ok(Self {
uuid: String::from(dir.file_name().unwrap().to_string_lossy()), uuid: String::from(
dir.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy(),
),
author, author,
creation_time, creation_time,
description: description.unwrap(), description,
dir: std::path::PathBuf::from(comment_dir), dir: std::path::PathBuf::from(comment_dir),
}) })
} }
@ -109,7 +113,11 @@ impl Comment {
&format!( &format!(
"add comment {} on issue {}", "add comment {} on issue {}",
comment.uuid, comment.uuid,
issue.dir.file_name().unwrap().to_string_lossy(), issue
.dir
.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy(),
), ),
)?; )?;
} }
@ -130,10 +138,15 @@ impl Comment {
crate::git::add(&description_filename)?; crate::git::add(&description_filename)?;
if crate::git::worktree_is_dirty(&self.dir.to_string_lossy())? { if crate::git::worktree_is_dirty(&self.dir.to_string_lossy())? {
crate::git::commit( crate::git::commit(
&description_filename.parent().unwrap(), &description_filename
.parent()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?,
&format!( &format!(
"edit comment {} on issue FIXME", // FIXME: name the issue that the comment is on "edit comment {} on issue FIXME", // FIXME: name the issue that the comment is on
self.dir.file_name().unwrap().to_string_lossy() self.dir
.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy()
), ),
)?; )?;
self.read_description()?; self.read_description()?;
@ -165,8 +178,8 @@ impl Comment {
.spawn()? .spawn()?
.wait_with_output()?; .wait_with_output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(CommentError::EditorError); return Err(CommentError::EditorError);
} }

View file

@ -48,8 +48,8 @@ impl Worktree {
.args(["worktree", "add", &path.path().to_string_lossy(), branch]) .args(["worktree", "add", &path.path().to_string_lossy(), branch])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(Self { path }) Ok(Self { path })
@ -67,8 +67,8 @@ impl Worktree {
]) ])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(Self { path }) Ok(Self { path })
@ -87,8 +87,8 @@ pub fn checkout_branch_in_worktree(
.args(["worktree", "add", &worktree_dir.to_string_lossy(), branch]) .args(["worktree", "add", &worktree_dir.to_string_lossy(), branch])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(()) Ok(())
@ -99,8 +99,8 @@ pub fn git_worktree_prune() -> Result<(), GitError> {
.args(["worktree", "prune"]) .args(["worktree", "prune"])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(()) Ok(())
@ -111,8 +111,8 @@ pub fn git_remove_branch(branch: &str) -> Result<(), GitError> {
.args(["branch", "-D", branch]) .args(["branch", "-D", branch])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(()) Ok(())
@ -139,11 +139,14 @@ pub fn worktree_is_dirty(dir: &str) -> Result<bool, GitError> {
pub fn add(file: &std::path::Path) -> Result<(), GitError> { pub fn add(file: &std::path::Path) -> Result<(), GitError> {
let result = std::process::Command::new("git") let result = std::process::Command::new("git")
.args(["add", &file.to_string_lossy()]) .args(["add", &file.to_string_lossy()])
.current_dir(file.parent().unwrap()) .current_dir(
file.parent()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?,
)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
return Ok(()); return Ok(());
@ -152,11 +155,14 @@ pub fn add(file: &std::path::Path) -> Result<(), GitError> {
pub fn restore_file(file: &std::path::Path) -> Result<(), GitError> { pub fn restore_file(file: &std::path::Path) -> Result<(), GitError> {
let result = std::process::Command::new("git") let result = std::process::Command::new("git")
.args(["restore", &file.to_string_lossy()]) .args(["restore", &file.to_string_lossy()])
.current_dir(file.parent().unwrap()) .current_dir(
file.parent()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?,
)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
return Ok(()); return Ok(());
@ -168,8 +174,8 @@ pub fn commit(dir: &std::path::Path, msg: &str) -> Result<(), GitError> {
.current_dir(dir) .current_dir(dir)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(()) Ok(())
@ -180,12 +186,18 @@ pub fn git_commit_file(file: &std::path::Path) -> Result<(), GitError> {
git_dir.pop(); git_dir.pop();
let result = std::process::Command::new("git") let result = std::process::Command::new("git")
.args(["add", &file.file_name().unwrap().to_string_lossy()]) .args([
"add",
&file
.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy(),
])
.current_dir(&git_dir) .current_dir(&git_dir)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -195,15 +207,20 @@ pub fn git_commit_file(file: &std::path::Path) -> Result<(), GitError> {
"-m", "-m",
&format!( &format!(
"update '{}' in issue {}", "update '{}' in issue {}",
file.file_name().unwrap().to_string_lossy(), file.file_name()
git_dir.file_name().unwrap().to_string_lossy() .ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy(),
git_dir
.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy()
), ),
]) ])
.current_dir(&git_dir) .current_dir(&git_dir)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -216,8 +233,8 @@ pub fn git_fetch(dir: &std::path::Path, remote: &str) -> Result<(), GitError> {
.current_dir(dir) .current_dir(dir)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
Ok(()) Ok(())
@ -253,13 +270,13 @@ pub fn sync(dir: &std::path::Path, remote: &str, branch: &str) -> Result<(), Git
"Sync failed! 'git log' error! Help, a human needs to fix the mess in {:?}", "Sync failed! 'git log' error! Help, a human needs to fix the mess in {:?}",
branch branch
); );
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
if result.stdout.len() > 0 { if result.stdout.len() > 0 {
println!("Changes fetched from remote {}:", remote); println!("Changes fetched from remote {}:", remote);
println!("{}", std::str::from_utf8(&result.stdout).unwrap()); println!("{}", &String::from_utf8_lossy(&result.stdout));
println!(""); println!("");
} }
@ -279,13 +296,13 @@ pub fn sync(dir: &std::path::Path, remote: &str, branch: &str) -> Result<(), Git
"Sync failed! 'git log' error! Help, a human needs to fix the mess in {:?}", "Sync failed! 'git log' error! Help, a human needs to fix the mess in {:?}",
branch branch
); );
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
if result.stdout.len() > 0 { if result.stdout.len() > 0 {
println!("Changes to push to remote {}:", remote); println!("Changes to push to remote {}:", remote);
println!("{}", std::str::from_utf8(&result.stdout).unwrap()); println!("{}", &String::from_utf8_lossy(&result.stdout));
println!(""); println!("");
} }
@ -299,8 +316,8 @@ pub fn sync(dir: &std::path::Path, remote: &str, branch: &str) -> Result<(), Git
"Sync failed! Merge error! Help, a human needs to fix the mess in {:?}", "Sync failed! Merge error! Help, a human needs to fix the mess in {:?}",
branch branch
); );
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -314,8 +331,8 @@ pub fn sync(dir: &std::path::Path, remote: &str, branch: &str) -> Result<(), Git
"Sync failed! Push error! Help, a human needs to fix the mess in {:?}", "Sync failed! Push error! Help, a human needs to fix the mess in {:?}",
branch branch
); );
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -332,13 +349,16 @@ pub fn git_log_oldest_timestamp(
"log", "log",
"--pretty=format:%at", "--pretty=format:%at",
"--", "--",
&path.file_name().unwrap().to_string_lossy(), &path
.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy(),
]) ])
.current_dir(&git_dir) .current_dir(&git_dir)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
let timestamp_str = std::str::from_utf8(&result.stdout).unwrap(); let timestamp_str = std::str::from_utf8(&result.stdout).unwrap();
@ -358,13 +378,16 @@ pub fn git_log_oldest_author(path: &std::path::Path) -> Result<String, GitError>
"log", "log",
"--pretty=format:%an <%ae>", "--pretty=format:%an <%ae>",
"--", "--",
&path.file_name().unwrap().to_string_lossy(), &path
.file_name()
.ok_or(std::io::Error::from(std::io::ErrorKind::NotFound))?
.to_string_lossy(),
]) ])
.current_dir(&git_dir) .current_dir(&git_dir)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
let author_str = std::str::from_utf8(&result.stdout).unwrap(); let author_str = std::str::from_utf8(&result.stdout).unwrap();
@ -383,8 +406,8 @@ pub fn create_orphan_branch(branch: &str) -> Result<(), GitError> {
.args(["worktree", "prune"]) .args(["worktree", "prune"])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -400,8 +423,8 @@ fn create_orphan_branch_at_path(
.args(["worktree", "add", "--orphan", "-b", branch, &worktree_dir]) .args(["worktree", "add", "--orphan", "-b", branch, &worktree_dir])
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -418,8 +441,8 @@ fn create_orphan_branch_at_path(
.current_dir(worktree_path) .current_dir(worktree_path)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }
@ -428,8 +451,8 @@ fn create_orphan_branch_at_path(
.current_dir(worktree_path) .current_dir(worktree_path)
.output()?; .output()?;
if !result.status.success() { if !result.status.success() {
println!("stdout: {}", std::str::from_utf8(&result.stdout).unwrap()); println!("stdout: {}", &String::from_utf8_lossy(&result.stdout));
println!("stderr: {}", std::str::from_utf8(&result.stderr).unwrap()); println!("stderr: {}", &String::from_utf8_lossy(&result.stderr));
return Err(GitError::Oops); return Err(GitError::Oops);
} }