diff --git a/README.md b/README.md index 98c9d42..3d0e7f5 100644 --- a/README.md +++ b/README.md @@ -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 diff --git a/bacon.toml b/bacon.toml index 31c1d7a..d14d88d 100644 --- a/bacon.toml +++ b/bacon.toml @@ -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" \ No newline at end of file diff --git a/src/commit/types/footer.rs b/src/commit/types/footer.rs index ff58cae..e87829b 100644 --- a/src/commit/types/footer.rs +++ b/src/commit/types/footer.rs @@ -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 } } diff --git a/src/commit/types/message.rs b/src/commit/types/message.rs index 7a89df1..81b9af6 100644 --- a/src/commit/types/message.rs +++ b/src/commit/types/message.rs @@ -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 { 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, diff --git a/src/commit/types/mod.rs b/src/commit/types/mod.rs index 8228840..8ea9725 100644 --- a/src/commit/types/mod.rs +++ b/src/commit/types/mod.rs @@ -18,3 +18,6 @@ pub use body::Body; mod message; pub use message::{CommitMessageError, ConventionalCommit}; + +mod references; +pub use references::References; diff --git a/src/commit/types/references.rs b/src/commit/types/references.rs new file mode 100644 index 0000000..abcf091 --- /dev/null +++ b/src/commit/types/references.rs @@ -0,0 +1,186 @@ +use super::Footer; + +#[repr(transparent)] +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct References(Vec); + +impl From for References +where + T: ToString, +{ + fn from(value: T) -> Self { + let references: Vec = 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 = 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); + } +} diff --git a/src/prompts/mock.rs b/src/prompts/mock.rs index 82de8ec..ee864fa 100644 --- a/src/prompts/mock.rs +++ b/src/prompts/mock.rs @@ -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 { + 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 { 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); diff --git a/src/prompts/prompter.rs b/src/prompts/prompter.rs index 523baad..5f69142 100644 --- a/src/prompts/prompter.rs +++ b/src/prompts/prompter.rs @@ -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; + /// Prompt the user to optionally add comma-separated ticket references + fn input_references(&self) -> Result; + /// Prompt the user to confirm applying the commit message fn confirm_apply(&self, message: &str) -> Result; @@ -91,6 +94,19 @@ impl Prompter for RealPrompts { } } + fn input_references(&self) -> Result { + 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 { loop { let answer = Text::new("Enter description (required):") diff --git a/src/prompts/workflow.rs b/src/prompts/workflow.rs index a2153f5..5e0496b 100644 --- a/src/prompts/workflow.rs +++ b/src/prompts/workflow.rs @@ -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 CommitWorkflow { 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 CommitWorkflow { self.prompts.input_breaking_change() } + /// Prompt user for references + fn references_input(&self) -> Result { + self.prompts.input_references() + } + /// Prompt user to optionally add a free-form body via an external editor fn body_input(&self) -> Result { self.prompts.input_body() @@ -129,6 +142,7 @@ impl CommitWorkflow { description: Description, breaking_change: BreakingChange, body: Body, + references: References, ) -> Result { // Format the message for preview let message = ConventionalCommit::format_preview( @@ -137,6 +151,7 @@ impl CommitWorkflow { &description, &breaking_change, &body, + &references, ); // Try to build the conventional commit (this validates the 72-char limit) @@ -146,6 +161,7 @@ impl CommitWorkflow { 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); diff --git a/tests/error_tests.rs b/tests/error_tests.rs index 0e9dc3a..5bd2599 100644 --- a/tests/error_tests.rs +++ b/tests/error_tests.rs @@ -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 = variants.iter().map(|e| e.clone()).collect(); + let cloned: Vec = variants.to_vec(); assert_eq!(variants.len(), cloned.len()); for (original, clone) in variants.iter().zip(cloned.iter()) { assert_eq!(original, clone);