feat(references): add ticket reference footers
Run checks and build archives / build (linux-x86_64) (push) Has been cancelled
Run checks and build archives / build (windows-x86_64) (push) Has been cancelled
Run checks and build archives / build (linux-aarch64) (push) Has been cancelled
Run checks and build archives / coverage-and-sonar (push) Successful in 5m29s

Refs: #4
This commit is contained in:
2026-06-14 16:24:45 +02:00
parent e6ac6890b2
commit bd4aaff3f3
10 changed files with 349 additions and 15 deletions
+2 -3
View File
@@ -38,9 +38,8 @@ gitea: none
## Features
- Interactive prompts for type, scope, breaking changes, and description
- All 11 commit types with descriptions (feat, fix, docs, style,
refactor, perf, test, build, ci, chore, revert)
- Interactive prompts for type, scope, breaking changes, ticket references, and description
- All 11 commit types with descriptions (feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert)
- Optional scope with validation
- 72-character first-line limit enforcement
- Preview before applying
+7
View File
@@ -38,6 +38,12 @@ command = [
]
need_stdout = true
[jobs.coverage]
command = [
"cargo", "tarpaulin", "--config", ".tarpaulin.local.toml", "--features", "test-utils"
]
need_stdout = true
[jobs.doc]
command = ["cargo", "doc", "--color", "always", "--no-deps"]
need_stdout = false
@@ -82,3 +88,4 @@ allow_warnings = true
[keybindings]
# alt-m = "job:my-job"
c = "job:clippy-all" # comment this to have 'c' run clippy on only the default target
v = "job:coverage"
+4 -2
View File
@@ -4,10 +4,12 @@ pub trait Footer {
fn as_footer(&self) -> String {
let default = format!("{}: {}", self.prefix(), self.note());
if default.chars().count() > 72 {
let mut footer = if default.chars().count() > 72 {
textwrap::wrap(&default, 71).join("\n ")
} else {
default
}
};
footer.push('\n');
footer
}
}
+27 -2
View File
@@ -1,4 +1,4 @@
use super::{Body, BreakingChange, CommitType, Description, Scope};
use super::{Body, BreakingChange, CommitType, Description, Footer, References, Scope};
use thiserror::Error;
/// Errors that can occur when creating a ConventionalCommit
@@ -23,6 +23,7 @@ pub struct ConventionalCommit {
description: Description,
breaking_change: BreakingChange,
body: Body,
references: References,
}
impl ConventionalCommit {
@@ -44,6 +45,7 @@ impl ConventionalCommit {
description: Description,
breaking_change: BreakingChange,
body: Body,
references: References,
) -> Result<Self, CommitMessageError> {
let commit = Self {
commit_type,
@@ -51,6 +53,7 @@ impl ConventionalCommit {
description,
breaking_change,
body,
references,
};
let len = commit.first_line_len();
if len > Self::FIRST_LINE_MAX_LENGTH {
@@ -92,6 +95,7 @@ impl ConventionalCommit {
&self.description,
&self.breaking_change,
&self.body,
&self.references,
)
}
@@ -106,14 +110,16 @@ impl ConventionalCommit {
description: &Description,
breaking_change: &BreakingChange,
body: &Body,
references: &References,
) -> String {
let scope = scope.header_segment();
let breaking_change_header = breaking_change.header_segment();
let breaking_change_footer = breaking_change.as_footer();
let refs_footer = references.as_footer();
format!(
r#"{commit_type}{scope}{breaking_change_header}: {description}
{}
{breaking_change_footer}"#,
{breaking_change_footer}{refs_footer}"#,
body.format()
)
.trim()
@@ -154,6 +160,7 @@ mod tests {
description,
breaking_change,
Body::default(),
References::default(),
)
.expect("test commit should have valid line length")
}
@@ -637,6 +644,7 @@ mod tests {
Description::parse(&desc_44).unwrap(),
BreakingChange::No,
Body::default(),
References::default(),
);
assert!(result.is_ok());
let commit = result.unwrap();
@@ -667,6 +675,7 @@ mod tests {
Description::parse(&desc_31).unwrap(),
BreakingChange::No,
Body::default(),
References::default(),
);
assert!(result.is_err());
assert_eq!(
@@ -691,6 +700,7 @@ mod tests {
Description::parse(&desc_40).unwrap(),
BreakingChange::No,
Body::default(),
References::default(),
);
assert!(result.is_err());
assert_eq!(
@@ -711,6 +721,7 @@ mod tests {
test_description("quick fix"),
BreakingChange::No,
Body::default(),
References::default(),
);
assert!(result.is_ok());
}
@@ -724,6 +735,7 @@ mod tests {
test_description("add feature"),
BreakingChange::No,
Body::default(),
References::default(),
);
assert!(result.is_ok());
}
@@ -750,6 +762,7 @@ mod tests {
test_description("test"),
BreakingChange::No,
Body::default(),
References::default(),
);
// Just verify it's a Result by using is_ok()
assert!(result.is_ok());
@@ -781,6 +794,7 @@ mod tests {
desc,
BreakingChange::No,
Body::default(),
References::default(),
);
// new() itself calls git_conventional::Commit::parse internally, so
// if this is Ok, SC-002 is satisfied for this case.
@@ -918,6 +932,7 @@ mod tests {
Description::parse(&desc_44).unwrap(),
BreakingChange::Yes,
Body::default(),
References::default(),
);
assert!(result.is_err());
assert_eq!(
@@ -940,6 +955,7 @@ mod tests {
test_description("quick fix"),
long_note.into(),
Body::default(),
References::default(),
);
assert!(result.is_ok());
}
@@ -959,6 +975,7 @@ mod tests {
&commit.description,
&BreakingChange::No,
&Body::default(),
&References::default(),
);
assert_eq!(preview, commit.format());
}
@@ -972,6 +989,7 @@ mod tests {
&test_description("drop legacy API"),
&"removes legacy endpoint".into(),
&Body::default(),
&References::default(),
);
assert_eq!(
preview,
@@ -988,6 +1006,7 @@ mod tests {
&test_description("drop Node 6"),
&"Node 6 is no longer supported".into(),
&Body::default(),
&References::default(),
);
assert_eq!(
preview,
@@ -1093,6 +1112,7 @@ mod tests {
test_description("add feature"),
BreakingChange::No,
Body::from("This explains the change."),
References::default(),
)
.unwrap();
assert_eq!(
@@ -1110,6 +1130,7 @@ mod tests {
test_description("handle null response"),
BreakingChange::No,
Body::from("Null responses were previously unhandled."),
References::default(),
)
.unwrap();
assert_eq!(
@@ -1127,6 +1148,7 @@ mod tests {
test_description("update README"),
BreakingChange::No,
Body::from("First paragraph.\n\nSecond paragraph."),
References::default(),
)
.unwrap();
assert_eq!(
@@ -1146,6 +1168,7 @@ mod tests {
test_description("drop legacy API"),
"removes legacy endpoint".into(),
Body::from("The endpoint was deprecated in v2."),
References::default(),
)
.unwrap();
assert_eq!(
@@ -1163,6 +1186,7 @@ mod tests {
&test_description("add feature"),
&BreakingChange::No,
&Body::from("This explains the change."),
&References::default(),
);
assert_eq!(preview, "feat: add feature\n\nThis explains the change.");
}
@@ -1178,6 +1202,7 @@ mod tests {
&test_description("drop old API"),
&"old API removed".into(),
&Body::from("Migration guide: see CHANGELOG."),
&References::default(),
);
assert_eq!(
preview,
+3
View File
@@ -18,3 +18,6 @@ pub use body::Body;
mod message;
pub use message::{CommitMessageError, ConventionalCommit};
mod references;
pub use references::References;
+186
View File
@@ -0,0 +1,186 @@
use super::Footer;
#[repr(transparent)]
#[derive(Debug, Default, Clone, PartialEq, Eq)]
pub struct References(Vec<String>);
impl<T> From<T> for References
where
T: ToString,
{
fn from(value: T) -> Self {
let references: Vec<String> = value
.to_string()
.split(",")
.flat_map(|e| match e.trim() {
"" => None,
e => Some(e.to_string()),
})
.collect();
Self(references)
}
}
impl Footer for References {
fn prefix(&self) -> &str {
"Refs: "
}
fn note(&self) -> &str {
""
}
fn as_footer(&self) -> String {
if self.0.is_empty() {
String::new()
} else {
let footers: Vec<String> = self
.0
.iter()
.map(|r| format!("{}{r}\n", self.prefix()))
.collect();
footers.join("")
}
}
}
#[cfg(test)]
mod tests {
use super::*;
/// Default is empty
#[test]
fn default_is_empty() {
let refs = References::default();
assert!(refs.0.is_empty());
}
/// Empty input produces empty references
#[test]
fn from_empty_string() {
assert_eq!(References::from(""), References::default());
}
/// Whitespace-only input produces empty references
#[test]
fn from_whitespace_only() {
assert_eq!(References::from(" "), References::default());
}
/// Single reference without commas
#[test]
fn from_single_reference() {
let refs = References::from("#123");
assert_eq!(refs.0, vec!["#123".to_string()]);
}
/// Comma-separated references are split and trimmed
#[test]
fn from_comma_separated() {
let refs = References::from("#123, #456");
assert_eq!(refs.0, vec!["#123".to_string(), "#456".to_string()]);
}
/// Leading whitespace around references is trimmed
#[test]
fn from_trims_leading_whitespace() {
let refs = References::from(" #123, #456");
assert_eq!(refs.0, vec!["#123".to_string(), "#456".to_string()]);
}
/// Trailing whitespace around references is trimmed
#[test]
fn from_trims_trailing_whitespace() {
let refs = References::from("#123 , #456 ");
assert_eq!(refs.0, vec!["#123".to_string(), "#456".to_string()]);
}
/// Empty segments from consecutive commas are filtered out
#[test]
fn from_filters_empty_segments() {
let refs = References::from("#123,,, #456");
assert_eq!(refs.0, vec!["#123".to_string(), "#456".to_string()]);
}
/// From works with owned String
#[test]
fn from_owned_string() {
let input = "#123, #456".to_string();
let refs = References::from(input);
assert_eq!(refs.0, vec!["#123".to_string(), "#456".to_string()]);
}
/// as_footer returns empty string for empty references
#[test]
fn as_footer_empty() {
let refs = References::default();
assert_eq!(refs.as_footer(), "");
}
/// as_footer returns single line for one reference
#[test]
fn as_footer_single() {
let refs = References::from("#123");
assert_eq!(refs.as_footer(), "Refs: #123\n");
}
/// as_footer returns multiple lines for multiple references
#[test]
fn as_footer_multiple() {
let refs = References::from("#123, #456");
assert_eq!(refs.as_footer(), "Refs: #123\nRefs: #456\n");
}
/// as_footer handles Jira-style references
#[test]
fn as_footer_jira_style() {
let refs = References::from("OPS-456, PROJ-789");
assert_eq!(refs.as_footer(), "Refs: OPS-456\nRefs: PROJ-789\n");
}
/// Footer trait prefix returns correct value
#[test]
fn footer_prefix() {
let refs = References::default();
assert_eq!(refs.prefix(), "Refs: ");
}
/// Footer trait note returns empty string
#[test]
fn footer_note() {
let refs = References::default();
assert_eq!(refs.note(), "");
}
/// Clone produces equal value
#[test]
fn clone_equality() {
let refs = References::from("#123, #456");
let cloned = refs.clone();
assert_eq!(refs, cloned);
}
/// Debug output is available
#[test]
fn debug_output() {
let refs = References::from("#123");
let debug = format!("{:?}", refs);
assert!(debug.contains("References"));
}
/// Different references are not equal
#[test]
fn inequality_different_refs() {
let a = References::from("#123");
let b = References::from("#456");
assert_ne!(a, b);
}
/// Empty vs non-empty are not equal
#[test]
fn inequality_empty_vs_non_empty() {
let empty = References::default();
let non_empty = References::from("#123");
assert_ne!(empty, non_empty);
}
}
+59 -1
View File
@@ -8,7 +8,7 @@
use std::sync::{Arc, Mutex};
use crate::{
commit::types::{Body, BreakingChange, CommitType, Description, Scope},
commit::types::{Body, BreakingChange, CommitType, Description, References, Scope},
error::Error,
prompts::prompter::Prompter,
};
@@ -20,6 +20,7 @@ enum MockResponse {
Scope(Scope),
Description(Description),
BreakingChange(BreakingChange),
References(References),
Body(Body),
Confirm(bool),
Error(Error),
@@ -81,6 +82,15 @@ impl MockPrompts {
self
}
/// Configure the mock to return specific references
pub fn with_references(self, references: References) -> Self {
self.responses
.lock()
.unwrap()
.push(MockResponse::References(references));
self
}
/// Configure the mock to return a specific body response
pub fn with_body(self, body: Body) -> Self {
self.responses
@@ -140,6 +150,14 @@ impl MockPrompts {
.contains(&"input_breaking_change".to_string())
}
/// Check if input_references was called
pub fn was_references_called(&self) -> bool {
self.prompts_called
.lock()
.unwrap()
.contains(&"input_references".to_string())
}
/// Check if confirm_apply was called
pub fn was_confirm_called(&self) -> bool {
self.prompts_called
@@ -207,6 +225,18 @@ impl Prompter for MockPrompts {
}
}
fn input_references(&self) -> Result<References, Error> {
self.prompts_called
.lock()
.unwrap()
.push("input_references".to_string());
match self.responses.lock().unwrap().remove(0) {
MockResponse::References(r) => Ok(r),
MockResponse::Error(e) => Err(e),
_ => panic!("MockPrompts: Expected References response, got different type"),
}
}
fn input_body(&self) -> Result<Body, Error> {
self.prompts_called
.lock()
@@ -336,6 +366,34 @@ mod tests {
assert!(mock.emitted_messages().is_empty());
}
#[test]
fn mock_input_references() {
let refs = References::from("#123, #456");
let mock = MockPrompts::new().with_references(refs.clone());
let result = mock.input_references();
assert!(result.is_ok());
assert_eq!(result.unwrap(), refs);
assert!(mock.was_references_called());
}
#[test]
fn mock_input_references_default() {
let mock = MockPrompts::new().with_references(References::default());
let result = mock.input_references();
assert!(result.is_ok());
assert_eq!(result.unwrap(), References::default());
assert!(mock.was_references_called());
}
#[test]
fn mock_input_references_error() {
let mock = MockPrompts::new().with_error(Error::Cancelled);
let result = mock.input_references();
assert!(result.is_err());
assert!(matches!(result.unwrap_err(), Error::Cancelled));
assert!(mock.was_references_called());
}
#[test]
fn mock_input_breaking_change_no() {
let mock = MockPrompts::new().with_breaking_change(BreakingChange::No);
+17 -1
View File
@@ -9,7 +9,7 @@ use inquire::{Confirm, Text};
use unicode_width::UnicodeWidthStr;
use crate::{
commit::types::{Body, BreakingChange, CommitType, Description, Scope},
commit::types::{Body, BreakingChange, CommitType, Description, References, Scope},
error::Error,
};
@@ -33,6 +33,9 @@ pub trait Prompter {
/// 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 optionally add comma-separated ticket references
fn input_references(&self) -> Result<References, Error>;
/// Prompt the user to confirm applying the commit message
fn confirm_apply(&self, message: &str) -> Result<bool, Error>;
@@ -91,6 +94,19 @@ impl Prompter for RealPrompts {
}
}
fn input_references(&self) -> Result<References, Error> {
let answer = inquire::Text::new("Enter comma-separated references (optional):")
.with_help_message("References are optional. If provided, will become footer(s) in the commit message. References must be comma-separated.")
.with_placeholder("Leave empty if no references")
.prompt_skippable()
.map_err(|_| Error::Cancelled)?;
match answer {
None => Ok(References::default()),
Some(s) if s.trim().is_empty() => Ok(References::default()),
Some(s) => Ok(References::from(s)),
}
}
fn input_description(&self) -> Result<Description, Error> {
loop {
let answer = Text::new("Enter description (required):")
+42 -4
View File
@@ -6,7 +6,7 @@
use crate::{
commit::types::{
Body, BreakingChange, CommitMessageError, CommitType, ConventionalCommit, Description,
Scope,
References, Scope,
},
error::Error,
jj::JjExecutor,
@@ -65,8 +65,16 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
let scope = self.scope_input()?;
let description = self.description_input()?;
let breaking_change = self.breaking_change_input()?;
let references = self.references_input()?;
let body = self.body_input()?;
match self.preview_and_confirm(commit_type, scope, description, breaking_change, body) {
match self.preview_and_confirm(
commit_type,
scope,
description,
breaking_change,
body,
references,
) {
Ok(conventional_commit) => {
self.executor
.describe(revset, &conventional_commit.to_string())
@@ -113,6 +121,11 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
self.prompts.input_breaking_change()
}
/// Prompt user for references
fn references_input(&self) -> Result<References, Error> {
self.prompts.input_references()
}
/// Prompt user to optionally add a free-form body via an external editor
fn body_input(&self) -> Result<Body, Error> {
self.prompts.input_body()
@@ -129,6 +142,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
description: Description,
breaking_change: BreakingChange,
body: Body,
references: References,
) -> Result<ConventionalCommit, Error> {
// Format the message for preview
let message = ConventionalCommit::format_preview(
@@ -137,6 +151,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
&description,
&breaking_change,
&body,
&references,
);
// Try to build the conventional commit (this validates the 72-char limit)
@@ -146,6 +161,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
description.clone(),
breaking_change,
body,
references,
) {
Ok(cc) => cc,
Err(CommitMessageError::FirstLineTooLong { actual, max }) => {
@@ -276,8 +292,15 @@ mod tests {
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, body);
let references = References::default();
let result = workflow.preview_and_confirm(
commit_type,
scope,
description,
breaking_change,
body,
references,
);
assert!(result.is_ok());
}
@@ -322,6 +345,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("add new feature").unwrap())
.with_breaking_change(BreakingChange::Yes)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -355,6 +379,7 @@ mod tests {
.with_scope(Scope::parse("api").unwrap())
.with_description(Description::parse("fix bug").unwrap())
.with_breaking_change(BreakingChange::No)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(false); // User cancels at confirmation
@@ -379,11 +404,13 @@ 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_references(References::default())
.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_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -478,6 +505,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("test").unwrap())
.with_breaking_change(BreakingChange::Yes)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -501,6 +529,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("test").unwrap())
.with_breaking_change(BreakingChange::Yes)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -519,6 +548,7 @@ mod tests {
.with_scope(Scope::parse("api").unwrap())
.with_description(Description::parse("test").unwrap())
.with_breaking_change(BreakingChange::No)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -566,6 +596,7 @@ mod tests {
Description::parse("remove old API").unwrap(),
BreakingChange::Yes,
Body::default(),
References::default(),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
@@ -593,6 +624,7 @@ mod tests {
Description::parse("drop legacy API").unwrap(),
breaking_change,
Body::default(),
References::default(),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
@@ -623,6 +655,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("remove old API").unwrap())
.with_breaking_change(BreakingChange::Yes)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -665,6 +698,7 @@ mod tests {
Description::parse("add feature").unwrap(),
BreakingChange::No,
Body::from("This explains the change."),
References::default(),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
@@ -692,6 +726,7 @@ mod tests {
Description::parse("drop legacy API").unwrap(),
"removes legacy endpoint".into(),
Body::from("The endpoint was deprecated in v2."),
References::default(),
);
assert!(result.is_ok(), "expected Ok, got: {:?}", result);
@@ -718,6 +753,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("add feature").unwrap())
.with_breaking_change(BreakingChange::No)
.with_references(References::default())
.with_body(Body::from("This explains the change."))
.with_confirm(true);
@@ -750,6 +786,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("fix crash").unwrap())
.with_breaking_change(BreakingChange::No)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
@@ -803,6 +840,7 @@ mod tests {
.with_scope(Scope::empty())
.with_description(Description::parse("add feature").unwrap())
.with_breaking_change(BreakingChange::No)
.with_references(References::default())
.with_body(Body::default())
.with_confirm(true);
+2 -2
View File
@@ -170,7 +170,7 @@ fn test_from_poison_error() {
/// Test from_revset_evaluation_error constructs RevsetResolutionError
#[test]
fn test_from_revset_evaluation_error() {
let underlying = std::io::Error::new(std::io::ErrorKind::Other, "store failure");
let underlying = std::io::Error::other("store failure");
let eval_err = jj_lib::revset::RevsetEvaluationError::Other(Box::new(underlying));
let error = Error::from_revset_evaluation_error("@", eval_err);
assert!(matches!(error, Error::RevsetResolutionError { .. }));
@@ -281,7 +281,7 @@ fn test_error_matching_all_variants() {
}
// Verify all variants can be cloned
let cloned: Vec<Error> = variants.iter().map(|e| e.clone()).collect();
let cloned: Vec<Error> = variants.to_vec();
assert_eq!(variants.len(), cloned.len());
for (original, clone) in variants.iter().zip(cloned.iter()) {
assert_eq!(original, clone);