cli: Clarify user facing patch message

Now logic for parsing and supporting user facing Patch messages is
shared for creation and editing, refactor and move supporting code to
terminal::patch.

This fixes a bug where the title may be incorrectly parsed, resulting in
it containing multiple lines.
This commit is contained in:
Slack Coder 2023-05-01 19:18:30 -05:00 committed by Alexis Sellier
parent 6bedb1256c
commit b5a00dfc4c
No known key found for this signature in database
4 changed files with 88 additions and 52 deletions

View File

@ -11,18 +11,6 @@ use crate::terminal::args::Error;
use super::Options;
pub const PATCH_MSG: &str = r#"
<!--
Please enter a patch message for your changes. An empty
message aborts the patch proposal.
The first line is the patch title. The patch description
follows, and must be separated with a blank line, just
like a commit message. Markdown is supported in the title
and description.
-->
"#;
/// 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> {

View File

@ -12,28 +12,6 @@ use crate::terminal as term;
use super::common::*;
use super::Options;
pub fn handle_patch_message(
message: term::patch::Message,
storage: &Repository,
head_branch: &git::raw::Branch,
) -> anyhow::Result<(String, String)> {
let head_oid = branch_oid(head_branch)?;
let head_commit = storage.backend.find_commit(*head_oid)?;
let commit_message = head_commit
.message()
.ok_or(anyhow!("commit summary is not valid UTF-8; aborting"))?;
let message = message.get(&format!("{commit_message}{PATCH_MSG}"))?;
let message = message.replace(PATCH_MSG.trim(), ""); // Delete help message.
let (title, description) = message.split_once("\n\n").unwrap_or((&message, ""));
let (title, description) = (title.trim(), description.trim());
if title.is_empty() {
anyhow::bail!("a patch title must be provided");
}
Ok((title.to_string(), description.to_owned()))
}
fn show_patch_commit_info(
storage: &Repository,
node_id: &NodeId,
@ -102,8 +80,13 @@ pub fn run(
// TODO: List matching working copy refs for all targets.
let (title, description) = handle_patch_message(message, storage, &head_branch)?;
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 signer = term::signer(profile)?;
let patch = if draft {

View File

@ -1,10 +1,11 @@
use super::common::*;
use super::*;
use radicle::cob::patch;
use radicle::prelude::*;
use radicle::storage::git::Repository;
use crate::terminal as term;
pub fn run(
repository: &Repository,
profile: &Profile,
@ -17,26 +18,14 @@ pub fn run(
anyhow::bail!("Patch `{patch_id}` not found");
};
let title = patch.title();
let description = patch.description();
let message = message.get(&format!("{title}\n\n{description}\n\n{PATCH_MSG}"))?;
let message = message.replace(PATCH_MSG.trim(), ""); // Delete help message.
let (title, description) = message.split_once("\n\n").unwrap_or((&message, ""));
let (title, description) = (title.trim(), description.trim());
let default_msg = term::patch::message(patch.title(), patch.description());
let (title, description) = term::patch::get_message(message, &default_msg)?;
if title.is_empty() {
anyhow::bail!("a patch title must be provided");
} else if title == patch.title() && description == patch.description() {
if title == patch.title() && description == patch.description() {
// Nothing to do
return Ok(());
}
patch.edit(
title.to_string(),
description.to_string(),
patch.target(),
&signer,
)?;
patch.edit(title, description, patch.target(), &signer)?;
Ok(())
}

View File

@ -43,6 +43,47 @@ impl Default for Message {
}
}
pub const PATCH_MSG: &str = r#"
<!--
Please enter a patch message for your changes. An empty
message aborts the patch proposal.
The first line is the patch title. The patch description
follows, and must be separated with a blank line, just
like a commit message. Markdown is supported in the title
and description.
-->
"#;
/// Combine the title and description fields to display to the user.
#[inline]
pub fn message(title: &str, description: &str) -> String {
format!("{title}\n\n{description}").trim().to_string()
}
/// 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(
message: term::patch::Message,
default_msg: &str,
) -> anyhow::Result<(String, String)> {
let display_msg = default_msg.trim_end();
let message = message.get(&format!("{display_msg}\n{PATCH_MSG}"))?;
let message = message.replace(PATCH_MSG.trim(), ""); // Delete help message.
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() {
anyhow::bail!("a patch title must be provided");
}
Ok((title, description))
}
/// List the given commits in a table.
pub fn list_commits(commits: &[git::raw::Commit]) -> anyhow::Result<()> {
let mut table = term::Table::default();
@ -80,3 +121,38 @@ pub fn print_commits_ahead_behind(
);
Ok(())
}
#[cfg(test)]
mod test {
use super::*;
#[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);
let res = get_message(
Message::Text("title\ndescription\nsecond description".to_string()),
"default text",
)
.unwrap();
assert_eq!(
(
"title".to_string(),
"description\nsecond description".to_string()
),
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);
}
}