From 20753608e861e4c9f0535dae5e3b75d3a167000f Mon Sep 17 00:00:00 2001 From: teernisse Date: Fri, 13 Mar 2026 11:13:33 -0400 Subject: [PATCH] fix(cli): flex-col min-width clamping and formatting consistency - render.rs: clamp flex column width to min(min_flex, natural) instead of a hardcoded 20, preventing layout overflow when natural width is small; rewrites flex_width test to be terminal-independent - list/issues.rs: adopt .flex_col() builder on table construction - list/mrs.rs, list/notes.rs: consolidate multi-line StyledCell::styled calls to single-line format - explain.rs: adopt flex_width() for related-issue title truncation, consolidate multi-line formatting --- src/cli/commands/explain.rs | 17 +++++++++--- src/cli/commands/list/issues.rs | 5 +++- src/cli/commands/list/mrs.rs | 5 +--- src/cli/commands/list/notes.rs | 5 +--- src/cli/render.rs | 48 +++++++++++++++++---------------- 5 files changed, 44 insertions(+), 36 deletions(-) diff --git a/src/cli/commands/explain.rs b/src/cli/commands/explain.rs index 3d97a54..7887902 100644 --- a/src/cli/commands/explain.rs +++ b/src/cli/commands/explain.rs @@ -1137,7 +1137,11 @@ pub fn print_explain(result: &ExplainResult) { // Entity header let (type_label, ref_style, ref_str) = match result.entity.entity_type.as_str() { - "issue" => ("Issue", Theme::issue_ref(), format!("#{}", result.entity.iid)), + "issue" => ( + "Issue", + Theme::issue_ref(), + format!("#{}", result.entity.iid), + ), "merge_request" => ("MR", Theme::mr_ref(), format!("!{}", result.entity.iid)), _ => ( result.entity.entity_type.as_str(), @@ -1246,8 +1250,10 @@ pub fn print_explain(result: &ExplainResult) { println!( " {} by {} ({} notes, last: {})", Theme::dim().render(&t.discussion_id), - Theme::username() - .render(&format!("@{}", t.started_by.as_deref().unwrap_or("unknown"))), + Theme::username().render(&format!( + "@{}", + t.started_by.as_deref().unwrap_or("unknown") + )), t.note_count, Theme::dim().render(&to_relative(&t.last_note_at)) ); @@ -1300,7 +1306,10 @@ pub fn print_explain(result: &ExplainResult) { println!( " {arrow} {} {}{state_str} ({})", ref_style.render(&format!("{ref_prefix}{}", ri.iid)), - render::truncate(ri.title.as_deref().unwrap_or("(untitled)"), render::flex_width(30, 20)), + render::truncate( + ri.title.as_deref().unwrap_or("(untitled)"), + render::flex_width(30, 20) + ), Theme::dim().render(&ri.reference_type) ); } diff --git a/src/cli/commands/list/issues.rs b/src/cli/commands/list/issues.rs index f2ced89..1f53069 100644 --- a/src/cli/commands/list/issues.rs +++ b/src/cli/commands/list/issues.rs @@ -359,7 +359,10 @@ pub fn print_list_issues(result: &ListResult) { } headers.extend(["Assignee", "Labels", "Disc", "Updated"]); - let mut table = LoreTable::new().headers(&headers).align(0, Align::Right).flex_col(1); + let mut table = LoreTable::new() + .headers(&headers) + .align(0, Align::Right) + .flex_col(1); for issue in &result.issues { let title = issue.title.clone(); diff --git a/src/cli/commands/list/mrs.rs b/src/cli/commands/list/mrs.rs index fa06aa3..a2380a2 100644 --- a/src/cli/commands/list/mrs.rs +++ b/src/cli/commands/list/mrs.rs @@ -356,10 +356,7 @@ pub fn print_list_mrs(result: &MrListResult) { StyledCell::styled(format!("!{}", mr.iid), Theme::info()), StyledCell::plain(title), state_cell, - StyledCell::styled( - format!("@{}", mr.author_username), - Theme::accent(), - ), + StyledCell::styled(format!("@{}", mr.author_username), Theme::accent()), StyledCell::styled(branches, Theme::info()), discussions, StyledCell::styled(relative_time, Theme::dim()), diff --git a/src/cli/commands/list/notes.rs b/src/cli/commands/list/notes.rs index 981f3c9..a5bb039 100644 --- a/src/cli/commands/list/notes.rs +++ b/src/cli/commands/list/notes.rs @@ -175,10 +175,7 @@ pub fn print_list_notes(result: &NoteListResult) { table.add_row(vec![ StyledCell::styled(note.gitlab_id.to_string(), Theme::info()), - StyledCell::styled( - format!("@{}", note.author_username), - Theme::accent(), - ), + StyledCell::styled(format!("@{}", note.author_username), Theme::accent()), StyledCell::plain(note_type), StyledCell::plain(body), StyledCell::plain(path), diff --git a/src/cli/render.rs b/src/cli/render.rs index 2f7879b..724170c 100644 --- a/src/cli/render.rs +++ b/src/cli/render.rs @@ -929,7 +929,8 @@ impl Table { + self.indent; let available = terminal_width().saturating_sub(fixed_sum); let natural = widths[fc]; - widths[fc] = available.clamp(20, natural); + let min_flex = 20.min(natural); + widths[fc] = available.clamp(min_flex, natural); } let mut out = String::new(); @@ -1385,12 +1386,12 @@ mod tests { let saved = std::env::var("COLUMNS").ok(); unsafe { std::env::set_var("COLUMNS", "60") }; - let mut table = Table::new() - .headers(&["ID", "Title", "State"]) - .flex_col(1); + let mut table = Table::new().headers(&["ID", "Title", "State"]).flex_col(1); table.add_row(vec![ StyledCell::plain("1"), - StyledCell::plain("A very long title that would normally extend beyond the terminal width"), + StyledCell::plain( + "A very long title that would normally extend beyond the terminal width", + ), StyledCell::plain("open"), ]); let result = table.render(); @@ -1419,9 +1420,7 @@ mod tests { let saved = std::env::var("COLUMNS").ok(); unsafe { std::env::set_var("COLUMNS", "120") }; - let mut table = Table::new() - .headers(&["ID", "Title", "State"]) - .flex_col(1); + let mut table = Table::new().headers(&["ID", "Title", "State"]).flex_col(1); table.add_row(vec![ StyledCell::plain("1"), StyledCell::plain("Short title"), @@ -1454,23 +1453,26 @@ mod tests { } #[test] - fn flex_width_respects_terminal() { - let saved = std::env::var("COLUMNS").ok(); - unsafe { std::env::set_var("COLUMNS", "100") }; + fn flex_width_respects_min() { + // flex_width = max(terminal_width() - overhead, min) + // With very large overhead, result should be at least `min` + let result = flex_width(10_000, 25); + assert_eq!( + result, 25, + "should clamp to min when overhead exceeds width" + ); - // With 30 overhead on a 100-col terminal, should get 70 - assert_eq!(flex_width(30, 20), 70); + // With zero overhead, result should be terminal_width() + let result = flex_width(0, 1); + assert!(result >= 1, "should be at least min"); - // With 90 overhead, should get min of 20 (not 10) - assert_eq!(flex_width(90, 20), 20); - - // With 50 overhead, should get 50 - assert_eq!(flex_width(50, 20), 50); - - match saved { - Some(v) => unsafe { std::env::set_var("COLUMNS", v) }, - None => unsafe { std::env::remove_var("COLUMNS") }, - } + // Monotonicity: more overhead → smaller or equal result + let a = flex_width(10, 20); + let b = flex_width(50, 20); + assert!( + a >= b, + "more overhead should yield smaller width: {a} vs {b}" + ); } // ── GlyphMode ──