From 3c202564bf81a3a3ce5ad19e4625a6bb17fc9f9b Mon Sep 17 00:00:00 2001 From: Slack Coder Date: Thu, 19 Jan 2023 07:57:22 -0500 Subject: [PATCH] cli: split into patch::create and patch::update Simplify codebase by splitting patch::create::run() into patch::create::run() and patch::update::run(). Patch has the subcommands `create` and `update` but both use the same method to do the heavy lifting. Spitting the code allows us to detangle the code path making it easier to follow. Also, reduce cognitive load by minimizing the amount of variables used and isolating them to utility functions. For instance, `head_branch` can be passed to these methods instead of `head_oid` and `head_branch`. Signed-off-by: Slack Coder --- radicle-cli/examples/rad-patch.md | 8 +- radicle-cli/src/commands/comment.rs | 10 +- radicle-cli/src/commands/patch.rs | 63 ++-- radicle-cli/src/commands/patch/common.rs | 117 ++++++++ radicle-cli/src/commands/patch/create.rs | 355 ++++++----------------- radicle-cli/src/commands/patch/update.rs | 148 ++++++++++ radicle-cli/src/commands/review.rs | 10 +- radicle-cli/src/terminal/patch.rs | 20 +- 8 files changed, 405 insertions(+), 326 deletions(-) create mode 100644 radicle-cli/src/commands/patch/update.rs diff --git a/radicle-cli/examples/rad-patch.md b/radicle-cli/examples/rad-patch.md index 98c68688..3492f9f3 100644 --- a/radicle-cli/examples/rad-patch.md +++ b/radicle-cli/examples/rad-patch.md @@ -89,18 +89,18 @@ Wait, lets add a README too! Just for fun. ``` $ touch README.md $ git add README.md -$ git commit --message "add README for the YOLOs" -[flux-capacitor-power 6792455] add README for the YOLOs +$ git commit --message "Add README, just for the fun" +[flux-capacitor-power 27857ec] Add README, just for the fun 1 file changed, 0 insertions(+), 0 deletions(-) create mode 100644 README.md $ rad patch update --message "Add README, just for the fun" --no-confirm 15141cf1497627e2db54362972dd9533f62d1dcb -🌱 Creating patch for heartwood +🌱 Updating patch for heartwood ok Pushing HEAD to storage... ok Analyzing remotes... -15141cf1497 R0 (3e674d1) -> R1 (6792455) +15141cf1497 R0 (3e674d1) -> R1 (27857ec) 1 commit(s) ahead, 0 commit(s) behind diff --git a/radicle-cli/src/commands/comment.rs b/radicle-cli/src/commands/comment.rs index c38d1331..cb79c220 100644 --- a/radicle-cli/src/commands/comment.rs +++ b/radicle-cli/src/commands/comment.rs @@ -12,7 +12,7 @@ use radicle::storage::WriteStorage; use crate::terminal as term; use crate::terminal::args::{Args, Error, Help}; -use crate::terminal::patch::Comment; +use crate::terminal::patch::Message; pub const HELP: Help = Help { name: "comment", @@ -33,7 +33,7 @@ Options #[derive(Debug)] pub struct Options { pub id: cob::ObjectId, - pub message: Comment, + pub message: Message, } #[inline] @@ -50,19 +50,19 @@ impl Args for Options { let mut parser = lexopt::Parser::from_args(args); let mut id: Option = None; - let mut message = Comment::default(); + let mut message = Message::default(); while let Some(arg) = parser.next()? { match arg { // Options. Long("message") | Short('m') => { - if message != Comment::Blank { + if message != Message::Blank { // We skip this code when `no-message` is specified. let txt: String = parser.value()?.to_string_lossy().into(); message.append(&txt); } } - Long("no-message") => message = Comment::Blank, + Long("no-message") => message = Message::Blank, // Common. Long("help") => return Err(Error::Help.into()), diff --git a/radicle-cli/src/commands/patch.rs b/radicle-cli/src/commands/patch.rs index 0784b390..312cb23e 100644 --- a/radicle-cli/src/commands/patch.rs +++ b/radicle-cli/src/commands/patch.rs @@ -6,6 +6,8 @@ mod create; mod list; #[path = "patch/show.rs"] mod show; +#[path = "patch/update.rs"] +mod update; use std::ffi::OsString; @@ -16,7 +18,7 @@ use radicle::prelude::*; use crate::terminal as term; use crate::terminal::args::{Args, Error, Help}; -use crate::terminal::patch::Comment; +use crate::terminal::patch::Message; pub const HELP: Help = Help { name: "patch", @@ -43,23 +45,6 @@ Options "#, }; -#[derive(Copy, Clone, Debug, Default, PartialEq, Eq)] -pub enum OptPatch { - #[default] - Any, - None, - Patch(PatchId), -} - -impl From for Option { - fn from(opt: OptPatch) -> Self { - match opt { - OptPatch::Patch(patch_id) => Some(patch_id), - _ => None, - } - } -} - #[derive(Debug, Default, PartialEq, Eq)] pub enum OperationName { Open, @@ -72,14 +57,14 @@ pub enum OperationName { #[derive(Debug)] pub enum Operation { Open { - message: Comment, + message: Message, }, Show { patch_id: PatchId, }, Update { - patch_id: OptPatch, - message: Comment, + patch_id: Option, + message: Message, }, List, } @@ -102,8 +87,8 @@ impl Args for Options { let mut op: Option = None; let mut verbose = false; let mut sync = true; - let mut patch_id = OptPatch::default(); - let mut message = Comment::default(); + let mut patch_id = None; + let mut message = Message::default(); let mut push = true; while let Some(arg) = parser.next()? { @@ -116,14 +101,14 @@ impl Args for Options { confirm = false; } Long("message") | Short('m') => { - if message != Comment::Blank { + if message != Message::Blank { // We skip this code when `no-message` is specified. let txt: String = parser.value()?.to_string_lossy().into(); message.append(&txt); } } Long("no-message") => { - message = Comment::Blank; + message = Message::Blank; } Long("sync") => { // By default it is already true, so @@ -156,11 +141,11 @@ impl Args for Options { unknown => anyhow::bail!("unknown operation '{}'", unknown), }, - Value(val) if op == Some(OperationName::Show) && patch_id == OptPatch::Any => { - patch_id = OptPatch::Patch(term::cob::parse_patch_id(val)?); + Value(val) if op == Some(OperationName::Show) && patch_id.is_none() => { + patch_id = Some(term::cob::parse_patch_id(val)?); } - Value(val) if op == Some(OperationName::Update) && patch_id == OptPatch::Any => { - patch_id = OptPatch::Patch(term::cob::parse_patch_id(val)?); + Value(val) if op == Some(OperationName::Update) && patch_id.is_none() => { + patch_id = Some(term::cob::parse_patch_id(val)?); } _ => return Err(anyhow::anyhow!(arg.unexpected())), } @@ -170,8 +155,7 @@ impl Args for Options { OperationName::Open => Operation::Open { message }, OperationName::List => Operation::List, OperationName::Show => Operation::Show { - patch_id: Option::from(patch_id) - .ok_or_else(|| anyhow!("a patch id must be provided"))?, + patch_id: patch_id.ok_or_else(|| anyhow!("a patch id must be provided"))?, }, OperationName::Update => Operation::Update { patch_id, message }, }; @@ -198,14 +182,7 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { match options.op { Operation::Open { ref message } => { - create::run( - &storage, - &profile, - &workdir, - OptPatch::None, - message.clone(), - options, - )?; + create::run(&storage, &profile, &workdir, message.clone(), options)?; } Operation::List => { list::run(&storage, &profile, Some(workdir), options)?; @@ -214,16 +191,16 @@ pub fn run(options: Options, ctx: impl term::Context) -> anyhow::Result<()> { show::run(&storage, &profile, &workdir, patch_id)?; } Operation::Update { - ref patch_id, + patch_id, ref message, } => { - create::run( + update::run( &storage, &profile, &workdir, - *patch_id, + patch_id, message.clone(), - options, + &options, )?; } } diff --git a/radicle-cli/src/commands/patch/common.rs b/radicle-cli/src/commands/patch/common.rs index 322ffbed..120259f4 100644 --- a/radicle-cli/src/commands/patch/common.rs +++ b/radicle-cli/src/commands/patch/common.rs @@ -1,13 +1,44 @@ +use std::fmt; +use std::path::Path; + +use anyhow::anyhow; + use radicle::cob::patch::{Clock, MergeTarget, Patch, PatchId, Patches}; use radicle::git; use radicle::git::raw::Oid; use radicle::prelude::*; +use radicle::storage; use radicle::storage::git::Repository; use radicle::storage::Remote; use crate::terminal as term; use crate::terminal::args::Error; +use super::Options; + +/// Give the name of the branch or an appropriate error. +#[inline] +pub fn branch_name<'a>(branch: &'a git::raw::Branch) -> anyhow::Result<&'a str> { + branch + .name()? + .ok_or(anyhow!("head branch must be valid UTF-8")) +} + +/// Give the oid of the branch or an appropriate error. +#[inline] +pub fn branch_oid(branch: &git::raw::Branch) -> anyhow::Result { + let oid = branch + .get() + .target() + .ok_or(anyhow!("invalid HEAD ref; aborting"))?; + Ok(oid.into()) +} + +#[inline] +pub fn confirm(prompt: D, options: &Options) -> bool { + !options.confirm || term::confirm(prompt) +} + /// List of merge targets. #[derive(Debug, Default)] pub struct MergeTargets { @@ -40,6 +71,38 @@ pub fn find_merge_targets( Ok(targets) } +/// Determine the merge target for this patch. This can ben any tracked remote's "default" +/// branch, as well as your own (eg. `rad/master`). +pub fn get_merge_target( + project: &Project, + storage: &Repository, + head_branch: &git::raw::Branch, +) -> anyhow::Result<(storage::Remote, git::Oid)> { + let head_oid = head_branch + .get() + .target() + .ok_or(anyhow!("invalid HEAD ref; aborting"))?; + let mut spinner = term::spinner("Analyzing remotes..."); + let targets = find_merge_targets(&head_oid, project.default_branch().as_refstr(), storage)?; + + // eg. `refs/namespaces//refs/heads/master` + let (target_peer, target_oid) = match targets.not_merged.as_slice() { + [] => { + spinner.message("All tracked peers are up to date."); + todo!("handle case without target"); + } + [target] => target, + _ => { + // TODO: Let user select which branch to use as a target. + todo!(); + } + }; + // TODO: Tell user how many peers don't have this change. + spinner.finish(); + + Ok((target_peer.clone(), *target_oid)) +} + /// Return the [`Oid`] of the merge target. pub fn patch_merge_target_oid(target: MergeTarget, repository: &Repository) -> anyhow::Result { match target { @@ -106,6 +169,60 @@ pub fn pretty_commit_version( Ok(oid) } +#[inline] +pub fn try_branch(reference: git::raw::Reference<'_>) -> anyhow::Result { + let branch = if reference.is_branch() { + git::raw::Branch::wrap(reference) + } else { + anyhow::bail!("cannot create patch from detached head; aborting") + }; + Ok(branch) +} + +/// Push branch to the local storage. +/// +/// The branch must be in storage for others to merge the `Patch`. +pub fn push_to_storage( + storage: &Repository, + head_branch: &git::raw::Branch, + options: &Options, +) -> anyhow::Result<()> { + let head_oid = branch_oid(head_branch)?; + let mut spinner = term::spinner(format!( + "Looking for HEAD ({}) in storage...", + term::format::secondary(term::format::oid(head_oid)) + )); + if storage.commit(head_oid).is_err() { + if !options.push { + spinner.failed(); + term::blank(); + + return Err(Error::WithHint { + err: anyhow!("Current branch head was not found in storage"), + hint: "hint: run `git push rad` and try again", + } + .into()); + } + spinner.message("Pushing HEAD to storage..."); + + let output = match head_branch.upstream() { + Ok(_) => git::run::<_, _, &str, &str>(Path::new("."), ["push", "rad"], [])?, + Err(_) => git::run::<_, _, &str, &str>( + Path::new("."), + ["push", "--set-upstream", "rad", branch_name(head_branch)?], + [], + )?, + }; + if options.verbose { + spinner.finish(); + term::blob(output); + } + } + spinner.finish(); + + Ok(()) +} + /// Find patches with a merge base equal to the one provided. pub fn find_unmerged_with_base( patch_head: Oid, diff --git a/radicle-cli/src/commands/patch/create.rs b/radicle-cli/src/commands/patch/create.rs index 30e986f1..58786649 100644 --- a/radicle-cli/src/commands/patch/create.rs +++ b/radicle-cli/src/commands/patch/create.rs @@ -1,20 +1,15 @@ -use std::fmt; -use std::path::Path; - use anyhow::{anyhow, Context}; -use radicle::cob::patch::{MergeTarget, PatchId, PatchMut, Patches}; +use radicle::cob::patch; use radicle::git; -use radicle::git::raw::Oid; use radicle::prelude::*; +use radicle::storage; use radicle::storage::git::Repository; use crate::terminal as term; -use crate::terminal::args::Error; -use crate::terminal::patch; -use super::common; -use super::{OptPatch, Options}; +use super::common::*; +use super::Options; const PATCH_MSG: &str = r#" "#; -const REVISION_MSG: &str = r#" - -"#; - -#[inline] -fn confirm(prompt: D, options: &Options) -> bool { - !options.confirm || term::confirm(prompt) -} - -/// Give the name of the branch or an appropriate error. -#[inline] -fn branch_name<'a>(branch: &'a git::raw::Branch) -> anyhow::Result<&'a str> { - branch - .name()? - .ok_or(anyhow!("head branch must be valid UTF-8")) -} - -/// Run patch creation. -pub fn run( - storage: &Repository, - profile: &Profile, +pub fn handle_patch_message( + message: term::patch::Message, workdir: &git::raw::Repository, - patch_id: OptPatch, - message: patch::Comment, - options: Options, -) -> anyhow::Result<()> { - let project = storage.project_of(&profile.public_key).context(format!( - "couldn't load project {} from local state", - storage.id - ))?; - - term::headline(&format!( - "🌱 Creating patch for {}", - term::format::highlight(project.name()) - )); - - let signer = term::signer(profile)?; - let mut patches = Patches::open(profile.public_key, storage)?; - - // `HEAD`; This is what we are proposing as a patch. - let head = workdir.head()?; - let head_oid = head.target().ok_or(anyhow!("invalid HEAD ref; aborting"))?; - let head_commit = workdir.find_commit(head_oid)?; - let head_branch = if head.is_branch() { - git::raw::Branch::wrap(head) - } else { - anyhow::bail!("cannot create patch from detached head; aborting") - }; - - // Make sure the `HEAD` commit can be found in the monorepo. Otherwise there - // is no way for anyone to merge this patch. - let mut spinner = term::spinner(format!( - "Looking for HEAD ({}) in storage...", - term::format::secondary(term::format::oid(head_oid)) - )); - if storage.commit(head_oid.into()).is_err() { - if !options.push { - spinner.failed(); - term::blank(); - - return Err(Error::WithHint { - err: anyhow!("Current branch head was not found in storage"), - hint: "hint: run `git push rad` and try again", - } - .into()); - } - spinner.message("Pushing HEAD to storage..."); - - let output = match head_branch.upstream() { - Ok(_) => git::run::<_, _, &str, &str>(Path::new("."), ["push", "rad"], [])?, - Err(_) => git::run::<_, _, &str, &str>( - Path::new("."), - ["push", "--set-upstream", "rad", branch_name(&head_branch)?], - [], - )?, - }; - if options.verbose { - spinner.finish(); - term::blob(output); - } - } - spinner.finish(); - - // Determine the merge target for this patch. This can ben any tracked remote's "default" - // branch, as well as your own (eg. `rad/master`). - let mut spinner = term::spinner("Analyzing remotes..."); - let targets = - common::find_merge_targets(&head_oid, project.default_branch().as_refstr(), storage)?; - - // eg. `refs/namespaces//refs/heads/master` - let (target_peer, target_oid) = match targets.not_merged.as_slice() { - [] => { - spinner.message("All tracked peers are up to date."); - return Ok(()); - } - [target] => target, - _ => { - // TODO: Let user select which branch to use as a target. - todo!(); - } - }; - // TODO: Tell user how many peers don't have this change. - spinner.finish(); - - // TODO: Handle case where `rad/master` isn't up to date with the target. - // In that case we should warn the user that their master branch is not up - // to date, and error out, unless the user specifies manually the merge - // base. - - // The merge base is basically the commit at which the histories diverge. - let base_oid = workdir.merge_base((*target_oid).into(), head_oid)?; - let commits = common::patch_commits(workdir, &base_oid, &head_oid)?; - - let patch = match &patch_id { - OptPatch::None => None, - OptPatch::Any => { - let mut spinner = term::spinner("Finding patches to update..."); - let mut result = common::find_unmerged_with_base( - head_oid, - **target_oid, - base_oid, - &patches, - workdir, - )?; - - if let Some((id, patch, clock)) = result.pop() { - if result.is_empty() { - spinner.message(format!( - "Found existing patch {} {}", - term::format::tertiary(term::format::cob(&id)), - term::format::italic(patch.title()) - )); - spinner.finish(); - term::blank(); - - Some((id, PatchMut::new(id, patch, clock, &mut patches))) - } else { - spinner.failed(); - term::blank(); - anyhow::bail!("More than one patch available to update, please specify an id with `rad patch --update `"); - } - } else { - spinner.failed(); - term::blank(); - anyhow::bail!("No patches found that share a base, please create a new patch or specify the patch id manually"); - } - } - OptPatch::Patch(id) => { - if let Ok(patch) = patches.get_mut(id) { - Some((*id, patch)) - } else { - anyhow::bail!("Patch `{}` not found", id); - } - } - }; - - if let Some((id, patch)) = patch { - if confirm("Update?", &options) { - term::blank(); - - return update( - patch, id, &base_oid, &head_oid, workdir, options, message, &signer, - ); - } else { - anyhow::bail!("Patch update aborted by user"); - } - } - - // TODO: List matching working copy refs for all targets. - - term::blank(); - term::info!( - "{}/{} ({}) <- {}/{} ({})", - term::format::dim(target_peer.id), - term::format::highlight(project.default_branch().to_string()), - term::format::secondary(term::format::oid(*target_oid)), - term::format::dim(term::format::node(patches.public_key())), - term::format::highlight(branch_name(&head_branch)?), - term::format::secondary(term::format::oid(head_oid)), - ); - - // TODO: Test case where the target branch has been re-written passed the merge-base, since the fork was created - // This can also happen *after* the patch is created. - - term::patch::print_commits_ahead_behind(workdir, head_oid, (*target_oid).into())?; - - // List commits in patch that aren't in the target branch. - term::blank(); - term::patch::list_commits(&commits)?; - term::blank(); - - if !confirm("Continue?", &options) { - anyhow::bail!("patch proposal aborted by user"); - } - + head_branch: &git::raw::Branch, +) -> anyhow::Result<(String, String)> { + let head_oid = branch_oid(head_branch)?; + let head_commit = workdir.find_commit(*head_oid)?; let commit_message = head_commit .message() .ok_or(anyhow!("commit summary is not valid UTF-8; aborting"))?; @@ -242,18 +46,101 @@ pub fn run( term::patch::print_title_desc(title, &description); term::blank(); - if !confirm("Create patch?", &options) { + Ok((title.to_string(), description)) +} + +fn show_patch_commit_info( + project: &Project, + workdir: &git::raw::Repository, + node_id: &NodeId, + head_branch: &git::raw::Branch, + target_peer: &storage::Remote, + target_oid: git::Oid, +) -> anyhow::Result<()> { + let head_oid = branch_oid(head_branch)?; + // The merge base is basically the commit at which the histories diverge. + let base_oid = workdir.merge_base(*target_oid, *head_oid)?; + let commits = patch_commits(workdir, &base_oid, &head_oid)?; + + term::blank(); + term::info!( + "{}/{} ({}) <- {}/{} ({})", + term::format::dim(target_peer.id), + term::format::highlight(project.default_branch().to_string()), + term::format::secondary(term::format::oid(*target_oid)), + term::format::dim(term::format::node(node_id)), + term::format::highlight(branch_name(head_branch)?), + term::format::secondary(term::format::oid(head_oid)), + ); + + // TODO: Test case where the target branch has been re-written passed the merge-base, since the fork was created + // This can also happen *after* the patch is created. + + term::patch::print_commits_ahead_behind(workdir, *head_oid, *target_oid)?; + + // List commits in patch that aren't in the target branch. + term::blank(); + term::patch::list_commits(&commits)?; + term::blank(); + + Ok(()) +} + +/// Run patch creation. +pub fn run( + storage: &Repository, + profile: &Profile, + workdir: &git::raw::Repository, + message: term::patch::Message, + options: Options, +) -> anyhow::Result<()> { + let mut patches = patch::Patches::open(profile.public_key, storage)?; + let project = storage.project_of(&profile.public_key).context(format!( + "couldn't load project {} from local state", + storage.id + ))?; + let head_branch = try_branch(workdir.head()?)?; + + term::headline(&format!( + "🌱 Creating patch for {}", + term::format::highlight(project.name()) + )); + + push_to_storage(storage, &head_branch, &options)?; + let (target_peer, target_oid) = get_merge_target(&project, storage, &head_branch)?; + + // TODO: Handle case where `rad/master` isn't up to date with the target. + // In that case we should warn the user that their master branch is not up + // to date, and error out, unless the user specifies manually the merge + // base. + + show_patch_commit_info( + &project, + workdir, + patches.public_key(), + &head_branch, + &target_peer, + target_oid, + )?; + + // TODO: List matching working copy refs for all targets. + + let (title, description) = handle_patch_message(message, workdir, &head_branch)?; + + if !confirm("Continue?", &options) { anyhow::bail!("patch proposal aborted by user"); } + let head_oid = branch_oid(&head_branch)?; + let base_oid = workdir.merge_base(*target_oid, *head_oid)?; let patch = patches.create( title, &description, - MergeTarget::default(), + patch::MergeTarget::default(), base_oid, head_oid, &[], - &signer, + &term::signer(profile)?, )?; term::blank(); @@ -265,53 +152,3 @@ pub fn run( Ok(()) } - -/// Update an existing patch with a new revision. -fn update( - mut patch: PatchMut, - patch_id: PatchId, - base: &Oid, - head: &Oid, - workdir: &git::raw::Repository, - options: Options, - message: patch::Comment, - signer: &G, -) -> anyhow::Result<()> { - // TODO(cloudhead): Handle error. - let (_, current_revision) = patch.latest().unwrap(); - let current_version = patch.version(); - - if *current_revision.oid == *head { - term::info!("Nothing to do, patch is already up to date."); - return Ok(()); - } - - term::info!( - "{} {} ({}) -> {} ({})", - term::format::tertiary(term::format::cob(&patch_id)), - term::format::dim(format!("R{current_version}")), - term::format::secondary(term::format::oid(current_revision.oid)), - term::format::dim(format!("R{}", current_version + 1)), - term::format::secondary(term::format::oid(*head)), - ); - let message = message.get(REVISION_MSG); - - // Difference between the two revisions. - term::patch::print_commits_ahead_behind(workdir, *head, *current_revision.oid)?; - term::blank(); - - if !confirm("Continue?", &options) { - anyhow::bail!("patch update aborted by user"); - } - patch.update(message, *base, *head, signer)?; - - term::blank(); - term::success!("Patch {} updated 🌱", term::format::highlight(patch_id)); - term::blank(); - - if options.sync { - // TODO - } - - Ok(()) -} diff --git a/radicle-cli/src/commands/patch/update.rs b/radicle-cli/src/commands/patch/update.rs new file mode 100644 index 00000000..dfac0f2c --- /dev/null +++ b/radicle-cli/src/commands/patch/update.rs @@ -0,0 +1,148 @@ +use anyhow::Context; + +use radicle::cob::patch; +use radicle::git; +use radicle::prelude::*; +use radicle::storage::git::Repository; + +use super::common::*; +use super::Options; +use crate::terminal as term; + +const REVISION_MSG: &str = r#" + +"#; + +fn select_patch( + patches: &patch::Patches, + workdir: &git::raw::Repository, + head_branch: &git::raw::Branch, + target_oid: git::Oid, +) -> anyhow::Result { + let head_oid = branch_oid(head_branch)?; + let base_oid = workdir.merge_base(*target_oid, *head_oid)?; + + let mut spinner = term::spinner("Finding patches to update..."); + let mut result = find_unmerged_with_base(*head_oid, *target_oid, base_oid, patches, workdir)?; + + let Some((id, patch, _)) = result.pop() else { + spinner.failed(); + term::blank(); + anyhow::bail!("No patches found that share a base, please create a new patch or specify the patch id manually"); + }; + + if !result.is_empty() { + spinner.failed(); + term::blank(); + anyhow::bail!("More than one patch available to update, please specify an id with `rad patch --update `"); + } + spinner.message(format!( + "Found existing patch {} {}", + term::format::tertiary(term::format::cob(&id)), + term::format::italic(patch.title()) + )); + spinner.finish(); + term::blank(); + + Ok(id) +} + +fn show_update_commit_info( + workdir: &git::raw::Repository, + patch_id: &patch::PatchId, + patch: &patch::Patch, + current_revision: &patch::Revision, + head_branch: &git::raw::Branch, +) -> anyhow::Result<()> { + // TODO(cloudhead): Handle error. + let current_version = patch.version(); + let head_oid = branch_oid(head_branch)?; + + term::blank(); + term::info!( + "{} {} ({}) -> {} ({})", + term::format::tertiary(term::format::cob(patch_id)), + term::format::dim(format!("R{current_version}")), + term::format::secondary(term::format::oid(current_revision.oid)), + term::format::dim(format!("R{}", current_version + 1)), + term::format::secondary(term::format::oid(*head_oid)), + ); + + // Difference between the two revisions. + let head_oid = branch_oid(head_branch)?; + term::patch::print_commits_ahead_behind(workdir, *head_oid, *current_revision.oid)?; + term::blank(); + + Ok(()) +} + +/// Run patch creation. +pub fn run( + storage: &Repository, + profile: &Profile, + workdir: &git::raw::Repository, + patch_id: Option, + message: term::patch::Message, + options: &Options, +) -> anyhow::Result<()> { + let project = storage.project_of(&profile.public_key).context(format!( + "couldn't load project {} from local state", + storage.id + ))?; + // `HEAD`; This is what we are proposing as a patch. + let head_branch = try_branch(workdir.head()?)?; + + term::headline(&format!( + "🌱 Updating patch for {}", + term::format::highlight(project.name()) + )); + + push_to_storage(storage, &head_branch, options)?; + + let (_, target_oid) = get_merge_target(&project, storage, &head_branch)?; + let mut patches = patch::Patches::open(profile.public_key, storage)?; + + let patch_id = match patch_id { + Some(patch_id) => patch_id, + None => select_patch(&patches, workdir, &head_branch, target_oid)?, + }; + let Ok(mut patch) = patches.get_mut(&patch_id) else { + anyhow::bail!("Patch `{patch_id}` not found"); + }; + + if !confirm("Update patch?", options) { + anyhow::bail!("Patch update aborted by user"); + } + + // TODO(cloudhead): Handle error. + let (_, current_revision) = patch.latest().unwrap(); + if *current_revision.oid == *branch_oid(&head_branch)? { + term::info!("Nothing to do, patch is already up to date."); + return Ok(()); + } + + show_update_commit_info(workdir, &patch_id, &patch, current_revision, &head_branch)?; + + if !confirm("Continue?", options) { + anyhow::bail!("patch update aborted by user"); + } + + let head_oid = branch_oid(&head_branch)?; + let base_oid = workdir.merge_base(*target_oid, *head_oid)?; + let message = message.get(REVISION_MSG); + let signer = term::signer(profile)?; + patch.update(message, base_oid, *head_oid, &signer)?; + + term::blank(); + term::success!("Patch {} updated 🌱", term::format::highlight(patch_id)); + term::blank(); + + if options.sync { + // TODO + } + + Ok(()) +} diff --git a/radicle-cli/src/commands/review.rs b/radicle-cli/src/commands/review.rs index baf19a4d..b84886bc 100644 --- a/radicle-cli/src/commands/review.rs +++ b/radicle-cli/src/commands/review.rs @@ -9,7 +9,7 @@ use radicle::rad; use crate::terminal as term; use crate::terminal::args::{Args, Error, Help}; -use crate::terminal::patch::Comment; +use crate::terminal::patch::Message; pub const HELP: Help = Help { name: "review", @@ -47,7 +47,7 @@ Markdown supported. pub struct Options { pub id: PatchId, pub revision: Option, - pub message: Comment, + pub message: Message, pub sync: bool, pub verbose: bool, pub verdict: Option, @@ -60,7 +60,7 @@ impl Args for Options { let mut parser = lexopt::Parser::from_args(args); let mut id: Option = None; let mut revision: Option = None; - let mut message = Comment::default(); + let mut message = Message::default(); let mut sync = true; let mut verbose = false; let mut verdict = None; @@ -85,13 +85,13 @@ impl Args for Options { sync = false; } Long("message") | Short('m') => { - if message != Comment::Blank { + if message != Message::Blank { let txt: String = parser.value()?.to_string_lossy().into(); message.append(&txt); } } Long("no-message") => { - message = Comment::Blank; + message = Message::Blank; } Long("verbose") | Short('v') => { verbose = true; diff --git a/radicle-cli/src/terminal/patch.rs b/radicle-cli/src/terminal/patch.rs index b7c83d4a..db33464e 100644 --- a/radicle-cli/src/terminal/patch.rs +++ b/radicle-cli/src/terminal/patch.rs @@ -2,9 +2,9 @@ use radicle::git; use crate::terminal as term; -/// How a comment is to be supplied by the user for a patch or issue on the terminal. +/// The user supplied `Patch` description. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum Comment { +pub enum Message { /// Prompt user to write comment in editor. Edit, /// Don't leave a comment. @@ -13,18 +13,18 @@ pub enum Comment { Text(String), } -impl Comment { - /// Get the comment as a string according to the method. +impl Message { + /// Get the `Message` as a string according to the method. pub fn get(self, help: &str) -> String { let comment = match self { - Comment::Edit => term::Editor::new() + Message::Edit => term::Editor::new() .require_save(true) .trim_newlines(true) .extension(".markdown") .edit(help) .unwrap(), - Comment::Blank => None, - Comment::Text(c) => Some(c), + Message::Blank => None, + Message::Text(c) => Some(c), }; let comment = comment.unwrap_or_default().replace(help, ""); let comment = comment.trim(); @@ -33,15 +33,15 @@ impl Comment { } pub fn append(&mut self, arg: &str) { - if let Comment::Text(v) = self { + if let Message::Text(v) = self { v.extend(["\n\n", arg]); } else { - *self = Comment::Text(arg.into()); + *self = Message::Text(arg.into()); }; } } -impl Default for Comment { +impl Default for Message { fn default() -> Self { Self::Edit }