diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index e12e6ad6..349ee41d 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -150,7 +150,7 @@ pub enum Operation { patch_id: Rev, }, Checkout { - patch_id: Rev, + revision_id: Rev, }, Comment { revision_id: Rev, @@ -339,7 +339,7 @@ impl Args for Options { patch_id: patch_id.ok_or_else(|| anyhow!("a patch id must be provided"))?, }, OperationName::Checkout => Operation::Checkout { - patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, + revision_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, }, OperationName::Comment => Operation::Comment { revision_id: patch_id @@ -419,9 +419,9 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { let patch_id = patch_id.resolve::(&repository.backend)?; delete::run(&patch_id, &profile, &repository)?; } - Operation::Checkout { patch_id } => { - let patch_id = patch_id.resolve(&repository.backend)?; - checkout::run(&patch_id, &repository, &workdir)?; + Operation::Checkout { revision_id } => { + let revision_id = revision_id.resolve::(&repository.backend)?; + checkout::run(&patch::RevisionId::from(revision_id), &repository, &workdir)?; } Operation::Comment { revision_id, diff --git a/radicle-cli/src/commands/patch/checkout.rs b/radicle-cli/src/commands/patch/checkout.rs index 0b261f88..77454081 100644 --- a/radicle-cli/src/commands/patch/checkout.rs +++ b/radicle-cli/src/commands/patch/checkout.rs @@ -1,7 +1,7 @@ use anyhow::anyhow; use radicle::cob::patch; -use radicle::cob::patch::{Patch, PatchId}; +use radicle::cob::patch::RevisionId; use radicle::git::RefString; use radicle::storage::git::Repository; use radicle::storage::ReadRepository; @@ -10,20 +10,31 @@ use radicle::{git, rad}; use crate::terminal as term; pub fn run( - patch_id: &PatchId, + revision_id: &RevisionId, stored: &Repository, working: &git::raw::Repository, ) -> anyhow::Result<()> { let patches = patch::Patches::open(stored)?; - let patch = patches - .get(patch_id)? - .ok_or_else(|| anyhow!("Patch `{patch_id}` not found"))?; + + let (patch_id, patch, _, revision) = patches + .find_by_revision(revision_id)? + .ok_or_else(|| anyhow!("Patch revision `{revision_id}` not found"))?; + let (root, _) = patch.root(); + // If we passed in the root revision, it's more likely that the user was specifying the + // patch itself. Hence, we checkout the latest update on the patch instead of that specific + // revision. + let revision = if *revision_id == root { + let (_, revision) = patch.latest(); + revision + } else { + &revision + }; let mut spinner = term::spinner("Performing checkout..."); let patch_branch = // SAFETY: Patch IDs are valid refstrings. - git::refname!("patch").join(RefString::try_from(term::format::cob(patch_id)).unwrap()); - let commit = find_patch_commit(&patch, stored, working)?; + git::refname!("patch").join(RefString::try_from(term::format::cob(&patch_id)).unwrap()); + let commit = find_patch_commit(revision, stored, working)?; // Create patch branch and switch to it. working.branch(patch_branch.as_str(), &commit, true)?; @@ -36,7 +47,7 @@ pub fn run( )); spinner.finish(); - if let Some(branch) = rad::setup_patch_upstream(patch_id, *patch.head(), working, false)? { + if let Some(branch) = rad::setup_patch_upstream(&patch_id, revision.head(), working, false)? { let tracking = branch .name()? .ok_or_else(|| anyhow!("failed to create tracking branch: invalid name"))?; @@ -52,23 +63,23 @@ pub fn run( /// Try to find the patch head in our working copy, and if we don't find it, /// fetch it from storage first. fn find_patch_commit<'a>( - patch: &Patch, + revision: &patch::Revision, stored: &Repository, working: &'a git::raw::Repository, ) -> anyhow::Result> { - let patch_head = *patch.head(); + let head = *revision.head(); - match working.find_commit(patch_head.into()) { + match working.find_commit(head) { Ok(commit) => Ok(commit), Err(e) if git::ext::is_not_found_err(&e) => { let url = git::url::File::new(stored.path()); working.remote_anonymous(url.to_string().as_str())?.fetch( - &[patch_head.to_string()], + &[head.to_string()], None, None, )?; - working.find_commit(patch_head.into()).map_err(|e| e.into()) + working.find_commit(head).map_err(|e| e.into()) } Err(e) => Err(e.into()), } diff --git a/radicle-cli/src/commands/patch/comment.rs b/radicle-cli/src/commands/patch/comment.rs index fa5350cd..688bff32 100644 --- a/radicle-cli/src/commands/patch/comment.rs +++ b/radicle-cli/src/commands/patch/comment.rs @@ -22,9 +22,9 @@ pub fn run( let mut patches = patch::Patches::open(repo)?; let revision_id = revision_id.resolve::(&repo.backend)?; - let Some((patch_id, patch, revision_id, revision)) = patches.find_by_revision(revision_id)? else { - anyhow::bail!("patch revision `{revision_id}` not found"); - }; + let (patch_id, patch, revision_id, revision) = patches + .find_by_revision(&patch::RevisionId::from(revision_id))? + .ok_or_else(|| anyhow!("Patch revision `{revision_id}` not found"))?; let mut patch = patch::PatchMut::new(patch_id, patch, &mut patches); let (body, reply_to) = prompt(message, reply_to, &revision, repo)?; let comment_id = patch.comment(revision_id, body, reply_to, &signer)?; diff --git a/radicle-cli/src/commands/patch/redact.rs b/radicle-cli/src/commands/patch/redact.rs index 9107cb59..f493eb6e 100644 --- a/radicle-cli/src/commands/patch/redact.rs +++ b/radicle-cli/src/commands/patch/redact.rs @@ -1,4 +1,5 @@ use radicle::cob::patch; +use radicle::git::Oid; use radicle::prelude::*; use radicle::storage::git::Repository; @@ -14,10 +15,10 @@ pub fn run( let signer = &term::signer(profile)?; let mut patches = patch::Patches::open(repository)?; - let revision_id = revision_id.resolve(&repository.backend)?; - let Some((patch_id, _, revision_id, _)) = patches.find_by_revision(revision_id)? else { - anyhow::bail!("patch revision `{revision_id}` not found"); - }; + let revision_id = revision_id.resolve::(&repository.backend)?; + let (patch_id, _, revision_id, _) = patches + .find_by_revision(&patch::RevisionId::from(revision_id))? + .ok_or_else(|| anyhow!("Patch revision `{revision_id}` not found"))?; let Ok(mut patch) = patches.get_mut(&patch_id) else { anyhow::bail!("Patch `{patch_id}` not found"); }; diff --git a/radicle-cli/src/commands/review.rs b/radicle-cli/src/commands/review.rs index a73fd230..0f03ee3f 100644 --- a/radicle-cli/src/commands/review.rs +++ b/radicle-cli/src/commands/review.rs @@ -5,7 +5,7 @@ use std::ffi::OsString; use anyhow::{anyhow, Context}; -use radicle::cob::patch::{PatchId, Patches, Verdict}; +use radicle::cob::patch::{PatchId, Patches, RevisionId, Verdict}; use radicle::prelude::*; use radicle::{git, rad}; @@ -209,9 +209,9 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { let mut patches = Patches::open(&repository)?; let (patch_id, revision) = if options.revision { - let id = options.id.resolve(&repository.backend)?; + let id = options.id.resolve::(&repository.backend)?; let (patch_id, _, rev_id, rev) = patches - .find_by_revision(id)? + .find_by_revision(&RevisionId::from(id))? .ok_or_else(|| anyhow!("revision {} does not exist", id))?; (patch_id, Some((rev_id, rev))) diff --git a/radicle/src/cob/patch.rs b/radicle/src/cob/patch.rs index 7526275f..7c93db35 100644 --- a/radicle/src/cob/patch.rs +++ b/radicle/src/cob/patch.rs @@ -2082,24 +2082,23 @@ where /// Find the `Patch` containing the given `Revision`. pub fn find_by_revision( &self, - id: EntryId, + revision: &RevisionId, ) -> Result, Error> { // Revision may be the patch's first, making it have the same ID. - let p_id = ObjectId::from(id); - let revision = RevisionId(id); + let p_id = ObjectId::from(revision.into_inner()); if let Some(p) = self.get(&p_id)? { return Ok(p - .revision(&revision) - .map(|r| (p_id, p.clone(), revision, r.clone()))); + .revision(revision) + .map(|r| (p_id, p.clone(), *revision, r.clone()))); } - let result = self .all()? .filter_map(|result| result.ok()) .find_map(|(p_id, p)| { - p.revision(&revision) - .map(|r| (p_id, p.clone(), revision, r.clone())) + p.revision(revision) + .map(|r| (p_id, p.clone(), *revision, r.clone())) }); + Ok(result) } @@ -2284,10 +2283,7 @@ mod test { assert_eq!(revision.oid, branch.oid); assert_eq!(revision.base, branch.base); - let (id, _, _, _) = patches - .find_by_revision(rev_id.into_inner()) - .unwrap() - .unwrap(); + let (id, _, _, _) = patches.find_by_revision(&rev_id).unwrap().unwrap(); assert_eq!(id, patch_id); }