From 6510705521d3bf8d30929ee5f69f3b2a5fff8a9a Mon Sep 17 00:00:00 2001 From: Fintan Halpenny Date: Thu, 16 Nov 2023 17:51:10 +0000 Subject: [PATCH] cli: add revision option to rad patch checkout Add the `--revision` option for `rad patch checkout`. This allows the user to specify a particular revision of the patch to checkout. If the revision is not specified it will use the latest revision of the patch. Signed-off-by: Fintan Halpenny X-Clacks-Overhead: GNU Terry Pratchett --- .../examples/rad-patch-checkout-revision.md | 51 +++++++++++++++++++ radicle-cli/src/commands/patch.rs | 29 +++++++++-- radicle-cli/src/commands/patch/checkout.rs | 35 ++++++------- radicle-cli/tests/commands.rs | 27 ++++++++++ 4 files changed, 118 insertions(+), 24 deletions(-) create mode 100644 radicle-cli/examples/rad-patch-checkout-revision.md diff --git a/radicle-cli/examples/rad-patch-checkout-revision.md b/radicle-cli/examples/rad-patch-checkout-revision.md new file mode 100644 index 00000000..583b948e --- /dev/null +++ b/radicle-cli/examples/rad-patch-checkout-revision.md @@ -0,0 +1,51 @@ +We may want to checkout a particular revision of a patch. +So first, let's add another change to the patch and a `LICENSE` file. + +``` +$ touch LICENSE +$ git add LICENSE +$ git commit --message "Add LICENSE, just for the business" +[patch/6ff4f09 639f44a] Add LICENSE, just for the business + 1 file changed, 0 insertions(+), 0 deletions(-) + create mode 100644 LICENSE +$ git push rad -o patch.message="Add LICENSE, just for the business" +``` + +We can see the list of revisions of the patch by `show`ing it: + +``` +$ rad patch show 6ff4f09 +╭─────────────────────────────────────────────────────────────────────╮ +│ Title Define power requirements │ +│ Patch 6ff4f09c1b5a81347981f59b02ef43a31a07cdae │ +│ Author z6MknSL…StBU8Vi (you) │ +│ Head 639f44a25145a37f747f3c84265037a9461e44c5 │ +│ Branches patch/6ff4f09 │ +│ Commits ahead 3, behind 0 │ +│ Status open │ +│ │ +│ See details. │ +├─────────────────────────────────────────────────────────────────────┤ +│ 639f44a Add LICENSE, just for the business │ +│ 27857ec Add README, just for the fun │ +│ 3e674d1 Define power requirements │ +├─────────────────────────────────────────────────────────────────────┤ +│ ● opened by z6MknSL…StBU8Vi (you) now │ +│ ↑ updated to 0c0942e2ff2488617d950ede15567ca39a29972e (27857ec) now │ +│ ↑ updated to 2bf48de79e371014f084b5501ecc9c9c4182e7fc (639f44a) now │ +╰─────────────────────────────────────────────────────────────────────╯ +``` + +So, let's checkout the previous revision, `0c0942e2`: + +``` +$ rad patch checkout 6ff4f09 --revision 0c0942e2 -f +✓ Switched to branch patch/6ff4f09 +``` + +And we can confirm that the current commit corresponds to `27857ec`: + +``` +$ git rev-parse HEAD +27857ec9eb04c69cacab516e8bf4b5fd36090f66 +``` diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index 43037df4..c0a34fec 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -87,6 +87,7 @@ Ready options Checkout options + --revision Checkout the given revision of the patch --name Provide a name for the branch to checkout -f, --force Checkout the head of the revision, even if the branch already exists @@ -155,7 +156,8 @@ pub enum Operation { patch_id: Rev, }, Checkout { - revision_id: Rev, + patch_id: Rev, + revision_id: Option, opts: checkout::Options, }, Comment { @@ -259,6 +261,13 @@ impl Args for Options { } // Checkout options + Long("revision") if op == Some(OperationName::Checkout) => { + let val = parser.value()?; + let rev = term::args::oid(&val)?; + + revision_id = Some(rev); + } + Long("force") | Short('f') if op == Some(OperationName::Checkout) => { checkout_opts.force = true; } @@ -356,7 +365,8 @@ impl Args for Options { patch_id: patch_id.ok_or_else(|| anyhow!("a patch id must be provided"))?, }, OperationName::Checkout => Operation::Checkout { - revision_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, + patch_id: patch_id.ok_or_else(|| anyhow!("a patch must be provided"))?, + revision_id, opts: checkout_opts, }, OperationName::Comment => Operation::Comment { @@ -437,10 +447,19 @@ 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 { revision_id, opts } => { - let revision_id = revision_id.resolve::(&repository.backend)?; + Operation::Checkout { + patch_id, + revision_id, + opts, + } => { + let patch_id = patch_id.resolve::(&repository.backend)?; + let revision_id = revision_id + .map(|rev| rev.resolve::(&repository.backend)) + .transpose()? + .map(patch::RevisionId::from); checkout::run( - &patch::RevisionId::from(revision_id), + &patch::PatchId::from(patch_id), + revision_id, &repository, &workdir, opts, diff --git a/radicle-cli/src/commands/patch/checkout.rs b/radicle-cli/src/commands/patch/checkout.rs index 3acd146f..affa6327 100644 --- a/radicle-cli/src/commands/patch/checkout.rs +++ b/radicle-cli/src/commands/patch/checkout.rs @@ -18,39 +18,36 @@ pub struct Options { } impl Options { - fn branch(&self, id: PatchId) -> anyhow::Result { + fn branch(&self, id: &PatchId) -> anyhow::Result { match &self.name { Some(refname) => Ok(Qualified::from_refstr(refname) .map_or_else(|| refname.clone(), |q| q.to_ref_string())), // SAFETY: Patch IDs are valid refstrings. - None => { - Ok(git::refname!("patch") - .join(RefString::try_from(term::format::cob(&id)).unwrap())) - } + None => Ok( + git::refname!("patch").join(RefString::try_from(term::format::cob(id)).unwrap()) + ), } } } pub fn run( - revision_id: &RevisionId, + patch_id: &PatchId, + revision_id: Option, stored: &Repository, working: &git::raw::Repository, opts: Options, ) -> anyhow::Result<()> { let patches = patch::Patches::open(stored)?; - 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 patch = patches + .get(patch_id)? + .ok_or_else(|| anyhow!("Patch `{patch_id}` not found"))?; + + let revision = match revision_id { + Some(id) => patch + .revision(&id) + .ok_or_else(|| anyhow!("Patch revision `{id}` not found"))?, + None => patch.latest().1, }; let mut spinner = term::spinner("Performing checkout..."); @@ -89,7 +86,7 @@ pub fn run( )); spinner.finish(); - if let Some(branch) = rad::setup_patch_upstream(&patch_id, revision.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"))?; diff --git a/radicle-cli/tests/commands.rs b/radicle-cli/tests/commands.rs index 0dd76e3c..59e9e940 100644 --- a/radicle-cli/tests/commands.rs +++ b/radicle-cli/tests/commands.rs @@ -440,6 +440,33 @@ fn rad_patch_checkout() { .unwrap(); } +#[test] +fn rad_patch_checkout_revision() { + let mut environment = Environment::new(); + let profile = environment.profile("alice"); + let working = tempfile::tempdir().unwrap(); + let home = &profile.home; + + // Setup a test repository. + fixtures::repository(working.path()); + + test("examples/rad-init.md", working.path(), Some(home), []).unwrap(); + test( + "examples/rad-patch-checkout.md", + working.path(), + Some(home), + [], + ) + .unwrap(); + test( + "examples/rad-patch-checkout-revision.md", + working.path(), + Some(home), + [], + ) + .unwrap(); +} + #[test] fn rad_patch_checkout_force() { let mut environment = Environment::new();