From 92648f934e270a076b6f3867a039060bcf8cd9bb Mon Sep 17 00:00:00 2001 From: Slack Coder Date: Mon, 8 May 2023 16:39:00 -0500 Subject: [PATCH] cli: Patch message from all commits Help the user when creating a patch by listing the messages of all commits associated with the patch in almost the same format as git uses for squashing. --- radicle-cli/examples/rad-merge-via-push.md | 18 +- radicle-cli/examples/rad-patch-via-push.md | 64 ++-- radicle-cli/src/commands/patch/create.rs | 11 +- radicle-cli/src/commands/patch/edit.rs | 3 +- radicle-cli/src/commands/patch/update.rs | 2 +- radicle-cli/src/terminal/patch.rs | 357 +++++++++++++++++++-- radicle-remote-helper/src/push.rs | 18 +- 7 files changed, 386 insertions(+), 87 deletions(-) diff --git a/radicle-cli/examples/rad-merge-via-push.md b/radicle-cli/examples/rad-merge-via-push.md index 1fcb280f..13da4b12 100644 --- a/radicle-cli/examples/rad-merge-via-push.md +++ b/radicle-cli/examples/rad-merge-via-push.md @@ -9,10 +9,10 @@ To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkE * [new reference] HEAD -> refs/patches ``` ``` (stderr) RAD_SOCKET=/dev/null -$ git checkout -b feature/2 -q +$ git checkout -b feature/2 -q master $ git commit --allow-empty -q -m "Second change" $ git push rad HEAD:refs/patches -✓ Patch bf923942537708f7aec4680a321be68cf933e747 opened +✓ Patch 928d76e22ef98a8406f2e4e4bcc8878533bbdfe0 opened To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi * [new reference] HEAD -> refs/patches ``` @@ -23,7 +23,7 @@ This creates some remote tracking branches for us: $ git branch -r rad/master rad/patches/0ec956c94256fa101db4c32956ce195a1aa0edf2 - rad/patches/bf923942537708f7aec4680a321be68cf933e747 + rad/patches/928d76e22ef98a8406f2e4e4bcc8878533bbdfe0 ``` And some remote refs: @@ -36,12 +36,12 @@ $ rad inspect --refs |-- cobs | `-- xyz.radicle.patch | |-- 0ec956c94256fa101db4c32956ce195a1aa0edf2 - | `-- bf923942537708f7aec4680a321be68cf933e747 + | `-- 928d76e22ef98a8406f2e4e4bcc8878533bbdfe0 |-- heads | |-- master | `-- patches | |-- 0ec956c94256fa101db4c32956ce195a1aa0edf2 - | `-- bf923942537708f7aec4680a321be68cf933e747 + | `-- 928d76e22ef98a8406f2e4e4bcc8878533bbdfe0 `-- rad |-- id `-- sigrefs @@ -61,9 +61,9 @@ When we push to `rad/master`, we automatically merge the patches: ``` (stderr) RAD_SOCKET=/dev/null $ git push rad master ✓ Patch 0ec956c94256fa101db4c32956ce195a1aa0edf2 merged -✓ Patch bf923942537708f7aec4680a321be68cf933e747 merged +✓ Patch 928d76e22ef98a8406f2e4e4bcc8878533bbdfe0 merged To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi - f2de534..e9fff34 master -> master + f2de534..d6399c7 master -> master ``` ``` $ rad patch --merged @@ -71,7 +71,7 @@ $ rad patch --merged │ ● ID Title Author Head + - Updated │ ├──────────────────────────────────────────────────────────────────────────────────┤ │ ✔ 0ec956c First change z6MknSL…StBU8Vi (you) 20aa5dd +0 -0 [ ... ] │ -│ ✔ bf92394 Second change z6MknSL…StBU8Vi (you) e9fff34 +0 -0 [ ... ] │ +│ ✔ 928d76e Second change z6MknSL…StBU8Vi (you) daf349f +0 -0 [ ... ] │ ╰──────────────────────────────────────────────────────────────────────────────────╯ ``` @@ -92,7 +92,7 @@ $ rad inspect --refs |-- cobs | `-- xyz.radicle.patch | |-- 0ec956c94256fa101db4c32956ce195a1aa0edf2 - | `-- bf923942537708f7aec4680a321be68cf933e747 + | `-- 928d76e22ef98a8406f2e4e4bcc8878533bbdfe0 |-- heads | `-- master `-- rad diff --git a/radicle-cli/examples/rad-patch-via-push.md b/radicle-cli/examples/rad-patch-via-push.md index bebe509d..dbb495c2 100644 --- a/radicle-cli/examples/rad-patch-via-push.md +++ b/radicle-cli/examples/rad-patch-via-push.md @@ -72,10 +72,10 @@ We can also create patches by pushing to the `rad/patches` remote. It's a bit simpler: ``` (stderr) -$ git checkout -b feature/2 -q +$ git checkout -b feature/2 -q master $ git commit -a -m "Add more things" -q --allow-empty $ git push rad/patches -✓ Patch 2af090f48003d86f735163794bfffdb2691f369e opened +✓ Patch b8ab1c99c1c8205680a3494f04fb3934ec738ddd opened To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi * [new reference] HEAD -> refs/patches ``` @@ -85,7 +85,7 @@ We see both branches with upstreams now: ``` $ git branch -vv feature/1 42d894a [rad/patches/2647168c23e7c2b2c1936d695443944e143bc3f7] Add things -* feature/2 b94a835 [rad/patches/2af090f48003d86f735163794bfffdb2691f369e] Add more things +* feature/2 8b0ea80 [rad/patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd] Add more things master f2de534 [rad/master] Second commit ``` @@ -97,7 +97,7 @@ $ rad patch │ ● ID Title Author Head + - Updated │ ├────────────────────────────────────────────────────────────────────────────────────┤ │ ● 2647168 Add things #1 z6MknSL…StBU8Vi (you) 42d894a +0 -0 [ ... ] │ -│ ● 2af090f Add more things z6MknSL…StBU8Vi (you) b94a835 +0 -0 [ ... ] │ +│ ● b8ab1c9 Add more things z6MknSL…StBU8Vi (you) 8b0ea80 +0 -0 [ ... ] │ ╰────────────────────────────────────────────────────────────────────────────────────╯ ``` @@ -116,9 +116,9 @@ $ git commit -a -m "Improve code" -q --allow-empty ``` (stderr) $ git push -✓ Patch 2af090f updated to f532e40e44de298b27d2255acb50b99bf0377a04 +✓ Patch b8ab1c9 updated to 8767880c31b9e4a04cdb07ad6faa9ce453980399 To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi - b94a835..662843e feature/2 -> patches/2af090f48003d86f735163794bfffdb2691f369e + 8b0ea80..02bef3f feature/2 -> patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd ``` This last `git push` worked without specifying an upstream branch despite the @@ -136,22 +136,21 @@ This allows for pushing to the remote patch branch without using the full We can then see that the patch head has moved: ``` -$ rad patch show 2af090f +$ rad patch show b8ab1c9 ╭──────────────────────────────────────────────────────────────────────────────╮ │ Title Add more things │ -│ Patch 2af090f48003d86f735163794bfffdb2691f369e │ +│ Patch b8ab1c99c1c8205680a3494f04fb3934ec738ddd │ │ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ -│ Head 662843ed81e76efa69d7901fb7bdd775043015d0 │ +│ Head 02bef3fac41b2f98bb3c02b868a53ddfecb55b5f │ │ Branches feature/2 │ -│ Commits ahead 3, behind 0 │ +│ Commits ahead 2, behind 0 │ │ Status open │ ├──────────────────────────────────────────────────────────────────────────────┤ -│ 662843e Improve code │ -│ b94a835 Add more things │ -│ 42d894a Add things │ +│ 02bef3f Improve code │ +│ 8b0ea80 Add more things │ ├──────────────────────────────────────────────────────────────────────────────┤ │ ● opened by (you) (z6MknSL…StBU8Vi) [ ... ] │ -│ ↑ updated to f532e40e44de298b27d2255acb50b99bf0377a04 (662843e) [ ... ] │ +│ ↑ updated to 8767880c31b9e4a04cdb07ad6faa9ce453980399 (02bef3f) [ ... ] │ ╰──────────────────────────────────────────────────────────────────────────────╯ ``` @@ -159,19 +158,19 @@ And we can check that all the refs are properly updated in our repository: ``` $ git rev-parse HEAD -662843ed81e76efa69d7901fb7bdd775043015d0 +02bef3fac41b2f98bb3c02b868a53ddfecb55b5f ``` ``` $ git status --short --branch -## feature/2...rad/patches/2af090f48003d86f735163794bfffdb2691f369e +## feature/2...rad/patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd ``` ``` -$ git rev-parse refs/remotes/rad/patches/2af090f48003d86f735163794bfffdb2691f369e -662843ed81e76efa69d7901fb7bdd775043015d0 -$ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi refs/heads/patches/2af090f48003d86f735163794bfffdb2691f369e -662843ed81e76efa69d7901fb7bdd775043015d0 refs/heads/patches/2af090f48003d86f735163794bfffdb2691f369e +$ git rev-parse refs/remotes/rad/patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd +02bef3fac41b2f98bb3c02b868a53ddfecb55b5f +$ git ls-remote rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi refs/heads/patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd +02bef3fac41b2f98bb3c02b868a53ddfecb55b5f refs/heads/patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd ``` ## Force push @@ -183,7 +182,7 @@ Let's try. ``` $ git commit --amend -m "Amended commit" --allow-empty -[feature/2 3507cd5] Amended commit +[feature/2 9304dbc] Amended commit Date: [..] ``` @@ -192,7 +191,7 @@ Now let's push to the patch head. ``` (stderr) (fail) $ git push To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi - ! [rejected] feature/2 -> patches/2af090f48003d86f735163794bfffdb2691f369e (non-fast-forward) + ! [rejected] feature/2 -> patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd (non-fast-forward) error: failed to push some refs to 'rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi' hint: Updates were rejected because a pushed branch tip is behind its remote hint: counterpart. Check out this branch and integrate the remote changes @@ -205,30 +204,29 @@ use `--force` to force the update. ``` (stderr) $ git push --force -✓ Patch 2af090f updated to d7590abe04594263eeb88cc0e28502139ec8414f +✓ Patch b8ab1c9 updated to f24334f8cea7b7a5bcaf3bc6deb1408c9bf507ad To rad://z42hL2jL4XNk6K8oHQaSWfMgCL7ji/z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi - + 662843e...3507cd5 feature/2 -> patches/2af090f48003d86f735163794bfffdb2691f369e (forced update) + + 02bef3f...9304dbc feature/2 -> patches/b8ab1c99c1c8205680a3494f04fb3934ec738ddd (forced update) ``` That worked. We can see the new revision if we call `rad patch show`: ``` -$ rad patch show 2af090f +$ rad patch show b8ab1c9 ╭──────────────────────────────────────────────────────────────────────────────╮ │ Title Add more things │ -│ Patch 2af090f48003d86f735163794bfffdb2691f369e │ +│ Patch b8ab1c99c1c8205680a3494f04fb3934ec738ddd │ │ Author did:key:z6MknSLrJoTcukLrE435hVNQT4JUhbvWLX4kUzqkEStBU8Vi │ -│ Head 3507cd57811fe5f21f6a0a610a1ac8068b3a5d9f │ +│ Head 9304dbc445925187994a7a93222a3f8bde73b785 │ │ Branches feature/2 │ -│ Commits ahead 3, behind 0 │ +│ Commits ahead 2, behind 0 │ │ Status open │ ├──────────────────────────────────────────────────────────────────────────────┤ -│ 3507cd5 Amended commit │ -│ b94a835 Add more things │ -│ 42d894a Add things │ +│ 9304dbc Amended commit │ +│ 8b0ea80 Add more things │ ├──────────────────────────────────────────────────────────────────────────────┤ │ ● opened by (you) (z6MknSL…StBU8Vi) [ ... ] │ -│ ↑ updated to f532e40e44de298b27d2255acb50b99bf0377a04 (662843e) [ ... ] │ -│ ↑ updated to d7590abe04594263eeb88cc0e28502139ec8414f (3507cd5) [ ... ] │ +│ ↑ updated to 8767880c31b9e4a04cdb07ad6faa9ce453980399 (02bef3f) [ ... ] │ +│ ↑ updated to f24334f8cea7b7a5bcaf3bc6deb1408c9bf507ad (9304dbc) [ ... ] │ ╰──────────────────────────────────────────────────────────────────────────────╯ ``` diff --git a/radicle-cli/src/commands/patch/create.rs b/radicle-cli/src/commands/patch/create.rs index ee622452..13a5b8c7 100644 --- a/radicle-cli/src/commands/patch/create.rs +++ b/radicle-cli/src/commands/patch/create.rs @@ -1,5 +1,3 @@ -use anyhow::anyhow; - use radicle::cob::patch; use radicle::git; use radicle::node::Handle; @@ -81,13 +79,10 @@ pub fn run( // TODO: List matching working copy refs for all targets. let head_oid = branch_oid(&head_branch)?; - let head_commit = storage.backend.find_commit(*head_oid)?; - let head_commit_msg = head_commit - .message() - .ok_or(anyhow!("commit summary is not valid UTF-8; aborting"))?; - let (title, description) = term::patch::get_message(message, head_commit_msg)?; - let base_oid = storage.backend.merge_base(*target_oid, *head_oid)?; + let (title, description) = + term::patch::get_create_message(message, &storage.backend, &base_oid.into(), &head_oid)?; + let signer = term::signer(profile)?; let patch = if draft { patches.draft( diff --git a/radicle-cli/src/commands/patch/edit.rs b/radicle-cli/src/commands/patch/edit.rs index 39734a15..075bf36f 100644 --- a/radicle-cli/src/commands/patch/edit.rs +++ b/radicle-cli/src/commands/patch/edit.rs @@ -18,8 +18,7 @@ pub fn run( anyhow::bail!("Patch `{patch_id}` not found"); }; - let default_msg = term::patch::message(patch.title(), patch.description()); - let (title, description) = term::patch::get_message(message, &default_msg)?; + let (title, description) = term::patch::get_edit_message(message, &patch)?; let title = if title != patch.title() { Some(title) diff --git a/radicle-cli/src/commands/patch/update.rs b/radicle-cli/src/commands/patch/update.rs index ecf2c7d2..b0eea912 100644 --- a/radicle-cli/src/commands/patch/update.rs +++ b/radicle-cli/src/commands/patch/update.rs @@ -92,7 +92,7 @@ pub fn run( let head_oid = branch_oid(&head_branch)?; let base_oid = storage.backend.merge_base(*target_oid, *head_oid)?; - let message = term::patch::get_update_message(message)?; + let message = term::patch::get_update_message(message, workdir, current_revision, &head_oid)?; let signer = term::signer(profile)?; let revision = patch.update(message, base_oid, *head_oid, &signer)?; diff --git a/radicle-cli/src/terminal/patch.rs b/radicle-cli/src/terminal/patch.rs index 03a3741a..6ffde699 100644 --- a/radicle-cli/src/terminal/patch.rs +++ b/radicle-cli/src/terminal/patch.rs @@ -1,11 +1,29 @@ +use std::fmt; +use std::fmt::Write; use std::io; use std::io::IsTerminal as _; +use thiserror::Error; + +use radicle::cob; +use radicle::cob::patch; use radicle::git; use crate::terminal as term; use crate::terminal::Element; +#[derive(Debug, Error)] +pub enum Error { + #[error(transparent)] + Fmt(#[from] fmt::Error), + #[error("git: {0}")] + Git(#[from] git::raw::Error), + #[error("i/o error: {0}")] + Io(#[from] io::Error), + #[error("invalid utf-8 string")] + InvalidUtf8, +} + /// The user supplied `Patch` description. #[derive(Clone, Debug, PartialEq, Eq)] pub enum Message { @@ -78,21 +96,127 @@ pub fn message(title: &str, description: &str) -> String { format!("{title}\n\n{description}").trim().to_string() } +/// Create a helpful default `Patch` message out of one or more commit messages. +fn message_from_commits(name: &str, commits: Vec) -> Result { + let mut commits = commits.into_iter().rev(); + let count = commits.len(); + let Some(commit) = commits.next() else { + return Ok(String::default()); + }; + let commit_msg = commit.message().ok_or(Error::InvalidUtf8)?.to_string(); + + if count == 1 { + return Ok(commit_msg); + } + + // Many commits + let mut msg = String::new(); + writeln!(&mut msg, "")?; + writeln!(&mut msg)?; + writeln!(&mut msg, "{commit_msg}")?; + writeln!(&mut msg)?; + + for (i, commit) in commits.enumerate() { + let commit_msg = commit.message().ok_or(Error::InvalidUtf8)?; + let commit_num = i + 2; + + writeln!(&mut msg, "")?; + writeln!(&mut msg)?; + writeln!(&mut msg, "{commit_msg}")?; + writeln!(&mut msg)?; + } + + Ok(msg.trim().to_string()) +} + +/// Return commits between the merge base and a head. +pub fn patch_commits<'a>( + repo: &'a git::raw::Repository, + base: &git::Oid, + head: &git::Oid, +) -> Result>, git::raw::Error> { + let mut commits = Vec::new(); + let mut revwalk = repo.revwalk()?; + revwalk.push_range(&format!("{base}..{head}"))?; + + for rev in revwalk { + let commit = repo.find_commit(rev?)?; + commits.push(commit); + } + Ok(commits) +} + +/// The message shown in the editor when creating a `Patch`. +fn create_display_message( + repo: &git::raw::Repository, + base: &git::Oid, + head: &git::Oid, +) -> Result { + let commits = patch_commits(repo, base, head)?; + if commits.is_empty() { + return Ok(PATCH_MSG.trim_start().to_string()); + } + + let summary = message_from_commits("patch", commits)?; + Ok(format!("{summary}\n{PATCH_MSG}")) +} + /// Get the Patch title and description from the command line arguments, or request it from the /// user. /// /// The user can bail out if an empty title is entered. -pub fn get_message( +pub fn get_create_message( message: term::patch::Message, - default_msg: &str, -) -> io::Result<(String, String)> { - let display_msg = default_msg.trim_end(); - - let message = message.get(&format!("{display_msg}\n{PATCH_MSG}"))?; + repo: &git::raw::Repository, + base: &git::Oid, + head: &git::Oid, +) -> Result<(String, String), Error> { + let display_msg = create_display_message(repo, base, head)?; + let message = message.get(&display_msg)?; let (title, description) = message.split_once('\n').unwrap_or((&message, "")); let (title, description) = (title.trim().to_string(), description.trim().to_string()); + if title.is_empty() { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "a patch title must be provided", + ) + .into()); + } + + Ok((title, description)) +} + +/// The message shown in the editor when editing a `Patch`. +fn edit_display_message(title: &str, description: &str) -> String { + format!("{}\n\n{}\n{PATCH_MSG}", title, description) + .trim_start() + .to_string() +} + +/// Get a patch edit message. +pub fn get_edit_message( + patch_message: term::patch::Message, + patch: &cob::patch::Patch, +) -> io::Result<(String, String)> { + let display_msg = edit_display_message(patch.title(), patch.description()); + let patch_message = patch_message.get(&display_msg)?; + let patch_message = patch_message.replace(PATCH_MSG.trim(), ""); // Delete help message. + + let (title, description) = patch_message + .split_once('\n') + .unwrap_or((&patch_message, "")); + let (title, description) = (title.trim().to_string(), description.trim().to_string()); + if title.is_empty() { return Err(io::Error::new( io::ErrorKind::InvalidInput, @@ -103,9 +227,34 @@ pub fn get_message( Ok((title, description)) } +/// The message shown in the editor when updating a `Patch`. +fn update_display_message( + repo: &git::raw::Repository, + last_rev_head: &git::Oid, + head: &git::Oid, +) -> Result { + if !repo.graph_descendant_of(**head, **last_rev_head)? { + return Ok(REVISION_MSG.trim_start().to_string()); + } + + let commits = patch_commits(repo, last_rev_head, head)?; + if commits.is_empty() { + return Ok(REVISION_MSG.trim_start().to_string()); + } + + let summary = message_from_commits("patch", commits)?; + Ok(format!("{summary}\n{REVISION_MSG}")) +} + /// Get a patch update message. -pub fn get_update_message(message: term::patch::Message) -> io::Result { - let message = message.get(REVISION_MSG)?; +pub fn get_update_message( + message: term::patch::Message, + repo: &git::raw::Repository, + latest: &patch::Revision, + head: &git::Oid, +) -> Result { + let display_msg = update_display_message(repo, &latest.head(), head)?; + let message = message.get(&display_msg)?; let message = message.trim(); Ok(message.to_owned()) @@ -152,34 +301,182 @@ pub fn print_commits_ahead_behind( #[cfg(test)] mod test { use super::*; + use radicle::git::refname; + use radicle::test::fixtures; + use std::path; + + fn commit( + repo: &git::raw::Repository, + branch: &git::RefStr, + parent: &git::Oid, + msg: &str, + ) -> git::Oid { + let sig = git::raw::Signature::new( + "anonymous", + "anonymous@radicle.xyz", + &git::raw::Time::new(0, 0), + ) + .unwrap(); + let head = repo.find_commit(**parent).unwrap(); + let tree = + git::write_tree(path::Path::new("README"), "Hello World!\n".as_bytes(), repo).unwrap(); + + let branch = git::refs::branch(branch); + let commit = git::commit(repo, &head, &branch, msg, &sig, &tree).unwrap(); + + commit.id().into() + } #[test] - fn test_get_message() { - let res = get_message( - Message::Text("title\n\ndescription".to_string()), - "default text", - ) - .unwrap(); - assert_eq!(("title".to_string(), "description".to_string()), res); + fn test_create_display_message() { + let tmpdir = tempfile::tempdir().unwrap(); + let (repo, commit_0) = fixtures::repository(&tmpdir); + let commit_0 = commit_0.into(); + let commit_1 = commit(&repo, &refname!("feature"), &commit_0, "commit 1"); + let commit_2 = commit(&repo, &refname!("feature"), &commit_1, "commit 2"); - let res = get_message( - Message::Text("title\ndescription\nsecond description".to_string()), - "default text", - ) - .unwrap(); + let res = create_display_message(&repo, &commit_0, &commit_0).unwrap(); assert_eq!( - ( - "title".to_string(), - "description\nsecond description".to_string() - ), + r#" + +"# + .trim_start(), res ); - let res = get_message( - Message::Text(" title \ndescription \n \n ".to_string()), - "default text", - ) - .unwrap(); - assert_eq!(("title".to_string(), "description".to_string()), res); + let res = create_display_message(&repo, &commit_0, &commit_1).unwrap(); + assert_eq!( + r#" +commit 1 + + +"# + .trim_start(), + res + ); + + let res = create_display_message(&repo, &commit_0, &commit_2).unwrap(); + assert_eq!( + r#" + + +commit 1 + + + +commit 2 + + +"# + .trim_start(), + res + ); + } + + #[test] + fn test_edit_display_message() { + let res = edit_display_message("title", "The patch description."); + assert_eq!( + r#" +title + +The patch description. + + +"# + .trim_start(), + res + ); + } + + #[test] + fn test_update_display_message() { + let tmpdir = tempfile::tempdir().unwrap(); + let (repo, commit_0) = fixtures::repository(&tmpdir); + let commit_0 = commit_0.into(); + + let commit_1 = commit(&repo, &refname!("feature"), &commit_0, "commit 1"); + let commit_2 = commit(&repo, &refname!("feature"), &commit_1, "commit 2"); + let commit_squashed = commit( + &repo, + &refname!("squashed-feature"), + &commit_0, + "commit squashed", + ); + + let res = update_display_message(&repo, &commit_1, &commit_1).unwrap(); + assert_eq!( + r#" + +"# + .trim_start(), + res + ); + + let res = update_display_message(&repo, &commit_1, &commit_2).unwrap(); + assert_eq!( + r#" +commit 2 + + +"# + .trim_start(), + res + ); + + let res = update_display_message(&repo, &commit_1, &commit_squashed).unwrap(); + assert_eq!( + r#" + +"# + .trim_start(), + res + ); } } diff --git a/radicle-remote-helper/src/push.rs b/radicle-remote-helper/src/push.rs index 2ec6f4e8..d3537774 100644 --- a/radicle-remote-helper/src/push.rs +++ b/radicle-remote-helper/src/push.rs @@ -65,6 +65,9 @@ pub enum Error { /// Patch COB error. #[error(transparent)] Patch(#[from] radicle::cob::patch::Error), + /// Patch edit message error. + #[error(transparent)] + PatchEdit(#[from] cli::patch::Error), /// Patch not found in store. #[error("patch `{0}` not found")] NotFound(patch::PatchId), @@ -254,11 +257,12 @@ fn patch_open( // not fail, since the reference will already exist with the correct OID. push_ref(src, &dst, false, working, stored.raw())?; - let mut patches = patch::Patches::open(stored)?; - let fallback = commit.message().unwrap_or_default(); - let (title, description) = cli::patch::get_message(msg, fallback)?; let (_, target) = stored.canonical_head()?; let base = stored.backend.merge_base(*target, commit.id())?; + let (title, description) = + cli::patch::get_create_message(msg, &stored.backend, &base.into(), &commit.id().into())?; + + let mut patches = patch::Patches::open(stored)?; let result = match patches.create( &title, &description, @@ -361,7 +365,13 @@ fn patch_update( if patch.revisions().any(|(_, r)| *r.head() == commit.id()) { return Ok(()); } - let message = cli::patch::get_update_message(msg)?; + let message = cli::patch::get_update_message( + msg, + &stored.backend, + patch.latest().1, + &commit.id().into(), + )?; + let (_, target) = stored.canonical_head()?; let base = stored.backend.merge_base(*target, commit.id())?; let revision = patch.update(message, base, commit.id(), signer)?;