cli: Allow checking out a specific revision

Allow `rad patch checkout` to specify a revision to checkout that may
not be the patch's latest.

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::<PatchId>(&repository.backend)?;
             delete::run(&patch_id, &profile, &repository)?;
         }
-        Operation::Checkout { patch_id } => {
+        Operation::Checkout { revision_id } => {
-            let patch_id = patch_id.resolve(&repository.backend)?;
+            let revision_id = revision_id.resolve::<radicle::git::Oid>(&repository.backend)?;
-            checkout::run(&patch_id, &repository, &workdir)?;
+            checkout::run(&patch::RevisionId::from(revision_id), &repository, &workdir)?;
         }
         Operation::Comment {
             revision_id,

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)?
+    let (patch_id, patch, _, revision) = patches
-        .ok_or_else(|| anyhow!("Patch `{patch_id}` not found"))?;
+        .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());
+        git::refname!("patch").join(RefString::try_from(term::format::cob(&patch_id)).unwrap());
-    let commit = find_patch_commit(&patch, stored, working)?;
+    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<git::raw::Commit<'a>> {
-    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()),
     }

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::<cob::EntryId>(&repo.backend)?;
-    let Some((patch_id, patch, revision_id, revision)) = patches.find_by_revision(revision_id)? else {
+    let (patch_id, patch, revision_id, revision) = patches
-        anyhow::bail!("patch revision `{revision_id}` not found");
+        .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)?;

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 revision_id = revision_id.resolve::<Oid>(&repository.backend)?;
-    let Some((patch_id, _, revision_id, _)) = patches.find_by_revision(revision_id)? else {
+    let (patch_id, _, revision_id, _) = patches
-        anyhow::bail!("patch revision `{revision_id}` not found");
+        .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");
     };

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::<git::Oid>(&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)))

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<Option<(PatchId, Patch, RevisionId, Revision)>, Error> {
         // Revision may be the patch's first, making it have the same ID.
-        let p_id = ObjectId::from(id);
+        let p_id = ObjectId::from(revision.into_inner());
-        let revision = RevisionId(id);
         if let Some(p) = self.get(&p_id)? {
             return Ok(p
-                .revision(&revision)
+                .revision(revision)
-                .map(|r| (p_id, p.clone(), revision, r.clone())));
+                .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)
+                p.revision(revision)
-                    .map(|r| (p_id, p.clone(), revision, r.clone()))
+                    .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
+        let (id, _, _, _) = patches.find_by_revision(&rev_id).unwrap().unwrap();
-            .find_by_revision(rev_id.into_inner())
-            .unwrap()
-            .unwrap();
         assert_eq!(id, patch_id);
     }