feat(prompt): add support for wide characters in prompt preview

Cargo.lock

@@ -1664,6 +1664,7 @@ dependencies = [
  "textwrap",
  "thiserror",
  "tokio",
+ "unicode-width",
 ]
 
 [[package]]

Cargo.toml

@@ -31,6 +31,7 @@ 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"
+unicode-width = "0.2.2"
 
 [dev-dependencies]
 assert_cmd = "2.1.2"

src/commit/types/footer.rs

@@ -2,10 +2,7 @@ pub trait Footer {
     fn prefix(&self) -> &str;
     fn note(&self) -> &str;
 
-    fn as_footer(&self) -> String
-    where
-        Self: std::fmt::Debug,
-    {
+    fn as_footer(&self) -> String {
         let default = format!("{}: {}", self.prefix(), self.note());
         if default.chars().count() > 72 {
             textwrap::wrap(&default, 71).join("\n ")

src/commit/types/scope.rs

@@ -18,9 +18,9 @@ impl Scope {
         if value.is_empty() {
             return Ok(Self::empty());
         }
-        if value.len() > Self::MAX_LENGTH {
+        if value.chars().count() > Self::MAX_LENGTH {
             return Err(ScopeError::TooLong {
-                actual: value.len(),
+                actual: value.chars().count(),
                 max: Self::MAX_LENGTH,
             });
         }
@@ -458,10 +458,6 @@ mod tests {
         assert!(msg.contains("30"));
     }
 
-    // =========================================================================
-    // header_segment() / header_segment_len() tests
-    // =========================================================================
-
     /// Test header_segment() returns empty string for empty scope
     #[test]
     fn header_segment_empty_scope_returns_empty_string() {
@@ -517,4 +513,50 @@ mod tests {
             );
         }
     }
+
+    /// A scope whose byte count exceeds MAX_LENGTH but whose char
+    /// count does not must be rejected with InvalidCharacter, not
+    /// TooLong.
+    ///
+    /// Before the fix the byte-based `.len()` check fired first,
+    /// producing a misleading "too long" error for a string that is
+    /// actually within the limit.
+    #[test]
+    fn length_limit_uses_char_count_not_byte_count() {
+        // "ñ" is 2 bytes in UTF-8; 16 × "ñ" = 16 chars, 32 bytes.
+        // char count 16 ≤ 30  →  length check passes
+        // regex rejects "ñ"   →  should return InvalidCharacter, not TooLong
+        let input = "ñ".repeat(16);
+        assert_eq!(input.chars().count(), 16, "sanity: 16 chars");
+        assert_eq!(input.len(), 32, "sanity: 32 bytes");
+
+        let result = Scope::parse(&input);
+        assert!(result.is_err());
+        assert_eq!(
+            result.unwrap_err(),
+            ScopeError::InvalidCharacter('ñ'),
+            "expected InvalidCharacter('ñ') for a 16-char / 32-byte input, not TooLong",
+        );
+    }
+
+    /// The actual length reported in TooLong must be the char count,
+    /// not the byte count.
+    ///
+    /// "a".repeat(30) + "é" is 31 chars and 32 bytes. The length
+    /// check should fire on char count (31 > 30) and report actual =
+    /// 31.
+    #[test]
+    fn too_long_error_actual_reports_char_count_not_byte_count() {
+        // 30 ASCII 'a' + 1 two-byte 'é' = 31 chars, 32 bytes
+        let input = "a".repeat(30) + "é";
+        assert_eq!(input.chars().count(), 31, "sanity: 31 chars");
+        assert_eq!(input.len(), 32, "sanity: 32 bytes");
+
+        let result = Scope::parse(&input);
+        assert_eq!(
+            result.unwrap_err(),
+            ScopeError::TooLong { actual: 31, max: 30 },
+            "actual should be the char count (31), not the byte count (32)",
+        );
+    }
 }

src/prompts/prompter.rs

@@ -6,6 +6,7 @@
 //! in production while accepting mock implementations in tests.
 
 use inquire::{Confirm, Text};
+use unicode_width::UnicodeWidthStr;
 
 use crate::{
     commit::types::{BreakingChange, CommitType, Description, Scope},
@@ -40,14 +41,19 @@ pub trait Prompter: Send + Sync {
 }
 
 fn format_message_box(message: &str) -> String {
-    let preview_width = 72 + 2; // max width + space padding
+    let preview_width = message
+        .split('\n')
+        .map(|line| line.width())
+        .max()
+        .unwrap_or(0)
+        .max(72);
     let mut lines: Vec<String> = 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!("┌{}┐", "─".repeat(preview_width + 2)));
+    for line in message.split('\n') {
+        let padding = preview_width.saturating_sub(line.width());
         lines.push(format!("│ {line}{:padding$} │", ""));
     }
-    lines.push(format!("└{}┘", "─".repeat(preview_width)));
+    lines.push(format!("└{}┘", "─".repeat(preview_width + 2)));
     lines.join("\n")
 }
 
@@ -270,4 +276,91 @@ mod tests {
         let expected = format!("│ {line_72} │");
         assert_eq!(lines[1], expected);
     }
+
+    /// A single CJK character (display width 2) is padded as if it occupies 2 columns,
+    /// not 1 — so the right-hand padding is 70 spaces, not 71
+    #[test]
+    fn format_message_box_single_cjk_char() {
+        let result = format_message_box("字");
+        let lines: Vec<&str> = result.split('\n').collect();
+        let expected = format!("│ 字{:70} │", "");
+        assert_eq!(lines[1], expected);
+    }
+
+    /// A single emoji (display width 2) is padded as if it occupies 2 columns
+    #[test]
+    fn format_message_box_single_emoji() {
+        let result = format_message_box("🦀");
+        let lines: Vec<&str> = result.split('\n').collect();
+        let expected = format!("│ 🦀{:70} │", "");
+        assert_eq!(lines[1], expected);
+    }
+
+    /// Mixed ASCII and CJK: padding accounts for the display width of the whole line
+    ///
+    /// "feat: " = 6 display cols, "漢字" = 4 display cols → total 10, padding = 62
+    #[test]
+    fn format_message_box_mixed_ascii_and_cjk() {
+        let result = format_message_box("feat: 漢字");
+        let lines: Vec<&str> = result.split('\n').collect();
+        let expected = format!("│ feat: 漢字{:62} │", "");
+        assert_eq!(lines[1], expected);
+    }
+
+    /// When a line exceeds 72 display columns the border expands to fit (width + 2 dashes)
+    #[test]
+    fn format_message_box_border_expands_beyond_72() {
+        let line_73 = "a".repeat(73);
+        let result = format_message_box(&line_73);
+        let lines: Vec<&str> = result.split('\n').collect();
+        let dashes = "─".repeat(75); // 73 + 2
+        assert_eq!(lines[0], format!("┌{dashes}┐"));
+        assert_eq!(lines[lines.len() - 1], format!("└{dashes}┘"));
+    }
+
+    /// A line that sets the box width gets zero right-hand padding
+    #[test]
+    fn format_message_box_widest_line_has_no_padding() {
+        let line_73 = "a".repeat(73);
+        let result = format_message_box(&line_73);
+        let lines: Vec<&str> = result.split('\n').collect();
+        assert_eq!(lines[1], format!("│ {line_73} │"));
+    }
+
+    /// In a multi-line message, shorter lines are padded out to match the widest line
+    #[test]
+    fn format_message_box_shorter_lines_padded_to_widest() {
+        let long_line = "a".repeat(80);
+        let result = format_message_box(&format!("{long_line}\nshort"));
+        let lines: Vec<&str> = result.split('\n').collect();
+        assert_eq!(lines[1], format!("│ {long_line} │"));
+        assert_eq!(lines[2], format!("│ short{:75} │", "")); // 80 - 5 = 75
+    }
+
+    /// All rows have equal char count when the box expands beyond 72
+    #[test]
+    fn format_message_box_all_rows_same_width_when_expanded() {
+        let long_line = "a".repeat(80);
+        let result = format_message_box(&format!("{long_line}\nshort"));
+        let widths: Vec<usize> = 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
+        );
+    }
+
+    /// Wide characters can also trigger box expansion beyond 72 columns
+    ///
+    /// 37 CJK characters × 2 display columns = 74 display columns → border uses 76 dashes
+    #[test]
+    fn format_message_box_wide_chars_expand_box() {
+        let wide_line = "字".repeat(37); // 74 display cols
+        let result = format_message_box(&wide_line);
+        let lines: Vec<&str> = result.split('\n').collect();
+        let dashes = "─".repeat(76); // 74 + 2
+        assert_eq!(lines[0], format!("┌{dashes}┐"));
+        assert_eq!(lines[1], format!("│ {wide_line} │")); // no padding
+    }
 }

src/prompts/workflow.rs

@@ -132,7 +132,7 @@ impl<J: JjExecutor, P: Prompter> CommitWorkflow<J, P> {
             commit_type,
             scope.clone(),
             description.clone(),
-            crate::commit::types::BreakingChange::No,
+            breaking_change,
         ) {
             Ok(cc) => cc,
             Err(CommitMessageError::FirstLineTooLong { actual, max }) => {
@@ -521,4 +521,99 @@ mod tests {
         let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
         assert!(matches!(workflow, CommitWorkflow { .. }));
     }
+
+    /// Preview_and_confirm must forward BreakingChange::Yes to
+    /// ConventionalCommit::new(), producing a commit whose string
+    /// contains '!'.
+    ///
+    /// Before the fix the parameter was ignored and
+    /// BreakingChange::No was hard-coded, so a confirmed
+    /// breaking-change commit was silently applied without the '!'
+    /// marker.
+    #[tokio::test]
+    async fn preview_and_confirm_forwards_breaking_change_yes() {
+        let mock_executor = MockJjExecutor::new();
+        let mock_prompts = MockPrompts::new().with_confirm(true);
+        let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
+
+        let result = workflow
+            .preview_and_confirm(
+                CommitType::Feat,
+                Scope::empty(),
+                Description::parse("remove old API").unwrap(),
+                BreakingChange::Yes,
+            )
+            .await;
+
+        assert!(result.is_ok(), "expected Ok, got: {:?}", result);
+        let message = result.unwrap().to_string();
+        assert!(
+            message.contains("feat!:"),
+            "expected '!' marker in described message, got: {:?}",
+            message,
+        );
+    }
+
+    /// Preview_and_confirm must forward BreakingChange::WithNote,
+    /// producing a commit with both the '!' header marker and the
+    /// BREAKING CHANGE footer.
+    #[tokio::test]
+    async fn preview_and_confirm_forwards_breaking_change_with_note() {
+        let mock_executor = MockJjExecutor::new();
+        let mock_prompts = MockPrompts::new().with_confirm(true);
+        let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
+
+        let breaking_change: BreakingChange = "removes legacy endpoint".into();
+        let result = workflow
+            .preview_and_confirm(
+                CommitType::Feat,
+                Scope::empty(),
+                Description::parse("drop legacy API").unwrap(),
+                breaking_change,
+            )
+            .await;
+
+        assert!(result.is_ok(), "expected Ok, got: {:?}", result);
+        let message = result.unwrap().to_string();
+        assert!(
+            message.contains("feat!:"),
+            "expected '!' header marker in message, got: {:?}",
+            message,
+        );
+        assert!(
+            message.contains("BREAKING CHANGE:"),
+            "expected BREAKING CHANGE footer in message, got: {:?}",
+            message,
+        );
+    }
+
+    /// The message passed to executor.describe() must include the '!'
+    /// marker when the user selects a breaking change.
+    ///
+    /// This test exercises the full run() path and inspects what was
+    /// actually handed to the jj executor, which is the authoritative
+    /// check that the described commit is correct.
+    #[tokio::test]
+    async fn full_workflow_describes_commit_with_breaking_change_marker() {
+        let mock_executor = MockJjExecutor::new().with_is_repo_response(Ok(true));
+        let mock_prompts = MockPrompts::new()
+            .with_commit_type(CommitType::Feat)
+            .with_scope(Scope::empty())
+            .with_description(Description::parse("remove old API").unwrap())
+            .with_breaking_change(BreakingChange::Yes)
+            .with_confirm(true);
+
+        let workflow = CommitWorkflow::with_prompts(mock_executor, mock_prompts);
+        let result: Result<(), Error> = workflow.run().await;
+
+        assert!(result.is_ok(), "expected workflow to succeed, got: {:?}", result);
+
+        let messages = workflow.executor.describe_messages();
+        assert_eq!(messages.len(), 1, "expected exactly one describe() call");
+        assert!(
+            messages[0].contains("feat!:"),
+            "expected '!' marker in the described message, got: {:?}",
+            messages[0],
+        );
+    }
 }