From 520a54aae464d7c7131cb7051d1589b0583d4e7b Mon Sep 17 00:00:00 2001 From: Lucien Cartier-Tilet Date: Mon, 9 Mar 2026 22:57:35 +0100 Subject: [PATCH] feat: implement breaking change input --- .tarpaulin.ci.toml | 2 +- .tarpaulin.local.toml | 2 +- Cargo.lock | 168 ++++++------ Cargo.toml | 1 + src/commit/types/breaking_change.rs | 125 +++++++++ src/commit/types/footer.rs | 16 ++ src/commit/types/message.rs | 386 ++++++++++++++++++++++++++-- src/commit/types/mod.rs | 6 + src/lib.rs | 4 +- src/prompts/mock.rs | 93 ++++++- src/prompts/prompter.rs | 114 +++++++- src/prompts/workflow.rs | 52 ++-- tests/cli.rs | 4 +- 13 files changed, 836 insertions(+), 137 deletions(-) create mode 100644 src/commit/types/breaking_change.rs create mode 100644 src/commit/types/footer.rs diff --git a/.tarpaulin.ci.toml b/.tarpaulin.ci.toml index f72d618..2325f5f 100644 --- a/.tarpaulin.ci.toml +++ b/.tarpaulin.ci.toml @@ -3,4 +3,4 @@ out = ["Xml"] target-dir = "coverage" output-dir = "coverage" fail-under = 60 -exclude-files = ["target/*", "private/*"] +exclude-files = ["target/*", "private/*", "tests/*"] diff --git a/.tarpaulin.local.toml b/.tarpaulin.local.toml index 97443d2..a5ad3e6 100644 --- a/.tarpaulin.local.toml +++ b/.tarpaulin.local.toml @@ -4,4 +4,4 @@ skip-clean = true target-dir = "coverage" output-dir = "coverage" fail-under = 60 -exclude-files = ["target/*", "private/*"] +exclude-files = ["target/*", "private/*", "tests/*"] diff --git a/Cargo.lock b/Cargo.lock index 2820dcb..eaf6a74 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -28,9 +28,9 @@ dependencies = [ [[package]] name = "anstream" -version = "0.6.21" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "43d5b281e737544384e969a5ccad3f1cdd24b48086a0fc1b2a5262a26b8f4f4a" +checksum = "824a212faf96e9acacdbd09febd34438f8f711fb84e09a8916013cd7815ca28d" dependencies = [ "anstyle", "anstyle-parse", @@ -43,15 +43,15 @@ dependencies = [ [[package]] name = "anstyle" -version = "1.0.13" +version = "1.0.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5192cca8006f1fd4f7237516f40fa183bb07f8fbdfedaa0036de5ea9b0b45e78" +checksum = "940b3a0ca603d1eade50a4846a2afffd5ef57a9feac2c0e2ec2e14f9ead76000" [[package]] name = "anstyle-parse" -version = "0.2.7" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "4e7644824f0aa2c7b9384579234ef10eb7efb6a0deb83f9630a49594dd9c15c2" +checksum = "52ce7f38b242319f7cabaa6813055467063ecdc9d355bbb4ce0c68908cd8130e" dependencies = [ "utf8parse", ] @@ -99,9 +99,9 @@ checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" [[package]] name = "assert_cmd" -version = "2.1.2" +version = "2.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9c5bcfa8749ac45dd12cb11055aeeb6b27a3895560d60d71e3c23bf979e60514" +checksum = "9a686bbee5efb88a82df0621b236e74d925f470e5445d3220a5648b892ec99c9" dependencies = [ "anstyle", "bstr", @@ -152,9 +152,9 @@ checksum = "3a8241f3ebb85c056b509d4327ad0358fbbba6ffb340bf388f26350aeda225b1" [[package]] name = "bitflags" -version = "2.10.0" +version = "2.11.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "812e12b5285cc515a9c72a5c1d3b6d46a19dac5acfef5265968c166106e31dd3" +checksum = "843867be96c8daad0d758b57df9392b6d8d271134fce549de6ce169ff98a92af" [[package]] name = "blake2" @@ -244,9 +244,9 @@ dependencies = [ [[package]] name = "clap" -version = "4.5.57" +version = "4.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6899ea499e3fb9305a65d5ebf6e3d2248c5fab291f300ad0a704fbe142eae31a" +checksum = "b193af5b67834b676abd72466a96c1024e6a6ad978a1f484bd90b85c94041351" dependencies = [ "clap_builder", "clap_derive", @@ -254,9 +254,9 @@ dependencies = [ [[package]] name = "clap_builder" -version = "4.5.57" +version = "4.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "7b12c8b680195a62a8364d16b8447b01b6c2c8f9aaf68bee653be34d4245e238" +checksum = "714a53001bf66416adb0e2ef5ac857140e7dc3a0c48fb28b2f10762fc4b5069f" dependencies = [ "anstream", "anstyle", @@ -266,9 +266,9 @@ dependencies = [ [[package]] name = "clap_derive" -version = "4.5.55" +version = "4.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a92793da1a46a5f2a02a6f4c46c6496b28c43638adea8306fcb0caa1634f24e5" +checksum = "1110bd8a634a1ab8cb04345d8d878267d57c3cf1b38d91b71af6686408bbca6a" dependencies = [ "heck", "proc-macro2", @@ -278,9 +278,9 @@ dependencies = [ [[package]] name = "clap_lex" -version = "0.7.7" +version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c3e64b0cc0439b12df2fa678eae89a1c56a529fd067a9115f7827f1fffd22b32" +checksum = "c8d4a3bb8b1e0c1050499d1815f5ab16d04f0959b233085fb31653fbfc9d98f9" [[package]] name = "clru" @@ -293,9 +293,9 @@ dependencies = [ [[package]] name = "colorchoice" -version = "1.0.4" +version = "1.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b05b61dc5112cbb17e4b6cd61790d9845d13888356391624cbe7e41efeac1e75" +checksum = "1d07550c9036bf2ae0c684c4297d503f838287c83c53686d05370d0e139ae570" [[package]] name = "convert_case" @@ -698,18 +698,6 @@ dependencies = [ "version_check", ] -[[package]] -name = "getrandom" -version = "0.3.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "899def5c37c4fd7b2664648c28120ecec138e4d395b459e5ca34f9cce2dd77fd" -dependencies = [ - "cfg-if", - "libc", - "r-efi 5.3.0", - "wasip2", -] - [[package]] name = "getrandom" version = "0.4.2" @@ -718,7 +706,7 @@ checksum = "0de51e6874e94e7bf76d726fc5d13ba782deca734ff60d5bb2fb2607c7406555" dependencies = [ "cfg-if", "libc", - "r-efi 6.0.0", + "r-efi", "rand_core", "wasip2", "wasip3", @@ -1575,9 +1563,9 @@ dependencies = [ [[package]] name = "inquire" -version = "0.9.2" +version = "0.9.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ae51d5da01ce7039024fbdec477767c102c454dbdb09d4e2a432ece705b1b25d" +checksum = "6654738b8024300cf062d04a1c13c10c8e2cea598ec1c47dc9b6641159429756" dependencies = [ "bitflags", "crossterm", @@ -1673,6 +1661,7 @@ dependencies = [ "jj-lib", "lazy-regex", "predicates", + "textwrap", "thiserror", "tokio", ] @@ -1757,9 +1746,9 @@ dependencies = [ [[package]] name = "lazy-regex" -version = "3.5.1" +version = "3.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "c5c13b6857ade4c8ee05c3c3dc97d2ab5415d691213825b90d3211c425c1f907" +checksum = "6bae91019476d3ec7147de9aa291cadb6d870abf2f3015d2da73a90325ac1496" dependencies = [ "lazy-regex-proc_macros", "once_cell", @@ -1769,9 +1758,9 @@ dependencies = [ [[package]] name = "lazy-regex-proc_macros" -version = "3.5.1" +version = "3.6.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "32a95c68db5d41694cea563c86a4ba4dc02141c16ef64814108cb23def4d5438" +checksum = "4de9c1e1439d8b7b3061b2d209809f447ca33241733d9a3c01eabf2dc8d94358" dependencies = [ "proc-macro2", "quote", @@ -1793,9 +1782,9 @@ checksum = "09edd9e8b54e49e587e4f6295a7d29c3ea94d469cb40ab8ca70b288248a81db2" [[package]] name = "libc" -version = "0.2.182" +version = "0.2.183" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "6800badb6cb2082ffd7b6a67e6125bb39f18782f793520caee8cb8846be06112" +checksum = "b5b646652bf6661599e1da8901b3b9522896f01e736bad5f723fe7a3a27f899d" [[package]] name = "libredox" @@ -1889,9 +1878,9 @@ dependencies = [ [[package]] name = "memchr" -version = "2.7.6" +version = "2.8.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f52b00d39961fc5b2736ea853c9cc86238e165017a493d1d5c8eac6bdc4cc273" +checksum = "f8ca58f447f06ed17d5fc4043ce1b10dd205e060fb3ce5b979b8ed8e59ff3f79" [[package]] name = "memmap2" @@ -1937,9 +1926,9 @@ dependencies = [ [[package]] name = "once_cell" -version = "1.21.3" +version = "1.21.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "42f5e15c9953c5e4ccceeb2e7382a716482c34515315f7b03532b8b4e8393d2d" +checksum = "9f7c3e4beb33f85d45ae3e3a1792185706c8e16d043238c593331cc7cd313b50" [[package]] name = "once_cell_polyfill" @@ -2021,9 +2010,9 @@ dependencies = [ [[package]] name = "pin-project-lite" -version = "0.2.16" +version = "0.2.17" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b3cff922bd51709b605d9ead9aa71031d81447142d828eb4a6eba76fe619f9b" +checksum = "a89322df9ebe1c1578d689c92318e070967d1042b512afbe49518723f4e6d5cd" [[package]] name = "plain" @@ -2063,9 +2052,9 @@ dependencies = [ [[package]] name = "predicates" -version = "3.1.3" +version = "3.1.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a5d19ee57562043d37e82899fade9a22ebab7be9cef5026b07fda9cdd4293573" +checksum = "ada8f2932f28a27ee7b70dd6c1c39ea0675c55a36879ab92f3a715eaa1e63cfe" dependencies = [ "anstyle", "difflib", @@ -2077,15 +2066,15 @@ dependencies = [ [[package]] name = "predicates-core" -version = "1.0.9" +version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "727e462b119fe9c93fd0eb1429a5f7647394014cf3c04ab2c0350eeb09095ffa" +checksum = "cad38746f3166b4031b1a0d39ad9f954dd291e7854fcc0eed52ee41a0b50d144" [[package]] name = "predicates-tree" -version = "1.0.12" +version = "1.0.13" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72dd2d6d381dfb73a193c7fca536518d7caee39fc8503f74e7dc0be0531b425c" +checksum = "d0de1b847b39c8131db0467e9df1ff60e6d0562ab8e9a16e568ad0fdb372e2f2" dependencies = [ "predicates-core", "termtree", @@ -2144,19 +2133,13 @@ dependencies = [ [[package]] name = "quote" -version = "1.0.44" +version = "1.0.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "21b2ebcf727b7760c461f091f9f0f539b77b8e87f2fd88131e7f1b433b3cece4" +checksum = "41f2619966050689382d2b44f664f4bc593e129785a36d6ee376ddf37259b924" dependencies = [ "proc-macro2", ] -[[package]] -name = "r-efi" -version = "5.3.0" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "69cdb34c158ceb288df11e18b4bd39de994f6657d83847bdffdbd7f346754b0f" - [[package]] name = "r-efi" version = "6.0.0" @@ -2170,7 +2153,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "bc266eb313df6c5c09c1c7b1fbe2510961e5bcd3add930c1e31f7ed9da0feff8" dependencies = [ "chacha20", - "getrandom 0.4.2", + "getrandom", "rand_core", ] @@ -2279,9 +2262,9 @@ checksum = "cab834c73d247e67f4fae452806d17d3c7501756d98c8808d7c9c7aa7d18f973" [[package]] name = "regex-syntax" -version = "0.8.9" +version = "0.8.10" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a96887878f22d7bad8a3b6dc5b7440e0ada9a245242924394987b21cf2210a4c" +checksum = "dc897dd8d9e8bd1ed8cdad82b5966c3e0ecae09fb1907d58efaa013543185d0a" [[package]] name = "rustc_version" @@ -2474,6 +2457,12 @@ dependencies = [ "serde", ] +[[package]] +name = "smawk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" + [[package]] name = "stable_deref_trait" version = "1.2.1" @@ -2500,9 +2489,9 @@ checksum = "13c2bddecc57b384dee18652358fb23172facb8a2c51ccc10d74c157bdea3292" [[package]] name = "syn" -version = "2.0.114" +version = "2.0.117" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d4d107df263a3013ef9b1879b0df87d706ff80f65a86ea879bd9c31f9b307c2a" +checksum = "e665b8803e7b1d2a727f4023456bbbbe74da67099c585258af0ad9c5013b9b99" dependencies = [ "proc-macro2", "quote", @@ -2511,12 +2500,12 @@ dependencies = [ [[package]] name = "tempfile" -version = "3.26.0" +version = "3.27.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "82a72c767771b47409d2345987fda8628641887d5466101319899796367354a0" +checksum = "32497e9a4c7b38532efcdebeef879707aa9f794296a4f0244f6f69e9bc8574bd" dependencies = [ "fastrand", - "getrandom 0.3.4", + "getrandom", "once_cell", "rustix", "windows-sys 0.61.2", @@ -2528,6 +2517,17 @@ version = "0.5.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8f50febec83f5ee1df3015341d8bd429f2d1cc62bcba7ea2076759d315084683" +[[package]] +name = "textwrap" +version = "0.16.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c13547615a44dc9c452a8a534638acdf07120d4b6847c8178705da06306a3057" +dependencies = [ + "smawk", + "unicode-linebreak", + "unicode-width", +] + [[package]] name = "thiserror" version = "2.0.18" @@ -2574,9 +2574,9 @@ checksum = "1f3ccbac311fea05f86f61904b462b55fb3df8837a366dfc601a0161d0532f20" [[package]] name = "tokio" -version = "1.49.0" +version = "1.50.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "72a2903cd7736441aac9df9d7688bd0ce48edccaadf181c3b90be801e81d3d86" +checksum = "27ad5e34374e03cfffefc301becb44e9dc3c17584f414349ebe29ed26661822d" dependencies = [ "bytes", "pin-project-lite", @@ -2585,9 +2585,9 @@ dependencies = [ [[package]] name = "tokio-macros" -version = "2.6.0" +version = "2.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "af407857209536a95c8e56f8231ef2c2e2aff839b22e07a1ffcbc617e9db9fa5" +checksum = "5c55a2eff8b69ce66c84f85e1da1c233edc36ceb85a2058d11b0d6a3c7e7569c" dependencies = [ "proc-macro2", "quote", @@ -2699,9 +2699,15 @@ checksum = "7eec5d1121208364f6793f7d2e222bf75a915c19557537745b195b253dd64217" [[package]] name = "unicode-ident" -version = "1.0.22" +version = "1.0.24" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "9312f7c4f6ff9069b165498234ce8be658059c6728633667c526e27dc2cf1df5" +checksum = "e6e4313cd5fcd3dad5cafa179702e2b244f760991f45397d14d4ebf38247da75" + +[[package]] +name = "unicode-linebreak" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" [[package]] name = "unicode-normalization" @@ -3038,9 +3044,9 @@ checksum = "589f6da84c646204747d1270a2a5661ea66ed1cced2631d546fdfb155959f9ec" [[package]] name = "winnow" -version = "0.7.14" +version = "0.7.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a5364e9d77fcdeeaa6062ced926ee3381faa2ee02d3eb83a5c27a8825540829" +checksum = "df79d97927682d2fd8adb29682d1140b343be4ac0f08fd68b7765d9c059d3945" dependencies = [ "memchr", ] @@ -3145,18 +3151,18 @@ dependencies = [ [[package]] name = "zerocopy" -version = "0.8.40" +version = "0.8.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "a789c6e490b576db9f7e6b6d661bcc9799f7c0ac8352f56ea20193b2681532e5" +checksum = "f2578b716f8a7a858b7f02d5bd870c14bf4ddbbcf3a4c05414ba6503640505e3" dependencies = [ "zerocopy-derive", ] [[package]] name = "zerocopy-derive" -version = "0.8.40" +version = "0.8.42" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "f65c489a7071a749c849713807783f70672b28094011623e200cb86dcb835953" +checksum = "7e6cc098ea4d3bd6246687de65af3f920c430e236bee1e3bf2e441463f08a02f" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index f671812..24d9f42 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -30,6 +30,7 @@ jj-lib = "0.39.0" lazy-regex = { version = "3.5.1", features = ["lite"] } thiserror = "2.0.18" tokio = { version = "1.49.0", features = ["macros", "rt-multi-thread"] } +textwrap = "0.16.2" [dev-dependencies] assert_cmd = "2.1.2" diff --git a/src/commit/types/breaking_change.rs b/src/commit/types/breaking_change.rs new file mode 100644 index 0000000..48d0abc --- /dev/null +++ b/src/commit/types/breaking_change.rs @@ -0,0 +1,125 @@ +use super::Footer; + +#[derive(Debug, Clone, PartialEq, Eq)] +#[repr(transparent)] +pub struct BreakingChangeNote(String); + +impl Footer for BreakingChangeNote { + fn note(&self) -> &str { + &self.0 + } + + fn prefix(&self) -> &str { + "BREAKING CHANGE" + } +} + +impl From for BreakingChangeNote +where + T: ToString, +{ + fn from(value: T) -> Self { + Self(value.to_string()) + } +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum BreakingChange { + No, + Yes, + WithNote(BreakingChangeNote), +} + +impl BreakingChange { + pub fn ignore(&self) -> bool { + matches!(self, BreakingChange::No) + } + + pub fn header_segment(&self) -> &str { + match self { + Self::No => "", + _ => "!", + } + } + + pub fn as_footer(&self) -> String { + match self { + BreakingChange::WithNote(footer) => footer.as_footer(), + _ => "".into(), + } + } +} + +impl From for BreakingChange +where + T: ToString, +{ + fn from(value: T) -> Self { + match value.to_string().trim() { + "" => Self::Yes, + value => Self::WithNote(value.into()), + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Empty string produces Yes(None) — no footer, only '!' in the header + #[test] + fn from_empty_string_yields_yes_none() { + assert_eq!(BreakingChange::from(String::new()), BreakingChange::Yes); + } + + /// Whitespace-only string produces Yes(None) + #[test] + fn from_whitespace_string_yields_yes_none() { + assert_eq!(BreakingChange::from(" ".to_string()), BreakingChange::Yes); + } + + /// Mixed whitespace (tabs, newlines) produces Yes(None) + #[test] + fn from_tab_newline_string_yields_yes_none() { + assert_eq!( + BreakingChange::from("\t\n ".to_string()), + BreakingChange::Yes + ); + } + + /// Non-empty string produces Yes(Some(...)) with the note preserved + #[test] + fn from_non_empty_string_yields_yes_some() { + assert_eq!( + BreakingChange::from("removes old API"), + BreakingChange::WithNote("removes old API".into()), + ); + } + + /// Surrounding whitespace is trimmed from the note + #[test] + fn from_string_trims_surrounding_whitespace() { + assert_eq!( + BreakingChange::from(" removes old API "), + BreakingChange::WithNote("removes old API".into()), + ); + } + + /// Leading whitespace only is trimmed, leaving the non-empty part + #[test] + fn from_string_trims_leading_whitespace() { + assert_eq!( + BreakingChange::from(" removes old API"), + BreakingChange::WithNote("removes old API".into()), + ); + } + + /// Trailing whitespace only is trimmed, leaving the non-empty part + #[test] + fn from_string_trims_trailing_whitespace() { + assert_eq!( + BreakingChange::from("removes old API "), + BreakingChange::WithNote("removes old API".into()), + ); + } +} diff --git a/src/commit/types/footer.rs b/src/commit/types/footer.rs new file mode 100644 index 0000000..2206acc --- /dev/null +++ b/src/commit/types/footer.rs @@ -0,0 +1,16 @@ +pub trait Footer { + fn prefix(&self) -> &str; + fn note(&self) -> &str; + + fn as_footer(&self) -> String + where + Self: std::fmt::Debug, + { + let default = format!("{}: {}", self.prefix(), self.note()); + if default.chars().count() > 72 { + textwrap::wrap(&default, 71).join("\n ") + } else { + default + } + } +} diff --git a/src/commit/types/message.rs b/src/commit/types/message.rs index 0182fb2..7e61f7c 100644 --- a/src/commit/types/message.rs +++ b/src/commit/types/message.rs @@ -1,4 +1,4 @@ -use super::{CommitType, Description, Scope}; +use super::{BreakingChange, CommitType, Description, Scope}; use thiserror::Error; /// Errors that can occur when creating a ConventionalCommit @@ -21,6 +21,7 @@ pub struct ConventionalCommit { commit_type: CommitType, scope: Scope, description: Description, + breaking_change: BreakingChange, } impl ConventionalCommit { @@ -40,11 +41,13 @@ impl ConventionalCommit { commit_type: CommitType, scope: Scope, description: Description, + breaking_change: BreakingChange, ) -> Result { let commit = Self { commit_type, scope, description, + breaking_change, }; let len = commit.first_line_len(); if len > Self::FIRST_LINE_MAX_LENGTH { @@ -65,13 +68,12 @@ impl ConventionalCommit { /// Calculate the length of the formatted first line /// /// Formula: - /// - With scope: `len(type) + len(scope) + 4 + len(description)` - /// (the 4 accounts for parentheses, colon, and space: "() ") - /// - Without scope: `len(type) + 2 + len(description)` + /// - `len(type)` + `len(scope)` + `len(breaking_change)` + 2 + `len(description)` /// (the 2 accounts for colon and space: ": ") pub fn first_line_len(&self) -> usize { self.commit_type.len() + self.scope.header_segment_len() + + if self.breaking_change.ignore() { 0 } else { 1 } + 2 // ": " + self.description.len() } @@ -81,7 +83,12 @@ impl ConventionalCommit { /// Returns `type(scope): description` if scope is non-empty, or /// `type: description` if scope is empty pub fn format(&self) -> String { - Self::format_preview(self.commit_type, &self.scope, &self.description) + Self::format_preview( + self.commit_type, + &self.scope, + &self.description, + &self.breaking_change, + ) } /// Format a preview of the commit message without creating a validated instance @@ -93,12 +100,18 @@ impl ConventionalCommit { commit_type: CommitType, scope: &Scope, description: &Description, + breaking_change: &BreakingChange, ) -> String { - if scope.is_empty() { - format!("{}: {}", commit_type, description) - } else { - format!("{}({}): {}", commit_type, scope, description) - } + let scope = scope.header_segment(); + let breaking_change_header = breaking_change.header_segment(); + let breaking_change_footer = breaking_change.as_footer(); + format!( + r#"{commit_type}{scope}{breaking_change_header}: {description} + +{breaking_change_footer}"#, + ) + .trim() + .to_string() } /// Returns the commit type @@ -142,8 +155,9 @@ mod tests { commit_type: CommitType, scope: Scope, description: Description, + breaking_change: BreakingChange, ) -> ConventionalCommit { - ConventionalCommit::new(commit_type, scope, description) + ConventionalCommit::new(commit_type, scope, description, breaking_change) .expect("test commit should have valid line length") } @@ -154,6 +168,7 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("add new feature"), + BreakingChange::No, ); assert_eq!(commit.commit_type(), CommitType::Feat); assert_eq!(commit.scope().as_str(), "cli"); @@ -167,6 +182,7 @@ mod tests { CommitType::Fix, Scope::empty(), test_description("fix critical bug"), + BreakingChange::No, ); assert_eq!(commit.commit_type(), CommitType::Fix); assert!(commit.scope().is_empty()); @@ -180,6 +196,7 @@ mod tests { CommitType::Feat, test_scope("auth"), test_description("add login"), + BreakingChange::No, ); assert_eq!(commit.format(), "feat(auth): add login"); } @@ -192,6 +209,7 @@ mod tests { CommitType::Fix, test_scope("user-auth"), test_description("fix token refresh"), + BreakingChange::No, ); assert_eq!(commit1.format(), "fix(user-auth): fix token refresh"); @@ -200,6 +218,7 @@ mod tests { CommitType::Docs, test_scope("api_docs"), test_description("update README"), + BreakingChange::No, ); assert_eq!(commit2.format(), "docs(api_docs): update README"); @@ -208,6 +227,7 @@ mod tests { CommitType::Chore, test_scope("PROJ-123/cleanup"), test_description("remove unused code"), + BreakingChange::No, ); assert_eq!( commit3.format(), @@ -222,6 +242,7 @@ mod tests { CommitType::Feat, Scope::empty(), test_description("add login"), + BreakingChange::No, ); assert_eq!(commit.format(), "feat: add login"); } @@ -233,6 +254,7 @@ mod tests { CommitType::Fix, Scope::empty(), test_description("fix critical bug"), + BreakingChange::No, ); assert_eq!(commit1.format(), "fix: fix critical bug"); @@ -240,6 +262,7 @@ mod tests { CommitType::Docs, Scope::empty(), test_description("update installation guide"), + BreakingChange::No, ); assert_eq!(commit2.format(), "docs: update installation guide"); } @@ -265,7 +288,7 @@ mod tests { ]; for (commit_type, expected) in expected_formats { - let commit = test_commit(commit_type, scope.clone(), desc.clone()); + let commit = test_commit(commit_type, scope.clone(), desc.clone(), BreakingChange::No); assert_eq!( commit.format(), expected, @@ -295,7 +318,12 @@ mod tests { ]; for (commit_type, expected) in expected_formats { - let commit = test_commit(commit_type, Scope::empty(), desc.clone()); + let commit = test_commit( + commit_type, + Scope::empty(), + desc.clone(), + BreakingChange::No, + ); assert_eq!( commit.format(), expected, @@ -312,6 +340,7 @@ mod tests { CommitType::Feat, test_scope("auth"), test_description("add login"), + BreakingChange::No, ); let display_output = format!("{}", commit); let format_output = commit.format(); @@ -325,6 +354,7 @@ mod tests { CommitType::Fix, test_scope("api"), test_description("handle null response"), + BreakingChange::No, ); assert_eq!(format!("{}", commit), "fix(api): handle null response"); } @@ -336,6 +366,7 @@ mod tests { CommitType::Docs, Scope::empty(), test_description("improve README"), + BreakingChange::No, ); assert_eq!(format!("{}", commit), "docs: improve README"); } @@ -345,8 +376,12 @@ mod tests { fn display_equals_format_for_all_types() { for commit_type in CommitType::all() { // With scope - let commit_with_scope = - test_commit(*commit_type, test_scope("test"), test_description("change")); + let commit_with_scope = test_commit( + *commit_type, + test_scope("test"), + test_description("change"), + BreakingChange::No, + ); assert_eq!( format!("{}", commit_with_scope), commit_with_scope.format(), @@ -355,8 +390,12 @@ mod tests { ); // Without scope - let commit_without_scope = - test_commit(*commit_type, Scope::empty(), test_description("change")); + let commit_without_scope = test_commit( + *commit_type, + Scope::empty(), + test_description("change"), + BreakingChange::No, + ); assert_eq!( format!("{}", commit_without_scope), commit_without_scope.format(), @@ -370,7 +409,12 @@ mod tests { #[test] fn commit_type_accessor_returns_correct_type() { for commit_type in CommitType::all() { - let commit = test_commit(*commit_type, Scope::empty(), test_description("test")); + let commit = test_commit( + *commit_type, + Scope::empty(), + test_description("test"), + BreakingChange::No, + ); assert_eq!(commit.commit_type(), *commit_type); } } @@ -382,6 +426,7 @@ mod tests { CommitType::Feat, test_scope("auth"), test_description("add feature"), + BreakingChange::No, ); assert_eq!(commit.scope().as_str(), "auth"); } @@ -393,6 +438,7 @@ mod tests { CommitType::Feat, Scope::empty(), test_description("add feature"), + BreakingChange::No, ); assert!(commit.scope().is_empty()); } @@ -404,6 +450,7 @@ mod tests { CommitType::Feat, Scope::empty(), test_description("add new authentication flow"), + BreakingChange::No, ); assert_eq!(commit.description().as_str(), "add new authentication flow"); } @@ -415,6 +462,7 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("add feature"), + BreakingChange::No, ); let cloned = original.clone(); assert_eq!(original, cloned); @@ -427,11 +475,13 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("add feature"), + BreakingChange::No, ); let commit2 = test_commit( CommitType::Feat, test_scope("cli"), test_description("add feature"), + BreakingChange::No, ); assert_eq!(commit1, commit2); } @@ -443,11 +493,13 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("change"), + BreakingChange::No, ); let commit2 = test_commit( CommitType::Fix, test_scope("cli"), test_description("change"), + BreakingChange::No, ); assert_ne!(commit1, commit2); } @@ -459,11 +511,13 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("change"), + BreakingChange::No, ); let commit2 = test_commit( CommitType::Feat, test_scope("api"), test_description("change"), + BreakingChange::No, ); assert_ne!(commit1, commit2); } @@ -475,11 +529,13 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("add feature"), + BreakingChange::No, ); let commit2 = test_commit( CommitType::Feat, test_scope("cli"), test_description("fix bug"), + BreakingChange::No, ); assert_ne!(commit1, commit2); } @@ -491,6 +547,7 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("add feature"), + BreakingChange::No, ); let debug_output = format!("{:?}", commit); assert!(debug_output.contains("ConventionalCommit")); @@ -504,6 +561,7 @@ mod tests { CommitType::Feat, test_scope("auth"), test_description("implement OAuth2 login flow"), + BreakingChange::No, ); assert_eq!(commit.format(), "feat(auth): implement OAuth2 login flow"); } @@ -515,6 +573,7 @@ mod tests { CommitType::Fix, Scope::empty(), test_description("prevent crash on empty input"), + BreakingChange::No, ); assert_eq!(commit.format(), "fix: prevent crash on empty input"); } @@ -526,6 +585,7 @@ mod tests { CommitType::Docs, test_scope("README"), test_description("add installation instructions"), + BreakingChange::No, ); assert_eq!( commit.format(), @@ -540,6 +600,7 @@ mod tests { CommitType::Refactor, test_scope("core"), test_description("extract validation logic"), + BreakingChange::No, ); assert_eq!(commit.format(), "refactor(core): extract validation logic"); } @@ -551,6 +612,7 @@ mod tests { CommitType::Ci, test_scope("github"), test_description("add release workflow"), + BreakingChange::No, ); assert_eq!(commit.format(), "ci(github): add release workflow"); } @@ -563,6 +625,7 @@ mod tests { CommitType::Feat, Scope::empty(), Description::parse(&long_desc).unwrap(), + BreakingChange::No, ); // Format should be "feat: " + 50 chars = 56 total chars let formatted = commit.format(); @@ -577,6 +640,7 @@ mod tests { CommitType::Feat, test_scope("my-scope_v2/feature"), test_description("add support"), + BreakingChange::No, ); assert_eq!(commit.format(), "feat(my-scope_v2/feature): add support"); } @@ -594,6 +658,7 @@ mod tests { CommitType::Feat, Scope::empty(), test_description("add login"), + BreakingChange::No, ); // "feat: add login" = 4 + 2 + 9 = 15 assert_eq!(commit.first_line_len(), 15); @@ -606,6 +671,7 @@ mod tests { CommitType::Feat, test_scope("auth"), test_description("add login"), + BreakingChange::No, ); // "feat(auth): add login" = 4 + 4 + 4 + 9 = 21 assert_eq!(commit.first_line_len(), 21); @@ -622,6 +688,7 @@ mod tests { CommitType::Feat, Scope::parse(&scope_20).unwrap(), Description::parse(&desc_44).unwrap(), + BreakingChange::No, ); assert!(result.is_ok()); let commit = result.unwrap(); @@ -650,6 +717,7 @@ mod tests { CommitType::Refactor, Scope::parse(&scope_30).unwrap(), Description::parse(&desc_31).unwrap(), + BreakingChange::No, ); assert!(result.is_err()); assert_eq!( @@ -672,6 +740,7 @@ mod tests { CommitType::Refactor, Scope::parse(&scope_30).unwrap(), Description::parse(&desc_40).unwrap(), + BreakingChange::No, ); assert!(result.is_err()); assert_eq!( @@ -690,6 +759,7 @@ mod tests { CommitType::Fix, Scope::empty(), test_description("quick fix"), + BreakingChange::No, ); assert!(result.is_ok()); } @@ -701,6 +771,7 @@ mod tests { CommitType::Feat, test_scope("cli"), test_description("add feature"), + BreakingChange::No, ); assert!(result.is_ok()); } @@ -721,8 +792,12 @@ mod tests { /// Test new() returns Result type #[test] fn new_returns_result() { - let result = - ConventionalCommit::new(CommitType::Feat, Scope::empty(), test_description("test")); + let result = ConventionalCommit::new( + CommitType::Feat, + Scope::empty(), + test_description("test"), + BreakingChange::No, + ); // Just verify it's a Result by using is_ok() assert!(result.is_ok()); } @@ -747,7 +822,7 @@ mod tests { None => Scope::empty(), }; let desc = Description::parse(*desc_str).unwrap(); - let commit = ConventionalCommit::new(*commit_type, scope, desc); + let commit = ConventionalCommit::new(*commit_type, scope, desc, BreakingChange::No); // new() itself calls git_conventional::Commit::parse internally, so // if this is Ok, SC-002 is satisfied for this case. assert!( @@ -771,4 +846,273 @@ mod tests { assert!(msg.contains("git-conventional")); assert!(msg.contains("missing type")); } + + /// Breaking change without note and without scope: header gets '!', no footer + #[test] + fn format_breaking_change_no_note_no_scope() { + let commit = test_commit( + CommitType::Feat, + Scope::empty(), + test_description("add login"), + BreakingChange::Yes, + ); + assert_eq!(commit.format(), "feat!: add login"); + } + + /// Breaking change without note and with scope: '!' goes after closing paren + #[test] + fn format_breaking_change_no_note_with_scope() { + let commit = test_commit( + CommitType::Feat, + test_scope("auth"), + test_description("add login"), + BreakingChange::Yes, + ); + assert_eq!(commit.format(), "feat(auth)!: add login"); + } + + /// Breaking change with note and without scope: footer is appended after a blank line + #[test] + fn format_breaking_change_with_note_no_scope() { + let commit = test_commit( + CommitType::Feat, + Scope::empty(), + test_description("drop Node 6"), + "Node 6 is no longer supported".into(), + ); + assert_eq!( + commit.format(), + "feat!: drop Node 6\n\nBREAKING CHANGE: Node 6 is no longer supported", + ); + } + + /// Breaking change with note and with scope: both '!' and footer are present + #[test] + fn format_breaking_change_with_note_and_scope() { + let commit = test_commit( + CommitType::Fix, + test_scope("api"), + test_description("drop Node 6"), + "Node 6 is no longer supported".into(), + ); + assert_eq!( + commit.format(), + "fix(api)!: drop Node 6\n\nBREAKING CHANGE: Node 6 is no longer supported", + ); + } + + /// Display with breaking change delegates to format() (no scope, with note) + #[test] + fn display_breaking_change_with_note() { + let commit = test_commit( + CommitType::Feat, + Scope::empty(), + test_description("drop Node 6"), + "Node 6 is no longer supported".into(), + ); + assert_eq!( + format!("{}", commit), + "feat!: drop Node 6\n\nBREAKING CHANGE: Node 6 is no longer supported", + ); + } + + /// first_line_len() counts the '!' for a breaking change without scope + /// + /// "feat!: add login" = 4 + 1 + 2 + 9 = 16 + #[test] + fn first_line_len_breaking_change_no_scope() { + let commit = test_commit( + CommitType::Feat, + Scope::empty(), + test_description("add login"), + BreakingChange::Yes, + ); + assert_eq!(commit.first_line_len(), 16); + } + + /// first_line_len() counts the '!' for a breaking change with scope + /// + /// "feat(auth)!: add login" = 4 + 6 + 1 + 2 + 9 = 22 + #[test] + fn first_line_len_breaking_change_with_scope() { + let commit = test_commit( + CommitType::Feat, + test_scope("auth"), + test_description("add login"), + BreakingChange::Yes, + ); + assert_eq!(commit.first_line_len(), 22); + } + + /// The `!` counts toward the 72-character first-line limit + /// + /// The inputs below produce exactly 72 chars without a breaking change + /// (covered by `exactly_72_characters_accepted`). With `!` they reach + /// 73 and must be rejected. + #[test] + fn breaking_change_exclamation_counts_toward_line_limit() { + let scope_20 = "a".repeat(20); + let desc_44 = "b".repeat(44); + let result = ConventionalCommit::new( + CommitType::Feat, + Scope::parse(&scope_20).unwrap(), + Description::parse(&desc_44).unwrap(), + BreakingChange::Yes, + ); + assert!(result.is_err()); + assert_eq!( + result.unwrap_err(), + CommitMessageError::FirstLineTooLong { + actual: 73, + max: 72 + }, + ); + } + + /// Breaking change footer does not count toward the 72-character first-line limit + #[test] + fn breaking_change_footer_does_not_count_toward_line_limit() { + // First line is short; the note itself is long — should still be accepted. + let long_note = "x".repeat(200); + let result = ConventionalCommit::new( + CommitType::Fix, + Scope::empty(), + test_description("quick fix"), + long_note.into(), + ); + assert!(result.is_ok()); + } + + /// format_preview() static method produces the same result as format() for identical inputs + #[test] + fn format_preview_matches_format() { + let commit = test_commit( + CommitType::Feat, + test_scope("auth"), + test_description("add login"), + BreakingChange::No, + ); + let preview = ConventionalCommit::format_preview( + commit.commit_type(), + commit.scope(), + commit.description(), + &BreakingChange::No, + ); + assert_eq!(preview, commit.format()); + } + + /// format_preview() with a breaking-change note produces the full multi-line message + #[test] + fn format_preview_breaking_change_with_note() { + let preview = ConventionalCommit::format_preview( + CommitType::Feat, + &Scope::empty(), + &test_description("drop legacy API"), + &"removes legacy endpoint".into(), + ); + assert_eq!( + preview, + "feat!: drop legacy API\n\nBREAKING CHANGE: removes legacy endpoint" + ); + } + + /// format_preview() with scope and breaking-change note + #[test] + fn format_preview_breaking_change_with_scope_and_note() { + let preview = ConventionalCommit::format_preview( + CommitType::Fix, + &test_scope("api"), + &test_description("drop Node 6"), + &"Node 6 is no longer supported".into(), + ); + assert_eq!( + preview, + "fix(api)!: drop Node 6\n\nBREAKING CHANGE: Node 6 is no longer supported" + ); + } + + /// Breaking-change footer is separated from the header by exactly one blank line + #[test] + fn format_breaking_change_footer_separator() { + let commit = test_commit( + CommitType::Fix, + Scope::empty(), + test_description("drop old API"), + "old API removed".into(), + ); + let formatted = commit.format(); + let parts: Vec<&str> = formatted.splitn(2, "\n\n").collect(); + assert_eq!( + parts.len(), + 2, + "expected header and footer separated by \\n\\n" + ); + assert_eq!(parts[0], "fix!: drop old API"); + assert_eq!(parts[1], "BREAKING CHANGE: old API removed"); + } + + /// format() output has no leading or trailing whitespace for any variant + #[test] + fn format_has_no_surrounding_whitespace() { + let no_bc = test_commit( + CommitType::Feat, + Scope::empty(), + test_description("add feature"), + BreakingChange::No, + ); + let f = no_bc.format(); + assert_eq!( + f, + f.trim(), + "format() must not have surrounding whitespace (no breaking change)" + ); + + let with_note = test_commit( + CommitType::Fix, + Scope::empty(), + test_description("fix bug"), + "important migration required".into(), + ); + let f2 = with_note.format(); + assert_eq!( + f2, + f2.trim(), + "format() must not have surrounding whitespace (with note)" + ); + } + + /// All commit types format correctly with breaking change and no note + #[test] + fn all_commit_types_format_with_breaking_change_no_note() { + let desc = test_description("test change"); + + let expected_formats = [ + (CommitType::Feat, "feat!: test change"), + (CommitType::Fix, "fix!: test change"), + (CommitType::Docs, "docs!: test change"), + (CommitType::Style, "style!: test change"), + (CommitType::Refactor, "refactor!: test change"), + (CommitType::Perf, "perf!: test change"), + (CommitType::Test, "test!: test change"), + (CommitType::Build, "build!: test change"), + (CommitType::Ci, "ci!: test change"), + (CommitType::Chore, "chore!: test change"), + (CommitType::Revert, "revert!: test change"), + ]; + + for (commit_type, expected) in expected_formats { + let commit = test_commit( + commit_type, + Scope::empty(), + desc.clone(), + BreakingChange::Yes, + ); + assert_eq!( + commit.format(), + expected, + "Format should be correct for {:?} with breaking change", + commit_type + ); + } + } } diff --git a/src/commit/types/mod.rs b/src/commit/types/mod.rs index 2002720..ee15394 100644 --- a/src/commit/types/mod.rs +++ b/src/commit/types/mod.rs @@ -1,3 +1,9 @@ +mod footer; +pub use footer::Footer; + +mod breaking_change; +pub use breaking_change::BreakingChange; + mod commit_type; pub use commit_type::CommitType; diff --git a/src/lib.rs b/src/lib.rs index d54ae18..dac0a9c 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -6,8 +6,8 @@ mod prompts; pub use crate::{ commit::types::{ - CommitMessageError, CommitType, ConventionalCommit, Description, DescriptionError, Scope, - ScopeError, + BreakingChange, CommitMessageError, CommitType, ConventionalCommit, Description, + DescriptionError, Scope, ScopeError, }, error::Error, jj::{JjExecutor, lib_executor::JjLib}, diff --git a/src/prompts/mock.rs b/src/prompts/mock.rs index 9244327..a6c8e74 100644 --- a/src/prompts/mock.rs +++ b/src/prompts/mock.rs @@ -8,7 +8,7 @@ use std::sync::{Arc, Mutex}; use crate::{ - commit::types::{CommitType, Description, Scope}, + commit::types::{BreakingChange, CommitType, Description, Scope}, error::Error, prompts::prompter::Prompter, }; @@ -19,6 +19,7 @@ enum MockResponse { CommitType(CommitType), Scope(Scope), Description(Description), + BreakingChange(BreakingChange), Confirm(bool), Error(Error), } @@ -70,6 +71,15 @@ impl MockPrompts { self } + /// Configure the mock to return a specific breaking change response + pub fn with_breaking_change(self, breaking_change: BreakingChange) -> Self { + self.responses + .lock() + .unwrap() + .push(MockResponse::BreakingChange(breaking_change)); + self + } + /// Configure the mock to return a specific confirmation response pub fn with_confirm(self, confirm: bool) -> Self { self.responses @@ -112,6 +122,14 @@ impl MockPrompts { .contains(&"input_description".to_string()) } + /// Check if input_breaking_change was called + pub fn was_breaking_change_called(&self) -> bool { + self.prompts_called + .lock() + .unwrap() + .contains(&"input_breaking_change".to_string()) + } + /// Check if confirm_apply was called pub fn was_confirm_called(&self) -> bool { self.prompts_called @@ -166,6 +184,19 @@ impl Prompter for MockPrompts { } } + fn input_breaking_change(&self) -> Result { + self.prompts_called + .lock() + .unwrap() + .push("input_breaking_change".to_string()); + + match self.responses.lock().unwrap().remove(0) { + MockResponse::BreakingChange(bc) => Ok(bc), + MockResponse::Error(e) => Err(e), + _ => panic!("MockPrompts: Expected BreakingChange response, got different type"), + } + } + fn confirm_apply(&self, _message: &str) -> Result { self.prompts_called .lock() @@ -281,4 +312,64 @@ mod tests { let mock = MockPrompts::new(); assert!(mock.emitted_messages().is_empty()); } + + #[test] + fn mock_input_breaking_change_no() { + let mock = MockPrompts::new().with_breaking_change(BreakingChange::No); + let result = mock.input_breaking_change(); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), BreakingChange::No); + assert!(mock.was_breaking_change_called()); + } + + #[test] + fn mock_input_breaking_change_yes_no_note() { + let mock = MockPrompts::new().with_breaking_change(BreakingChange::Yes); + let result = mock.input_breaking_change(); + assert!(result.is_ok()); + assert_eq!(result.unwrap(), BreakingChange::Yes); + assert!(mock.was_breaking_change_called()); + } + + #[test] + fn mock_input_breaking_change_yes_with_note() { + let mock = MockPrompts::new().with_breaking_change("removes old API".into()); + let result = mock.input_breaking_change(); + assert!(result.is_ok()); + assert_eq!( + result.unwrap(), + BreakingChange::WithNote("removes old API".into()) + ); + assert!(mock.was_breaking_change_called()); + } + + #[test] + fn mock_input_breaking_change_error() { + let mock = MockPrompts::new().with_error(Error::Cancelled); + let result = mock.input_breaking_change(); + assert!(result.is_err()); + assert!(matches!(result.unwrap_err(), Error::Cancelled)); + } + + #[test] + fn mock_tracks_breaking_change_call() { + let mock = MockPrompts::new() + .with_commit_type(CommitType::Fix) + .with_scope(Scope::empty()) + .with_description(Description::parse("test").unwrap()) + .with_breaking_change(BreakingChange::No) + .with_confirm(true); + + mock.select_commit_type().unwrap(); + mock.input_scope().unwrap(); + mock.input_description().unwrap(); + mock.input_breaking_change().unwrap(); + mock.confirm_apply("test").unwrap(); + + assert!(mock.was_commit_type_called()); + assert!(mock.was_scope_called()); + assert!(mock.was_description_called()); + assert!(mock.was_breaking_change_called()); + assert!(mock.was_confirm_called()); + } } diff --git a/src/prompts/prompter.rs b/src/prompts/prompter.rs index 683c0ef..548e718 100644 --- a/src/prompts/prompter.rs +++ b/src/prompts/prompter.rs @@ -5,8 +5,10 @@ //! [`CommitWorkflow`](super::CommitWorkflow) to use real interactive prompts //! in production while accepting mock implementations in tests. +use inquire::{Confirm, Text}; + use crate::{ - commit::types::{CommitType, Description, Scope}, + commit::types::{BreakingChange, CommitType, Description, Scope}, error::Error, }; @@ -24,6 +26,9 @@ pub trait Prompter: Send + Sync { /// Prompt the user to input a required description fn input_description(&self) -> Result; + /// Prompt the user for breaking change + fn input_breaking_change(&self) -> Result; + /// Prompt the user to confirm applying the commit message fn confirm_apply(&self, message: &str) -> Result; @@ -34,6 +39,18 @@ pub trait Prompter: Send + Sync { fn emit_message(&self, msg: &str); } +fn format_message_box(message: &str) -> String { + let preview_width = 72 + 2; // max width + space padding + let mut lines: Vec = Vec::new(); + lines.push(format!("┌{}┐", "─".repeat(preview_width))); + for line in message.split("\n") { + let padding = 72_usize.saturating_sub(line.chars().count()); + lines.push(format!("│ {line}{:padding$} │", "")); + } + lines.push(format!("└{}┘", "─".repeat(preview_width))); + lines.join("\n") +} + /// Production implementation of [`Prompter`] using the `inquire` crate #[derive(Debug)] pub struct RealPrompts; @@ -137,18 +154,30 @@ impl Prompter for RealPrompts { } } + fn input_breaking_change(&self) -> Result { + if !Confirm::new("Does this revision include a breaking change?") + .with_default(false) + .prompt() + .map_err(|_| Error::Cancelled)? + { + return Ok(BreakingChange::No); + } + let answer = Text::new("Enter the description of the breaking change:") + .with_help_message("Enter an empty message to skip creating a message footer") + .prompt() + .map_err(|_| Error::Cancelled)?; + let trimmed = answer.trim(); + Ok(trimmed.into()) + } + fn confirm_apply(&self, message: &str) -> Result { use inquire::Confirm; // Show preview - println!(); - println!("📝 Commit Message Preview:"); - println!("┌────────────────────────────────────────────────────────────────────────┐"); - // Pad with spaces to fill the box - let padding = 72_usize.saturating_sub(message.chars().count()) - 1; - println!("│ {message}{:padding$}│", ""); - println!("└────────────────────────────────────────────────────────────────────────┘"); - println!(); + println!( + "\n📝 Commit Message Preview:\n{}\n", + format_message_box(message) + ); // Get confirmation Confirm::new("Apply this commit message?") @@ -174,4 +203,71 @@ mod tests { fn _accepts_prompter(_p: impl Prompter) {} _accepts_prompter(real); } + + /// Top border uses exactly preview_width (74) dashes; bottom likewise + #[test] + fn format_message_box_borders() { + let result = format_message_box("hello"); + let lines: Vec<&str> = result.split('\n').collect(); + let dashes = "─".repeat(74); + assert_eq!(lines[0], format!("┌{dashes}┐")); + assert_eq!(lines[lines.len() - 1], format!("└{dashes}┘")); + } + + /// A single-line message produces exactly 3 rows: top, content, bottom + #[test] + fn format_message_box_single_line_row_count() { + let result = format_message_box("feat: add login"); + assert_eq!(result.split('\n').count(), 3); + } + + /// A message with one `\n` produces 4 rows: top, two content, bottom + #[test] + fn format_message_box_multi_line_row_count() { + let result = format_message_box("feat: add login\nsecond line"); + assert_eq!(result.split('\n').count(), 4); + } + + /// A breaking-change message (`\n\n`) produces an empty content row for the blank line + #[test] + fn format_message_box_blank_separator_line() { + let msg = "feat!: drop old API\n\nBREAKING CHANGE: removed"; + let result = format_message_box(msg); + assert_eq!(result.split('\n').count(), 5); // top + 3 content + bottom + } + + /// All output rows have identical char counts (the box is rectangular) + #[test] + fn format_message_box_all_rows_same_width() { + let msg = "feat(auth): add login\n\nBREAKING CHANGE: old API removed"; + let result = format_message_box(msg); + let widths: Vec = result.split('\n').map(|l| l.chars().count()).collect(); + let expected = widths[0]; + assert!( + widths.iter().all(|&w| w == expected), + "rows have differing widths: {:?}", + widths + ); + } + + /// An empty message produces a single fully-padded content row + #[test] + fn format_message_box_empty_message() { + let result = format_message_box(""); + let lines: Vec<&str> = result.split('\n').collect(); + assert_eq!(lines.len(), 3); + // "│ " + 72 spaces + " │" = 76 chars + let expected = format!("│ {:72} │", ""); + assert_eq!(lines[1], expected); + } + + /// A line of exactly 72 characters leaves no right-hand padding + #[test] + fn format_message_box_line_exactly_72_chars() { + let line_72 = "a".repeat(72); + let result = format_message_box(&line_72); + let lines: Vec<&str> = result.split('\n').collect(); + let expected = format!("│ {line_72} │"); + assert_eq!(lines[1], expected); + } } diff --git a/src/prompts/workflow.rs b/src/prompts/workflow.rs index 29c37ea..d1dbe5a 100644 --- a/src/prompts/workflow.rs +++ b/src/prompts/workflow.rs @@ -4,7 +4,9 @@ //! creating a conventional commit message using interactive prompts. use crate::{ - commit::types::{CommitMessageError, CommitType, ConventionalCommit, Description, Scope}, + commit::types::{ + BreakingChange, CommitMessageError, CommitType, ConventionalCommit, Description, Scope, + }, error::Error, jj::JjExecutor, prompts::prompter::{Prompter, RealPrompts}, @@ -52,30 +54,19 @@ impl CommitWorkflow { /// - Repository operation fails /// - Message validation fails pub async fn run(&self) -> Result<(), Error> { - // Verify we're in a jj repository if !self.executor.is_repository().await? { return Err(Error::NotARepository); } - - // Step 1: Select commit type (kept across retries) let commit_type = self.type_selection().await?; - - // Steps 2–4 loop: re-prompt scope and description when the combined - // first line would exceed 72 characters (issue 3.4). loop { - // Step 2: Input scope (optional) let scope = self.scope_input().await?; - - // Step 3: Input description (required) let description = self.description_input().await?; - - // Step 4: Preview and confirm + let breaking_change = self.breaking_change_input().await?; match self - .preview_and_confirm(commit_type, scope, description) + .preview_and_confirm(commit_type, scope, description, breaking_change) .await { Ok(conventional_commit) => { - // Step 5: Apply the message self.executor .describe(&conventional_commit.to_string()) .await?; @@ -99,35 +90,49 @@ impl CommitWorkflow { /// Prompt user to input an optional scope /// - /// Returns Ok(Scope) with the validated scope, or Error::Cancelled if user cancels + /// Returns Ok(Scope) with the validated scope, or + /// Error::Cancelled if user cancels async fn scope_input(&self) -> Result { self.prompts.input_scope() } /// Prompt user to input a required description /// - /// Returns Ok(Description) with the validated description, or Error::Cancelled if user cancels + /// Returns Ok(Description) with the validated description, or + /// Error::Cancelled if user cancels async fn description_input(&self) -> Result { self.prompts.input_description() } + /// Prompt user for breaking change + /// + /// Returns Ok(BreakingChange) with the validated breaking change, + /// or Error::Cancel if user cancels + async fn breaking_change_input(&self) -> Result { + self.prompts.input_breaking_change() + } + /// Preview the formatted conventional commit message and get user confirmation /// - /// This method also validates that the complete first line doesn't exceed 72 characters + /// This method also validates that the complete first line + /// doesn't exceed 72 characters async fn preview_and_confirm( &self, commit_type: CommitType, scope: Scope, description: Description, + breaking_change: BreakingChange, ) -> Result { // Format the message for preview - let message = ConventionalCommit::format_preview(commit_type, &scope, &description); + let message = + ConventionalCommit::format_preview(commit_type, &scope, &description, &breaking_change); // Try to build the conventional commit (this validates the 72-char limit) let conventional_commit: ConventionalCommit = match ConventionalCommit::new( commit_type, scope.clone(), description.clone(), + crate::commit::types::BreakingChange::No, ) { Ok(cc) => cc, Err(CommitMessageError::FirstLineTooLong { actual, max }) => { @@ -252,9 +257,9 @@ mod tests { let commit_type = CommitType::Feat; let scope = Scope::empty(); let description = Description::parse("test description").unwrap(); - + let breaking_change = BreakingChange::No; let result = workflow - .preview_and_confirm(commit_type, scope, description) + .preview_and_confirm(commit_type, scope, description, breaking_change) .await; assert!(result.is_ok()); } @@ -299,6 +304,7 @@ mod tests { .with_commit_type(CommitType::Feat) .with_scope(Scope::empty()) .with_description(Description::parse("add new feature").unwrap()) + .with_breaking_change(BreakingChange::Yes) .with_confirm(true); // Create workflow with both mocks @@ -330,6 +336,7 @@ mod tests { .with_commit_type(CommitType::Fix) .with_scope(Scope::parse("api").unwrap()) .with_description(Description::parse("fix bug").unwrap()) + .with_breaking_change(BreakingChange::No) .with_confirm(false); // User cancels at confirmation let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts); @@ -352,9 +359,11 @@ mod tests { // First iteration: scope + description exceed 72 chars combined .with_scope(Scope::parse("very-long-scope-name").unwrap()) .with_description(Description::parse("a".repeat(45)).unwrap()) + .with_breaking_change(BreakingChange::No) // Second iteration: short enough to succeed .with_scope(Scope::empty()) .with_description(Description::parse("short description").unwrap()) + .with_breaking_change(BreakingChange::No) .with_confirm(true); // Clone before moving into workflow so we can inspect emitted messages after @@ -447,6 +456,7 @@ mod tests { .with_commit_type(*commit_type) .with_scope(Scope::empty()) .with_description(Description::parse("test").unwrap()) + .with_breaking_change(BreakingChange::Yes) .with_confirm(true); let workflow = CommitWorkflow::with_prompts( @@ -468,6 +478,7 @@ mod tests { .with_commit_type(CommitType::Feat) .with_scope(Scope::empty()) .with_description(Description::parse("test").unwrap()) + .with_breaking_change(BreakingChange::Yes) .with_confirm(true); let workflow = CommitWorkflow::with_prompts( @@ -484,6 +495,7 @@ mod tests { .with_commit_type(CommitType::Feat) .with_scope(Scope::parse("api").unwrap()) .with_description(Description::parse("test").unwrap()) + .with_breaking_change(BreakingChange::No) .with_confirm(true); let workflow = CommitWorkflow::with_prompts( diff --git a/tests/cli.rs b/tests/cli.rs index 881553a..45724e2 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -1,6 +1,6 @@ use assert_fs::TempDir; #[cfg(feature = "test-utils")] -use jj_cz::{CommitType, Description, MockPrompts, Scope}; +use jj_cz::{BreakingChange, CommitType, Description, MockPrompts, Scope}; use jj_cz::{CommitWorkflow, Error, JjLib}; #[cfg(feature = "test-utils")] use jj_lib::{config::StackedConfig, settings::UserSettings, workspace::Workspace}; @@ -27,6 +27,7 @@ async fn test_happy_path_integration() { .with_commit_type(CommitType::Feat) .with_scope(Scope::empty()) .with_description(Description::parse("add new feature").unwrap()) + .with_breaking_change(BreakingChange::No) .with_confirm(true); // Create a mock executor that tracks calls @@ -85,6 +86,7 @@ async fn test_cancellation() { .with_commit_type(CommitType::Feat) .with_scope(Scope::empty()) .with_description(Description::parse("test").unwrap()) + .with_breaking_change(BreakingChange::No) .with_confirm(true); let workflow = CommitWorkflow::with_prompts(executor, mock_prompts);