From 8181f57ec3428195ac822590257cb3c135e25b5d Mon Sep 17 00:00:00 2001 From: nsfisis Date: Wed, 3 Jun 2026 23:42:06 +0900 Subject: feat(downloader): return Result from getUnpushedChanges to bubble RuntimeException Match PHP GitDownloader::getUnpushedChanges, which returns ?string while throwing \RuntimeException on git command failure. Change the signature to Result> and replace the three panic\!() sites with Err(RuntimeException), mirroring the get_local_changes port. Thread ? through the clean_changes and StatusCommand call sites. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/shirabe/src/command/status_command.rs | 2 +- .../src/downloader/dvcs_downloader_interface.rs | 9 +++- crates/shirabe/src/downloader/git_downloader.rs | 50 +++++++++++----------- 3 files changed, 33 insertions(+), 28 deletions(-) diff --git a/crates/shirabe/src/command/status_command.rs b/crates/shirabe/src/command/status_command.rs index dba4cc8..01c1b5e 100644 --- a/crates/shirabe/src/command/status_command.rs +++ b/crates/shirabe/src/command/status_command.rs @@ -199,7 +199,7 @@ impl StatusCommand { if let Some(dvcs_downloader) = downloader.as_dvcs_downloader_interface() { if let Some(unpushed) = - dvcs_downloader.get_unpushed_changes(package.clone(), target_dir.clone()) + dvcs_downloader.get_unpushed_changes(package.clone(), target_dir.clone())? { unpushed_changes.insert(target_dir, unpushed); } diff --git a/crates/shirabe/src/downloader/dvcs_downloader_interface.rs b/crates/shirabe/src/downloader/dvcs_downloader_interface.rs index 4c8ebd3..59f5774 100644 --- a/crates/shirabe/src/downloader/dvcs_downloader_interface.rs +++ b/crates/shirabe/src/downloader/dvcs_downloader_interface.rs @@ -1,8 +1,13 @@ //! ref: composer/src/Composer/Downloader/DvcsDownloaderInterface.php +use anyhow::Result; + use crate::package::PackageInterfaceHandle; pub trait DvcsDownloaderInterface { - fn get_unpushed_changes(&self, package: PackageInterfaceHandle, path: String) - -> Option; + fn get_unpushed_changes( + &self, + package: PackageInterfaceHandle, + path: String, + ) -> Result>; } diff --git a/crates/shirabe/src/downloader/git_downloader.rs b/crates/shirabe/src/downloader/git_downloader.rs index acf935b..05ef096 100644 --- a/crates/shirabe/src/downloader/git_downloader.rs +++ b/crates/shirabe/src/downloader/git_downloader.rs @@ -461,11 +461,11 @@ impl GitDownloader { &self, _package: PackageInterfaceHandle, path: &str, - ) -> Option { + ) -> Result> { GitUtil::clean_env(&self.inner.process); let path = self.normalize_path(path); if !self.has_metadata_repository(&path) { - return None; + return Ok(None); } let command = vec![ @@ -482,15 +482,15 @@ impl GitDownloader { .execute_args(&command, &mut output, Some(path.clone())) != 0 { - // TODO(phase-b): bubble error via Result later - panic!( - "{}", - format!( + return Err(RuntimeException { + message: format!( "Failed to execute {}\n\n{}", implode(" ", &command), self.inner.process.borrow().get_error_output(), - ) - ); + ), + code: 0, + } + .into()); } let mut refs = trim(&output, None); @@ -499,7 +499,7 @@ impl GitDownloader { .unwrap_or(false) { // could not match the HEAD for some reason - return None; + return Ok(None); } let head_ref = head_match .get(&CaptureKey::ByIndex(1)) @@ -515,7 +515,7 @@ impl GitDownloader { .unwrap_or(false) { // not on a branch, we are either on a not-modified tag or some sort of detached head, so skip this - return None; + return Ok(None); } let candidate_branches: Vec = branches_match .get(&CaptureKey::ByIndex(1)) @@ -584,15 +584,15 @@ impl GitDownloader { Some(path.clone()), ) != 0 { - // TODO(phase-b): bubble error via Result later - panic!( - "{}", - format!( + return Err(RuntimeException { + message: format!( "Failed to execute {}\n\n{}", implode(" ", &command), self.inner.process.borrow().get_error_output(), - ) - ); + ), + code: 0, + } + .into()); } let output = trim(&output, None); @@ -629,15 +629,15 @@ impl GitDownloader { Some(path.clone()), ) != 0 { - // TODO(phase-b): bubble error via Result later - panic!( - "{}", - format!( + return Err(RuntimeException { + message: format!( "Failed to execute {}\n\n{}", implode(" ", &command), self.inner.process.borrow().get_error_output(), - ) - ); + ), + code: 0, + } + .into()); } refs = trim(&output, None); } @@ -648,7 +648,7 @@ impl GitDownloader { } } - unpushed_changes + Ok(unpushed_changes) } pub(crate) async fn clean_changes( @@ -660,7 +660,7 @@ impl GitDownloader { GitUtil::clean_env(&self.inner.process); let path = self.normalize_path(path); - let unpushed = self.get_unpushed_changes(package.clone(), &path); + let unpushed = self.get_unpushed_changes(package.clone(), &path)?; if let Some(unpushed) = unpushed.as_deref() { if self.inner.io.is_interactive() || self @@ -1325,7 +1325,7 @@ impl DvcsDownloaderInterface for GitDownloader { &self, package: PackageInterfaceHandle, path: String, - ) -> Option { + ) -> Result> { GitDownloader::get_unpushed_changes(self, package, &path) } } -- cgit v1.3.1