feat: edit body for commit messages
This commit is contained in:
+24
-1
@@ -8,7 +8,7 @@
|
||||
use std::sync::{Arc, Mutex};
|
||||
|
||||
use crate::{
|
||||
commit::types::{BreakingChange, CommitType, Description, Scope},
|
||||
commit::types::{Body, BreakingChange, CommitType, Description, Scope},
|
||||
error::Error,
|
||||
prompts::prompter::Prompter,
|
||||
};
|
||||
@@ -20,6 +20,7 @@ enum MockResponse {
|
||||
Scope(Scope),
|
||||
Description(Description),
|
||||
BreakingChange(BreakingChange),
|
||||
Body(Body),
|
||||
Confirm(bool),
|
||||
Error(Error),
|
||||
}
|
||||
@@ -80,6 +81,15 @@ impl MockPrompts {
|
||||
self
|
||||
}
|
||||
|
||||
/// Configure the mock to return a specific body response
|
||||
pub fn with_body(self, body: Body) -> Self {
|
||||
self.responses
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push(MockResponse::Body(body));
|
||||
self
|
||||
}
|
||||
|
||||
/// Configure the mock to return a specific confirmation response
|
||||
pub fn with_confirm(self, confirm: bool) -> Self {
|
||||
self.responses
|
||||
@@ -197,6 +207,19 @@ impl Prompter for MockPrompts {
|
||||
}
|
||||
}
|
||||
|
||||
fn input_body(&self) -> Result<Body, Error> {
|
||||
self.prompts_called
|
||||
.lock()
|
||||
.unwrap()
|
||||
.push("input_body".to_string());
|
||||
|
||||
match self.responses.lock().unwrap().remove(0) {
|
||||
MockResponse::Body(body) => Ok(body),
|
||||
MockResponse::Error(e) => Err(e),
|
||||
_ => panic!("MockPrompts: Expected Body response, got different type"),
|
||||
}
|
||||
}
|
||||
|
||||
fn confirm_apply(&self, _message: &str) -> Result<bool, Error> {
|
||||
self.prompts_called
|
||||
.lock()
|
||||
|
||||
+36
-1
@@ -9,7 +9,7 @@ use inquire::{Confirm, Text};
|
||||
use unicode_width::UnicodeWidthStr;
|
||||
|
||||
use crate::{
|
||||
commit::types::{BreakingChange, CommitType, Description, Scope},
|
||||
commit::types::{Body, BreakingChange, CommitType, Description, Scope},
|
||||
error::Error,
|
||||
};
|
||||
|
||||
@@ -30,6 +30,9 @@ pub trait Prompter: Send + Sync {
|
||||
/// Prompt the user for breaking change
|
||||
fn input_breaking_change(&self) -> Result<BreakingChange, Error>;
|
||||
|
||||
/// Prompt the user to optionally add a free-form body via an external editor
|
||||
fn input_body(&self) -> Result<Body, Error>;
|
||||
|
||||
/// Prompt the user to confirm applying the commit message
|
||||
fn confirm_apply(&self, message: &str) -> Result<bool, Error>;
|
||||
|
||||
@@ -176,6 +179,38 @@ impl Prompter for RealPrompts {
|
||||
Ok(trimmed.into())
|
||||
}
|
||||
|
||||
fn input_body(&self) -> Result<Body, Error> {
|
||||
use inquire::Editor;
|
||||
|
||||
let wants_body = Confirm::new("Add a body?")
|
||||
.with_default(false)
|
||||
.prompt()
|
||||
.map_err(|_| Error::Cancelled)?;
|
||||
|
||||
if !wants_body {
|
||||
return Ok(Body::default());
|
||||
}
|
||||
|
||||
let template = "\
|
||||
JJ: Body (optional). Markdown is supported.\n\
|
||||
JJ: Wrap prose lines at 72 characters where possible.\n\
|
||||
JJ: Lines starting with \"JJ:\" will be removed.\n";
|
||||
|
||||
let raw = Editor::new("Body:")
|
||||
.with_predefined_text(template)
|
||||
.with_file_extension(".md")
|
||||
.prompt()
|
||||
.map_err(|_| Error::Cancelled)?;
|
||||
|
||||
let stripped: String = raw
|
||||
.lines()
|
||||
.filter(|line| !line.starts_with("JJ:"))
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
|
||||
Ok(Body::from(stripped))
|
||||
}
|
||||
|
||||
fn confirm_apply(&self, message: &str) -> Result<bool, Error> {
|
||||
use inquire::Confirm;
|
||||
|
||||
|
||||
+157
-5
@@ -5,7 +5,8 @@
|
||||
|
||||
use crate::{
|
||||
commit::types::{
|
||||
BreakingChange, CommitMessageError, CommitType, ConventionalCommit, Description, Scope,
|
||||
Body, BreakingChange, CommitMessageError, CommitType, ConventionalCommit, Description,
|
||||
Scope,
|
||||
},
|
||||
error::Error,
|
||||
jj::JjExecutor,
|
||||
@@ -62,8 +63,9 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
|
||||
let scope = self.scope_input().await?;
|
||||
let description = self.description_input().await?;
|
||||
let breaking_change = self.breaking_change_input().await?;
|
||||
let body = self.body_input().await?;
|
||||
match self
|
||||
.preview_and_confirm(commit_type, scope, description, breaking_change)
|
||||
.preview_and_confirm(commit_type, scope, description, breaking_change, body)
|
||||
.await
|
||||
{
|
||||
Ok(conventional_commit) => {
|
||||
@@ -112,6 +114,11 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
|
||||
self.prompts.input_breaking_change()
|
||||
}
|
||||
|
||||
/// Prompt user to optionally add a free-form body via an external editor
|
||||
async fn body_input(&self) -> Result<Body, Error> {
|
||||
self.prompts.input_body()
|
||||
}
|
||||
|
||||
/// Preview the formatted conventional commit message and get user confirmation
|
||||
///
|
||||
/// This method also validates that the complete first line
|
||||
@@ -122,10 +129,16 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
|
||||
scope: Scope,
|
||||
description: Description,
|
||||
breaking_change: BreakingChange,
|
||||
body: Body,
|
||||
) -> Result<ConventionalCommit, Error> {
|
||||
// Format the message for preview
|
||||
let message =
|
||||
ConventionalCommit::format_preview(commit_type, &scope, &description, &breaking_change);
|
||||
let message = ConventionalCommit::format_preview(
|
||||
commit_type,
|
||||
&scope,
|
||||
&description,
|
||||
&breaking_change,
|
||||
&body,
|
||||
);
|
||||
|
||||
// Try to build the conventional commit (this validates the 72-char limit)
|
||||
let conventional_commit: ConventionalCommit = match ConventionalCommit::new(
|
||||
@@ -133,6 +146,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
|
||||
scope.clone(),
|
||||
description.clone(),
|
||||
breaking_change,
|
||||
body,
|
||||
) {
|
||||
Ok(cc) => cc,
|
||||
Err(CommitMessageError::FirstLineTooLong { actual, max }) => {
|
||||
@@ -258,8 +272,9 @@ mod tests {
|
||||
let scope = Scope::empty();
|
||||
let description = Description::parse("test description").unwrap();
|
||||
let breaking_change = BreakingChange::No;
|
||||
let body = Body::default();
|
||||
let result = workflow
|
||||
.preview_and_confirm(commit_type, scope, description, breaking_change)
|
||||
.preview_and_confirm(commit_type, scope, description, breaking_change, body)
|
||||
.await;
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
@@ -305,6 +320,7 @@ mod tests {
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("add new feature").unwrap())
|
||||
.with_breaking_change(BreakingChange::Yes)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
// Create workflow with both mocks
|
||||
@@ -337,6 +353,7 @@ mod tests {
|
||||
.with_scope(Scope::parse("api").unwrap())
|
||||
.with_description(Description::parse("fix bug").unwrap())
|
||||
.with_breaking_change(BreakingChange::No)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(false); // User cancels at confirmation
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
|
||||
@@ -360,10 +377,12 @@ mod tests {
|
||||
.with_scope(Scope::parse("very-long-scope-name").unwrap())
|
||||
.with_description(Description::parse("a".repeat(45)).unwrap())
|
||||
.with_breaking_change(BreakingChange::No)
|
||||
.with_body(Body::default())
|
||||
// Second iteration: short enough to succeed
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("short description").unwrap())
|
||||
.with_breaking_change(BreakingChange::No)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
// Clone before moving into workflow so we can inspect emitted messages after
|
||||
@@ -457,6 +476,7 @@ mod tests {
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("test").unwrap())
|
||||
.with_breaking_change(BreakingChange::Yes)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(
|
||||
@@ -479,6 +499,7 @@ mod tests {
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("test").unwrap())
|
||||
.with_breaking_change(BreakingChange::Yes)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(
|
||||
@@ -496,6 +517,7 @@ mod tests {
|
||||
.with_scope(Scope::parse("api").unwrap())
|
||||
.with_description(Description::parse("test").unwrap())
|
||||
.with_breaking_change(BreakingChange::No)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(
|
||||
@@ -542,6 +564,7 @@ mod tests {
|
||||
Scope::empty(),
|
||||
Description::parse("remove old API").unwrap(),
|
||||
BreakingChange::Yes,
|
||||
Body::default(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -570,6 +593,7 @@ mod tests {
|
||||
Scope::empty(),
|
||||
Description::parse("drop legacy API").unwrap(),
|
||||
breaking_change,
|
||||
Body::default(),
|
||||
)
|
||||
.await;
|
||||
|
||||
@@ -601,6 +625,7 @@ mod tests {
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("remove old API").unwrap())
|
||||
.with_breaking_change(BreakingChange::Yes)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
|
||||
@@ -620,4 +645,131 @@ mod tests {
|
||||
messages[0],
|
||||
);
|
||||
}
|
||||
|
||||
// --- Body tests ---
|
||||
// preview_and_confirm() tests compile now but will fail until the Body::default()
|
||||
// at line 138 of preview_and_confirm() is replaced with the `body` parameter.
|
||||
// The full_workflow_* tests additionally require MockPrompts::with_body().
|
||||
|
||||
/// preview_and_confirm must forward the body to ConventionalCommit::new()
|
||||
///
|
||||
/// Currently the implementation passes Body::default() instead of the
|
||||
/// received body, so this test will fail until that is fixed.
|
||||
#[tokio::test]
|
||||
async fn preview_and_confirm_forwards_body() {
|
||||
let mock_executor = MockJjExecutor::new();
|
||||
let mock_prompts = MockPrompts::new().with_confirm(true);
|
||||
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
|
||||
|
||||
let result = workflow
|
||||
.preview_and_confirm(
|
||||
CommitType::Feat,
|
||||
Scope::empty(),
|
||||
Description::parse("add feature").unwrap(),
|
||||
BreakingChange::No,
|
||||
Body::from("This explains the change."),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
|
||||
assert!(
|
||||
result
|
||||
.unwrap()
|
||||
.to_string()
|
||||
.contains("This explains the change."),
|
||||
"body must appear in the commit message"
|
||||
);
|
||||
}
|
||||
|
||||
/// preview_and_confirm must forward the body even when a breaking change is present
|
||||
///
|
||||
/// Expected format: "type!: desc\n\nbody\n\nBREAKING CHANGE: note"
|
||||
#[tokio::test]
|
||||
async fn preview_and_confirm_forwards_body_with_breaking_change() {
|
||||
let mock_executor = MockJjExecutor::new();
|
||||
let mock_prompts = MockPrompts::new().with_confirm(true);
|
||||
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
|
||||
|
||||
let result = workflow
|
||||
.preview_and_confirm(
|
||||
CommitType::Feat,
|
||||
Scope::empty(),
|
||||
Description::parse("drop legacy API").unwrap(),
|
||||
"removes legacy endpoint".into(),
|
||||
Body::from("The endpoint was deprecated in v2."),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
|
||||
let message = result.unwrap().to_string();
|
||||
assert!(
|
||||
message.contains("The endpoint was deprecated in v2."),
|
||||
"body must appear in the commit message, got: {message:?}"
|
||||
);
|
||||
assert!(
|
||||
message.contains("BREAKING CHANGE: removes legacy endpoint"),
|
||||
"breaking change footer must still be present, got: {message:?}"
|
||||
);
|
||||
}
|
||||
|
||||
/// The full run() workflow must collect a body and include it in the
|
||||
/// described commit.
|
||||
///
|
||||
/// Requires MockPrompts::with_body() and run() to call body_input().
|
||||
#[tokio::test]
|
||||
async fn full_workflow_describes_commit_with_body() {
|
||||
let mock_executor = MockJjExecutor::new().with_is_repo_response(Ok(true));
|
||||
let mock_prompts = MockPrompts::new()
|
||||
.with_commit_type(CommitType::Feat)
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("add feature").unwrap())
|
||||
.with_breaking_change(BreakingChange::No)
|
||||
.with_body(Body::from("This explains the change."))
|
||||
.with_confirm(true);
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
|
||||
let result: Result<(), Error> = workflow.run().await;
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"expected workflow to succeed, got: {:?}",
|
||||
result
|
||||
);
|
||||
|
||||
let messages = workflow.executor.describe_messages();
|
||||
assert_eq!(messages.len(), 1, "expected exactly one describe() call");
|
||||
assert!(
|
||||
messages[0].contains("This explains the change."),
|
||||
"body must appear in the described commit, got: {:?}",
|
||||
messages[0]
|
||||
);
|
||||
}
|
||||
|
||||
/// run() must still work correctly when the user declines to add a body
|
||||
///
|
||||
/// Requires MockPrompts::with_body() returning Body::default().
|
||||
#[tokio::test]
|
||||
async fn full_workflow_with_no_body_succeeds() {
|
||||
let mock_executor = MockJjExecutor::new().with_is_repo_response(Ok(true));
|
||||
let mock_prompts = MockPrompts::new()
|
||||
.with_commit_type(CommitType::Fix)
|
||||
.with_scope(Scope::empty())
|
||||
.with_description(Description::parse("fix crash").unwrap())
|
||||
.with_breaking_change(BreakingChange::No)
|
||||
.with_body(Body::default())
|
||||
.with_confirm(true);
|
||||
|
||||
let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
|
||||
let result: Result<(), Error> = workflow.run().await;
|
||||
|
||||
assert!(
|
||||
result.is_ok(),
|
||||
"expected workflow to succeed, got: {:?}",
|
||||
result
|
||||
);
|
||||
|
||||
let messages = workflow.executor.describe_messages();
|
||||
assert_eq!(messages.len(), 1);
|
||||
assert_eq!(messages[0], "fix: fix crash");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user