From 107f6fb47714815c0d7a9eb663684b492b32b058 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Fri, 20 Mar 2026 16:31:38 +0530 Subject: [PATCH 1/4] fix(ui): truncate long file paths so +/- stats stay visible The file list panel is 20% of terminal width. File paths were rendered at full length with the +N -N change stats appended after. Long paths simply shoved the stats off the right edge into the void, making them invisible. It turns out that rendering a path like "src/some/deeply/nested/module/thing.rs +5 -2" into a 25-column panel without any truncation is... optimistic. Truncate paths with an ellipsis to guarantee the stats suffix fits. Uses char-boundary-safe slicing (not byte indexing, because panicking on non-ASCII paths would be embarrassing), and a digit_count helper to measure stat widths without allocating throwaway format strings every frame. --- src/ui/render.rs | 45 ++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/src/ui/render.rs b/src/ui/render.rs index 3dfb8c1..2f9d8e3 100644 --- a/src/ui/render.rs +++ b/src/ui/render.rs @@ -173,6 +173,9 @@ pub fn render_file_list(f: &mut Frame, app: &App, area: Rect) { return; } + // Available width inside the block: area width minus borders (2) minus highlight symbol width (2) + let inner_width = area.width.saturating_sub(4) as usize; + let items: Vec = app .file_names .iter() @@ -188,7 +191,34 @@ pub fn render_file_list(f: &mut Frame, app: &App, area: Rect) { Style::default().fg(t.fg_normal) }; - let mut spans = vec![Span::styled(file.clone(), name_style)]; + // Calculate how much space the stats suffix needs (no allocations) + let stats_width = if adds > 0 || dels > 0 { + let mut w = 1; // leading space + if adds > 0 { + w += 1 + digit_count(adds); // "+" + digits + } + if adds > 0 && dels > 0 { + w += 1; // space between + } + if dels > 0 { + w += 1 + digit_count(dels); // "-" + digits + } + w + } else { + 0 + }; + + let max_name_width = inner_width.saturating_sub(stats_width); + let char_count = file.chars().count(); + let display_name = if char_count > max_name_width && max_name_width > 1 { + let keep = max_name_width.saturating_sub(1); + let truncated: String = file.chars().take(keep).collect(); + format!("{}\u{2026}", truncated) + } else { + file.clone() + }; + + let mut spans = vec![Span::styled(display_name, name_style)]; if adds > 0 || dels > 0 { spans.push(Span::styled(" ", Style::default())); if adds > 0 { @@ -823,6 +853,19 @@ fn clamp_scroll(app: &mut App, content_area_height: u16) { } } +fn digit_count(n: usize) -> usize { + if n == 0 { + return 1; + } + let mut count = 0; + let mut v = n; + while v > 0 { + count += 1; + v /= 10; + } + count +} + fn count_file_changes(app: &App, file: &str) -> (usize, usize) { if let Some((base, head)) = app.file_changes.get(file) { let dels = base.iter().filter(|(_, l)| l.starts_with('-')).count(); From 1db1bc48849719e18d61d2dc386b2bf68717b09d Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Fri, 20 Mar 2026 16:48:45 +0530 Subject: [PATCH 2/4] fix(ui): truncate file paths from the left so filenames stay visible MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It turns out the original truncation was keeping the directory prefix and chopping the filename — which is exactly backwards. Nobody cares that a path starts with "src/components/"; they care that it ends with "Button.tsx". Showing "src/components/d…" when you could show "…/deep/Button.tsx" is just not helpful. While at it, fix the edge case where max_name_width saturates to 0 or 1 — the old guard (max_name_width > 1) silently skipped truncation entirely, rendering the full name and defeating the whole point of the feature. Now we just show "…" when there's no room. Add unit tests for digit_count because it's a pure function and there's no excuse not to. Also add .DS_Store to .gitignore because apparently that wasn't done yet. Please don't commit macOS metadata files. --- .gitignore | 1 + src/ui/render.rs | 31 +++++++++++++++++++++++++++---- 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index ea8c4bf..0592392 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,2 @@ /target +.DS_Store diff --git a/src/ui/render.rs b/src/ui/render.rs index 2f9d8e3..62f7a09 100644 --- a/src/ui/render.rs +++ b/src/ui/render.rs @@ -210,10 +210,15 @@ pub fn render_file_list(f: &mut Frame, app: &App, area: Rect) { let max_name_width = inner_width.saturating_sub(stats_width); let char_count = file.chars().count(); - let display_name = if char_count > max_name_width && max_name_width > 1 { - let keep = max_name_width.saturating_sub(1); - let truncated: String = file.chars().take(keep).collect(); - format!("{}\u{2026}", truncated) + let display_name = if char_count > max_name_width { + if max_name_width <= 1 { + "\u{2026}".to_string() + } else { + // Keep the tail — the filename is more useful than the directory prefix + let skip = char_count - (max_name_width - 1); + let truncated: String = file.chars().skip(skip).collect(); + format!("\u{2026}{}", truncated) + } } else { file.clone() }; @@ -866,6 +871,24 @@ fn digit_count(n: usize) -> usize { count } +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_digit_count() { + assert_eq!(digit_count(0), 1); + assert_eq!(digit_count(1), 1); + assert_eq!(digit_count(9), 1); + assert_eq!(digit_count(10), 2); + assert_eq!(digit_count(99), 2); + assert_eq!(digit_count(100), 3); + assert_eq!(digit_count(999), 3); + assert_eq!(digit_count(1000), 4); + assert_eq!(digit_count(usize::MAX), usize::MAX.to_string().len()); + } +} + fn count_file_changes(app: &App, file: &str) -> (usize, usize) { if let Some((base, head)) = app.file_changes.get(file) { let dels = base.iter().filter(|(_, l)| l.starts_with('-')).count(); From c52f6f8995425bfeda5b60293a3417b7e59aab93 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Fri, 20 Mar 2026 16:56:24 +0530 Subject: [PATCH 3/4] refactor(ui): use unicode-width for path truncation and kill code duplication MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous truncation logic used chars().count() to measure path widths, which is *wrong* for East Asian characters — a CJK char takes 2 display columns but counts as 1 char. Narrow your terminal enough with a Japanese directory name and the stats still get pushed off screen. Not great. Switch to unicode-width (already a transitive dep via ratatui, so this costs us exactly zero new crate downloads) for proper display column measurement. While at it, the stats width calculation and the stats span building were encoding the same format in two separate places — a classic "change one, forget the other" bug waiting to happen. Extract both into build_file_stats() which returns the spans *and* their width in one pass. The now-orphaned digit_count() goes away with it. Also replace the magic `4` with a named constant because unnamed magic numbers are how you end up debugging layout issues at 2am. --- Cargo.lock | 1 + Cargo.toml | 1 + src/ui/render.rs | 169 ++++++++++++++++++++++++++++------------------- 3 files changed, 102 insertions(+), 69 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 05ad54c..f8b8152 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -616,6 +616,7 @@ dependencies = [ "syntect", "toml", "two-face", + "unicode-width", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 140647c..053893b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,6 +21,7 @@ serde = { version = "1.0.228", features = ["derive"] } syntect = { version = "5.3.0", default-features = false, features = ["default-themes", "regex-fancy"] } two-face = { version = "0.5.1", default-features = false, features = ["syntect-default-fancy"] } toml = "1.0.6" +unicode-width = "0.2" [profile.release] codegen-units = 1 diff --git a/src/ui/render.rs b/src/ui/render.rs index 62f7a09..a786458 100644 --- a/src/ui/render.rs +++ b/src/ui/render.rs @@ -9,6 +9,8 @@ use ratatui::{ Frame, }; +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + use crate::diff::LineChange; use super::rebase::render_rebase_ui; @@ -173,8 +175,9 @@ pub fn render_file_list(f: &mut Frame, app: &App, area: Rect) { return; } - // Available width inside the block: area width minus borders (2) minus highlight symbol width (2) - let inner_width = area.width.saturating_sub(4) as usize; + // Borders (2) + highlight symbol "▌ " (2) + const FILE_LIST_CHROME_WIDTH: u16 = 4; + let inner_width = area.width.saturating_sub(FILE_LIST_CHROME_WIDTH) as usize; let items: Vec = app .file_names @@ -191,57 +194,12 @@ pub fn render_file_list(f: &mut Frame, app: &App, area: Rect) { Style::default().fg(t.fg_normal) }; - // Calculate how much space the stats suffix needs (no allocations) - let stats_width = if adds > 0 || dels > 0 { - let mut w = 1; // leading space - if adds > 0 { - w += 1 + digit_count(adds); // "+" + digits - } - if adds > 0 && dels > 0 { - w += 1; // space between - } - if dels > 0 { - w += 1 + digit_count(dels); // "-" + digits - } - w - } else { - 0 - }; - + let (stat_spans, stats_width) = build_file_stats(adds, dels, t); let max_name_width = inner_width.saturating_sub(stats_width); - let char_count = file.chars().count(); - let display_name = if char_count > max_name_width { - if max_name_width <= 1 { - "\u{2026}".to_string() - } else { - // Keep the tail — the filename is more useful than the directory prefix - let skip = char_count - (max_name_width - 1); - let truncated: String = file.chars().skip(skip).collect(); - format!("\u{2026}{}", truncated) - } - } else { - file.clone() - }; + let display_name = truncate_path(file, max_name_width); let mut spans = vec![Span::styled(display_name, name_style)]; - if adds > 0 || dels > 0 { - spans.push(Span::styled(" ", Style::default())); - if adds > 0 { - spans.push(Span::styled( - format!("+{}", adds), - Style::default().fg(t.fg_added), - )); - } - if adds > 0 && dels > 0 { - spans.push(Span::styled(" ", Style::default())); - } - if dels > 0 { - spans.push(Span::styled( - format!("-{}", dels), - Style::default().fg(t.fg_removed), - )); - } - } + spans.extend(stat_spans); ListItem::new(Line::from(spans)) }) @@ -858,17 +816,58 @@ fn clamp_scroll(app: &mut App, content_area_height: u16) { } } -fn digit_count(n: usize) -> usize { - if n == 0 { - return 1; +/// Build the styled stats spans (e.g. " +3 -1") and return their total display width. +fn build_file_stats<'a>(adds: usize, dels: usize, theme: &Theme) -> (Vec>, usize) { + if adds == 0 && dels == 0 { + return (vec![], 0); } - let mut count = 0; - let mut v = n; - while v > 0 { - count += 1; - v /= 10; + + let mut spans = Vec::new(); + let mut width = 1; // leading space + spans.push(Span::styled(" ", Style::default())); + + if adds > 0 { + let s = format!("+{}", adds); + width += s.len(); + spans.push(Span::styled(s, Style::default().fg(theme.fg_added))); } - count + if adds > 0 && dels > 0 { + width += 1; + spans.push(Span::styled(" ", Style::default())); + } + if dels > 0 { + let s = format!("-{}", dels); + width += s.len(); + spans.push(Span::styled(s, Style::default().fg(theme.fg_removed))); + } + + (spans, width) +} + +/// Truncate a path from the left so it fits within `max_width` display columns, +/// preserving the filename (tail). Uses unicode display widths so East Asian +/// full-width characters are measured correctly. +fn truncate_path(path: &str, max_width: usize) -> String { + let display_width = UnicodeWidthStr::width(path); + if display_width <= max_width { + return path.to_string(); + } + if max_width <= 1 { + return "\u{2026}".to_string(); + } + // Reserve 1 column for the "…" prefix, keep as much of the tail as possible + let target = max_width - 1; + let mut width = 0; + let mut start_byte = path.len(); + for (idx, ch) in path.char_indices().rev() { + let ch_width = UnicodeWidthChar::width(ch).unwrap_or(0); + if width + ch_width > target { + break; + } + width += ch_width; + start_byte = idx; + } + format!("\u{2026}{}", &path[start_byte..]) } #[cfg(test)] @@ -876,16 +875,48 @@ mod tests { use super::*; #[test] - fn test_digit_count() { - assert_eq!(digit_count(0), 1); - assert_eq!(digit_count(1), 1); - assert_eq!(digit_count(9), 1); - assert_eq!(digit_count(10), 2); - assert_eq!(digit_count(99), 2); - assert_eq!(digit_count(100), 3); - assert_eq!(digit_count(999), 3); - assert_eq!(digit_count(1000), 4); - assert_eq!(digit_count(usize::MAX), usize::MAX.to_string().len()); + fn test_truncate_path_no_truncation_needed() { + assert_eq!(truncate_path("src/main.rs", 20), "src/main.rs"); + } + + #[test] + fn test_truncate_path_exact_fit() { + assert_eq!(truncate_path("abcde", 5), "abcde"); + } + + #[test] + fn test_truncate_path_truncates_from_left() { + // 10 chars, max 6 → "…" + last 5 chars + assert_eq!(truncate_path("abcdefghij", 6), "\u{2026}fghij"); + } + + #[test] + fn test_truncate_path_very_narrow() { + assert_eq!(truncate_path("abcdefghij", 1), "\u{2026}"); + assert_eq!(truncate_path("abcdefghij", 0), "\u{2026}"); + } + + #[test] + fn test_truncate_path_width_2() { + // max_width=2 → "…" + 1 char + assert_eq!(truncate_path("abcdef", 2), "\u{2026}f"); + } + + #[test] + fn test_truncate_path_cjk_characters() { + // Each CJK char is 2 display columns wide + // "日本語" = 6 columns, max 5 → "…" + "本語" (4 cols) = 5 + assert_eq!(truncate_path("日本語", 5), "\u{2026}本語"); + } + + #[test] + fn test_truncate_path_mixed_ascii_cjk() { + // "src/日本語.rs" — test that mixed content truncates correctly + let path = "src/日本語.rs"; + let truncated = truncate_path(path, 8); + // Should end with the tail that fits in 7 cols (8 - 1 for "…") + assert!(truncated.starts_with('\u{2026}')); + assert!(UnicodeWidthStr::width(truncated.as_str()) <= 8); } } From 6c766d8da70561a49308e836d89765f9bfbdc6e3 Mon Sep 17 00:00:00 2001 From: bahdotsh Date: Fri, 20 Mar 2026 17:01:18 +0530 Subject: [PATCH 4/4] fix(ui): use unicode-width in build_file_stats and add tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commit switched truncate_path to unicode-width for correct display column measurement, but build_file_stats was still using s.len() — which is byte length, not display width. For the ASCII-only stats strings we produce today that's technically fine, but it's the kind of inconsistency that bites you later when someone changes the format and doesn't realize half the width math uses one measurement system and half uses another. Use UnicodeWidthStr::width() consistently. While at it, add unit tests for build_file_stats covering the basic cases (adds-only, dels-only, both, neither, large numbers) with a round-trip assertion that the returned width actually matches the rendered span width. The kind of test that makes future divergence impossible to miss. --- src/ui/render.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 55 insertions(+), 2 deletions(-) diff --git a/src/ui/render.rs b/src/ui/render.rs index a786458..c90f595 100644 --- a/src/ui/render.rs +++ b/src/ui/render.rs @@ -828,7 +828,7 @@ fn build_file_stats<'a>(adds: usize, dels: usize, theme: &Theme) -> (Vec 0 { let s = format!("+{}", adds); - width += s.len(); + width += UnicodeWidthStr::width(s.as_str()); spans.push(Span::styled(s, Style::default().fg(theme.fg_added))); } if adds > 0 && dels > 0 { @@ -837,7 +837,7 @@ fn build_file_stats<'a>(adds: usize, dels: usize, theme: &Theme) -> (Vec 0 { let s = format!("-{}", dels); - width += s.len(); + width += UnicodeWidthStr::width(s.as_str()); spans.push(Span::styled(s, Style::default().fg(theme.fg_removed))); } @@ -918,6 +918,59 @@ mod tests { assert!(truncated.starts_with('\u{2026}')); assert!(UnicodeWidthStr::width(truncated.as_str()) <= 8); } + + // ── build_file_stats ──────────────────────────────────────────────── + + fn stats_content_width(spans: &[Span]) -> usize { + spans + .iter() + .map(|s| UnicodeWidthStr::width(s.content.as_ref())) + .sum() + } + + #[test] + fn test_build_file_stats_no_changes() { + let t = Theme::dark(); + let (spans, width) = build_file_stats(0, 0, &t); + assert!(spans.is_empty()); + assert_eq!(width, 0); + } + + #[test] + fn test_build_file_stats_adds_only() { + let t = Theme::dark(); + let (spans, width) = build_file_stats(42, 0, &t); + // " +42" → 1 + 3 = 4 + assert_eq!(width, 4); + assert_eq!(stats_content_width(&spans), width); + } + + #[test] + fn test_build_file_stats_dels_only() { + let t = Theme::dark(); + let (spans, width) = build_file_stats(0, 7, &t); + // " -7" → 1 + 2 = 3 + assert_eq!(width, 3); + assert_eq!(stats_content_width(&spans), width); + } + + #[test] + fn test_build_file_stats_adds_and_dels() { + let t = Theme::dark(); + let (spans, width) = build_file_stats(3, 1, &t); + // " +3 -1" → 1 + 2 + 1 + 2 = 6 + assert_eq!(width, 6); + assert_eq!(stats_content_width(&spans), width); + } + + #[test] + fn test_build_file_stats_large_numbers() { + let t = Theme::dark(); + let (spans, width) = build_file_stats(1000, 99999, &t); + // " +1000 -99999" → 1 + 5 + 1 + 6 = 13 + assert_eq!(width, 13); + assert_eq!(stats_content_width(&spans), width); + } } fn count_file_changes(app: &App, file: &str) -> (usize, usize) {