From f480137ad6f92b770d197a6e15c0f83cf4fbf60a Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 01:01:27 -0700 Subject: [PATCH 1/8] fix: table cell rendering and unsaved-changes false positive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Render inline markdown (links, bold, italic, code, wiki links) inside table cells and wire up Enter-to-open for the first [text](url) per cell, including the "Open ↗" hint on the cursor'd row. Previously table cells rendered their raw source, so `[label](url)` appeared literally and columns reserved space for hidden markup. Scoped to simple scenarios: one markdown link per cell, no bare-URL autolinking. Also fix has_unsaved_changes to compare line-by-line with the same str::lines() semantics that enter_edit_mode uses. The old raw-string compare fired a false positive whenever a note ended in "\n" (most files), making edit-then-Esc round-trips prompt the unsaved-changes dialog even when no edits were made. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/state.rs | 60 +++++++++++++++++++++++++++++++++++++++++++---- src/ui/content.rs | 55 +++++++++++++++++++++++++++++++++++-------- src/ui/mod.rs | 1 + 3 files changed, 102 insertions(+), 14 deletions(-) diff --git a/src/app/state.rs b/src/app/state.rs index 44f4a81..ba4d4e3 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -2202,7 +2202,7 @@ impl App { if !is_sep { for (col_idx, cell) in cells.iter().enumerate() { if col_idx < column_widths.len() { - column_widths[col_idx] = column_widths[col_idx].max(cell.chars().count()); + column_widths[col_idx] = column_widths[col_idx].max(crate::ui::cell_visible_width(cell)); } } } @@ -2638,12 +2638,60 @@ impl App { !self.item_all_links_at(self.content_cursor).is_empty() } + /// Extract `[text](url)` links from each table cell and map positions into the + /// row's rendered column space. Simple scenarios only: at most one markdown link + /// per cell, and the link's pre-prefix can contain inline formatting but no other links. + fn extract_simple_table_links(cells: &[String], column_widths: &[usize]) -> Vec<(String, String, usize, usize)> { + let mut links = Vec::new(); + let mut col_cursor = 0usize; // column within content area (after ` │` prefix) + for (i, cell) in cells.iter().enumerate() { + let width = column_widths.get(i).copied().unwrap_or_else(|| crate::ui::cell_visible_width(cell)); + let visible = crate::ui::cell_visible_width(cell); + let left_pad = width.saturating_sub(visible) / 2; + let cell_start = col_cursor + 1 /* leading space */ + left_pad; + + if let Some(br_start) = cell.find('[') { + if !cell[br_start..].starts_with("[[") { + if let Some(br_end_rel) = cell[br_start + 1..].find(']') { + let br_end = br_start + 1 + br_end_rel; + if cell[br_end..].starts_with("](") { + if let Some(pr_end_rel) = cell[br_end + 2..].find(')') { + let pr_end = br_end + 2 + pr_end_rel; + let label = &cell[br_start + 1..br_end]; + let url = &cell[br_end + 2..pr_end]; + if !url.is_empty() { + let display = if label.is_empty() { url.to_string() } else { label.to_string() }; + let pre_visible = crate::ui::cell_visible_width(&cell[..br_start]); + let start = cell_start + pre_visible; + let end = start + display.chars().count(); + links.push((display, url.to_string(), start, end)); + } + } + } + } + } + } + + col_cursor += 1 + width + 1; // " " + width + " " + if i + 1 < cells.len() { + col_cursor += 1; // "│" between cells + } + } + links + } + /// Extract all links and images from a specific content item as (text, url, start_col, end_col) tuples /// The columns are character positions in the rendered line (after prefix like "▶ " or "• ") pub fn item_links_at(&self, index: usize) -> Vec<(String, String, usize, usize)> { let text = match self.content_items.get(index) { Some(ContentItem::TextLine(line)) => line.as_str(), Some(ContentItem::TaskItem { text, .. }) => text.as_str(), + Some(ContentItem::TableRow { cells, is_separator, column_widths, .. }) => { + if *is_separator { + return Vec::new(); + } + return Self::extract_simple_table_links(cells, column_widths); + } _ => return Vec::new(), }; @@ -2940,7 +2988,8 @@ impl App { } len } - Some(ContentItem::TaskItem { .. }) => 6, + Some(ContentItem::TaskItem { .. }) => 6, + Some(ContentItem::TableRow { .. }) => 3, // " " cursor indicator + "│" left border _ => 2, } } @@ -4591,8 +4640,11 @@ impl App { pub fn has_unsaved_changes(&self) -> bool { if let Some(note) = self.notes.get(self.selected_note) { - let current_content = self.editor.lines().join("\n"); - current_content != note.content + // Compare line-by-line with the same semantics `enter_edit_mode` uses + // (`str::lines()` drops trailing newlines). Comparing the raw strings instead + // fires a false positive whenever the file ends with "\n" — which is most files. + let note_lines: Vec<&str> = note.content.lines().collect(); + self.editor.lines() != note_lines } else { false } diff --git a/src/ui/content.rs b/src/ui/content.rs index deaaedb..c6b42bf 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -441,7 +441,10 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { } } ContentItem::TableRow { cells, is_separator, is_header, column_widths } => { - render_table_row(f, &app.theme, &cells, is_separator, is_header, &column_widths, chunks[chunk_idx], is_cursor_line); + let has_link = !is_separator + && (is_cursor_line || is_hovered) + && !app.item_links_at(item_idx).is_empty(); + render_table_row(f, &app.theme, &cells, is_separator, is_header, &column_widths, chunks[chunk_idx], is_cursor_line, has_link); } ContentItem::Details { summary, content_lines, id } => { let is_open = app.details_open_states.get(&id).copied().unwrap_or(false); @@ -464,6 +467,13 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { } } +/// Visible width of a table cell after inline markdown shrinks +/// (e.g. `[label](url)` -> `label`). Used both for column layout and render padding. +pub(crate) fn cell_visible_width(cell: &str) -> usize { + let total = cell.chars().count(); + total.saturating_sub(calc_formatting_shrinkage(cell, total)) +} + /// Calculate how many characters are removed by inline formatting before a given position /// This accounts for **bold**, *italic*, ~~strikethrough~~, `code`, [[wiki links]], and [markdown](links) fn calc_formatting_shrinkage(text: &str, up_to_pos: usize) -> usize { @@ -1791,6 +1801,7 @@ fn render_table_row( column_widths: &[usize], area: Rect, is_cursor: bool, + has_link: bool, ) { let cursor_indicator = if is_cursor { "▶ " } else { " " }; let border_color = theme.border; @@ -1809,27 +1820,51 @@ fn render_table_row( spans.push(Span::styled(dashes, Style::default().fg(border_color))); } } else { + let default_style = if is_header { + Style::default().fg(theme.info).add_modifier(Modifier::BOLD) + } else { + Style::default().fg(theme.foreground) + }; + let text_color = theme.content.text; + for (i, cell) in cells.iter().enumerate() { if i > 0 { spans.push(Span::styled("│", Style::default().fg(border_color))); } let expanded_cell = expand_tabs(cell); - let width = column_widths.get(i).copied().unwrap_or(expanded_cell.chars().count()); - let cell_content = format!(" {:^width$} ", expanded_cell, width = width); - - let cell_style = if is_header { - Style::default().fg(theme.info).add_modifier(Modifier::BOLD) - } else { - Style::default().fg(theme.foreground) - }; + let visible = cell_visible_width(&expanded_cell); + let width = column_widths.get(i).copied().unwrap_or(visible); + let pad = width.saturating_sub(visible); + let left_pad = pad / 2; + let right_pad = pad - left_pad; + + spans.push(Span::styled(format!(" {}", " ".repeat(left_pad)), default_style)); + + // `parse_inline_formatting` is generic over the validator's closure type. + // We don't need wiki-link validation in tables — a fn-pointer type satisfies `F: Fn(&str) -> bool`. + let inline = parse_inline_formatting:: bool>(&expanded_cell, theme, None, None); + for sp in inline { + // Restyle plain-text spans (those the inline parser emits with the default body color) + // so headers get bold+info. Links/code/wiki keep their own styles. + let is_plain = sp.style.bg.is_none() + && sp.style.add_modifier.is_empty() + && sp.style.sub_modifier.is_empty() + && (sp.style.fg.is_none() || sp.style.fg == Some(text_color.into())); + let style = if is_plain { default_style } else { sp.style }; + spans.push(Span::styled(sp.content.into_owned(), style)); + } - spans.push(Span::styled(cell_content, cell_style)); + spans.push(Span::styled(format!("{} ", " ".repeat(right_pad)), default_style)); } } spans.push(Span::styled("│", Style::default().fg(border_color))); + if has_link { + spans.push(Span::styled(" Open ↗", Style::default().fg(theme.content.link))); + } + let styled_line = Line::from(spans); let style = if is_cursor { diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 6053c09..a6b17d4 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -20,6 +20,7 @@ use ratatui::{ use crate::app::{App, ContextMenuState, DialogState, SearchPickerState, Mode, WikiAutocompleteState}; pub use content::render_content; +pub(crate) use content::cell_visible_width; pub use dialogs::{ render_create_folder_dialog, render_create_note_dialog, render_create_note_in_folder_dialog, render_create_wiki_note_dialog, render_delete_confirm_dialog, render_delete_folder_confirm_dialog, From e382e96c3e9e05603a6cf97404eb1f075315d9a5 Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 01:33:35 -0700 Subject: [PATCH 2/8] feat: table column alignment; fix link-shrinkage off-by-one Parse :--- / ---: / :---: in the separator row to derive per-column alignment. Fix calc_formatting_shrinkage counting `[x](url)` as url_len+3 instead of url_len+4, which drifted borders by N on rows with N links. Adds unit tests for cell_visible_width, extract_simple_table_links, and Alignment::from_separator_cell. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/state.rs | 124 ++++++++++++++++++++++++++++++++++++++++++++-- src/ui/content.rs | 87 +++++++++++++++++++++++++++----- 2 files changed, 194 insertions(+), 17 deletions(-) diff --git a/src/app/state.rs b/src/app/state.rs index ba4d4e3..65eecc7 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -212,6 +212,27 @@ pub struct ImageState { pub path: String, } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum Alignment { + Left, + Center, + Right, +} + +impl Alignment { + /// Classify a GFM table separator cell (e.g. `:---`, `---:`, `:---:`, `---`) + /// into its alignment. Any cell without a leading `:` is treated as Left + /// (matches GFM's default-left convention). + pub fn from_separator_cell(cell: &str) -> Alignment { + let t = cell.trim(); + match (t.starts_with(':'), t.ends_with(':')) { + (true, true) => Alignment::Center, + (false, true) => Alignment::Right, + _ => Alignment::Left, + } + } +} + #[derive(Debug, Clone)] pub enum ContentItem { TextLine(String), @@ -219,7 +240,7 @@ pub enum ContentItem { CodeLine(String), CodeFence(String), TaskItem { text: String, checked: bool, line_index: usize }, - TableRow { cells: Vec, is_separator: bool, is_header: bool, column_widths: Vec }, + TableRow { cells: Vec, is_separator: bool, is_header: bool, column_widths: Vec, alignments: Vec }, Details { summary: String, content_lines: Vec, id: usize }, FrontmatterLine { key: String, value: String }, FrontmatterDelimiter, @@ -2214,6 +2235,20 @@ impl App { let separator_idx = table_rows.iter().position(|(_, is_sep)| *is_sep); + // Derive per-column alignment from the separator row. Tables without + // a separator fall back to Left (GFM default). + let mut alignments: Vec = vec![Alignment::Left; num_cols]; + if let Some(sep_idx) = separator_idx { + if let Some((sep_cells, _)) = table_rows.get(sep_idx) { + for (col_idx, cell) in sep_cells.iter().enumerate() { + if col_idx >= alignments.len() { + break; + } + alignments[col_idx] = Alignment::from_separator_cell(cell); + } + } + } + for (row_idx, (cells, is_separator)) in table_rows.into_iter().enumerate() { let is_header = separator_idx.map(|sep_idx| row_idx < sep_idx).unwrap_or(false); self.content_items.push(ContentItem::TableRow { @@ -2221,6 +2256,7 @@ impl App { is_separator, is_header, column_widths: column_widths.clone(), + alignments: alignments.clone(), }); self.content_item_source_lines.push(table_start_line + row_idx); } @@ -2641,13 +2677,19 @@ impl App { /// Extract `[text](url)` links from each table cell and map positions into the /// row's rendered column space. Simple scenarios only: at most one markdown link /// per cell, and the link's pre-prefix can contain inline formatting but no other links. - fn extract_simple_table_links(cells: &[String], column_widths: &[usize]) -> Vec<(String, String, usize, usize)> { + fn extract_simple_table_links(cells: &[String], column_widths: &[usize], alignments: &[Alignment]) -> Vec<(String, String, usize, usize)> { let mut links = Vec::new(); let mut col_cursor = 0usize; // column within content area (after ` │` prefix) for (i, cell) in cells.iter().enumerate() { let width = column_widths.get(i).copied().unwrap_or_else(|| crate::ui::cell_visible_width(cell)); let visible = crate::ui::cell_visible_width(cell); - let left_pad = width.saturating_sub(visible) / 2; + let pad = width.saturating_sub(visible); + let alignment = alignments.get(i).copied().unwrap_or(Alignment::Left); + let left_pad = match alignment { + Alignment::Left => 0, + Alignment::Right => pad, + Alignment::Center => pad / 2, + }; let cell_start = col_cursor + 1 /* leading space */ + left_pad; if let Some(br_start) = cell.find('[') { @@ -2686,11 +2728,11 @@ impl App { let text = match self.content_items.get(index) { Some(ContentItem::TextLine(line)) => line.as_str(), Some(ContentItem::TaskItem { text, .. }) => text.as_str(), - Some(ContentItem::TableRow { cells, is_separator, column_widths, .. }) => { + Some(ContentItem::TableRow { cells, is_separator, column_widths, alignments, .. }) => { if *is_separator { return Vec::new(); } - return Self::extract_simple_table_links(cells, column_widths); + return Self::extract_simple_table_links(cells, column_widths, alignments); } _ => return Vec::new(), }; @@ -5913,3 +5955,75 @@ fn fuzzy_match(text: &str, query: &str) -> Option { None } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn alignment_from_separator_cell_classifies_each_form() { + assert_eq!(Alignment::from_separator_cell("---"), Alignment::Left); + assert_eq!(Alignment::from_separator_cell(":---"), Alignment::Left); + assert_eq!(Alignment::from_separator_cell("---:"), Alignment::Right); + assert_eq!(Alignment::from_separator_cell(":---:"), Alignment::Center); + // Surrounding whitespace should not change classification. + assert_eq!(Alignment::from_separator_cell(" :---: "), Alignment::Center); + } + + #[test] + fn extract_simple_table_links_single_link_in_second_cell() { + // Row: "| Name | [Top 5](https://x.test) |" + // Cells (already trimmed during parse): ["Name", "[Top 5](https://x.test)"] + // Column widths follow visible width: cell 0 = 4, cell 1 = 6 ("Top 5"). + let cells = vec!["Name".to_string(), "[Top 5](https://x.test)".to_string()]; + let widths = vec![4, 6]; + let alignments = vec![Alignment::Left, Alignment::Left]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + + assert_eq!(links.len(), 1); + let (label, url, start, end) = &links[0]; + assert_eq!(label, "Top 5"); + assert_eq!(url, "https://x.test"); + // Layout within content area (prefix ` │` not counted): + // cell 0 occupies " Name " (cols 0..=5), "│" at 6, cell 1 opens at 7 with " " leading. + // Left-aligned, so label starts at 7 + 1 = 8. + assert_eq!(*start, 8); + assert_eq!(*end, 8 + "Top 5".chars().count()); + } + + #[test] + fn extract_simple_table_links_respects_right_alignment() { + // Right-aligned cell: label sits flush against the right edge. + // Cells: ["X", "[a](u)"]; widths: [3, 5]; alignment: [Left, Right]. + // Cell 0 occupies " X " (1 + width 3 + 1 = 5 chars) + "│" -> col_cursor = 6. + // Cell 1 visible = 1 ("a"), pad = 4, Right -> left_pad = 4. + // Link starts at col_cursor(6) + 1 (leading space) + 4 (left_pad) = 11. + let cells = vec!["X".to_string(), "[a](u)".to_string()]; + let widths = vec![3, 5]; + let alignments = vec![Alignment::Left, Alignment::Right]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + assert_eq!(links.len(), 1); + assert_eq!(links[0].0, "a"); + assert_eq!(links[0].2, 11); + assert_eq!(links[0].3, 12); + } + + #[test] + fn extract_simple_table_links_ignores_wiki_link() { + // `[[wiki]]` is not a markdown link; should not be emitted here. + let cells = vec!["X".to_string(), "[[wiki]]".to_string()]; + let widths = vec![3, 4]; + let alignments = vec![Alignment::Left, Alignment::Left]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + assert!(links.is_empty()); + } + + #[test] + fn extract_simple_table_links_skips_link_with_empty_url() { + let cells = vec!["[label]()".to_string()]; + let widths = vec![5]; + let alignments = vec![Alignment::Left]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + assert!(links.is_empty()); + } +} diff --git a/src/ui/content.rs b/src/ui/content.rs index c6b42bf..73b0a91 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -440,11 +440,11 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { } } } - ContentItem::TableRow { cells, is_separator, is_header, column_widths } => { + ContentItem::TableRow { cells, is_separator, is_header, column_widths, alignments } => { let has_link = !is_separator && (is_cursor_line || is_hovered) && !app.item_links_at(item_idx).is_empty(); - render_table_row(f, &app.theme, &cells, is_separator, is_header, &column_widths, chunks[chunk_idx], is_cursor_line, has_link); + render_table_row(f, &app.theme, &cells, is_separator, is_header, &column_widths, &alignments, chunks[chunk_idx], is_cursor_line, has_link); } ContentItem::Details { summary, content_lines, id } => { let is_open = app.details_open_states.get(&id).copied().unwrap_or(false); @@ -561,11 +561,12 @@ fn calc_formatting_shrinkage(text: &str, up_to_pos: usize) -> usize { } if chars[pos] == '[' { if let Some((bracket_end, paren_end)) = find_markdown_link(&chars, pos) { - let url_len = paren_end - bracket_end - 2; + let url_len = paren_end - bracket_end - 2; if paren_end < up_to_pos { - shrinkage += 1 + url_len + 2; + // Full `[label](url)` seen before up_to_pos: strips `[` + `](` + url + `)` = 4 + url_len. + shrinkage += url_len + 4; } else if bracket_end < up_to_pos { - shrinkage += 1; + shrinkage += 1; } pos = paren_end + 1; continue; @@ -644,7 +645,8 @@ fn find_markdown_link(chars: &[char], start: usize) -> Option<(usize, usize)> { /// Calculate the adjusted column for a table cell /// Raw format: "| cell1 | cell2 |" /// Rendered: "▶ │ cell1 │ cell2 │" with cells padded to column widths -fn calc_table_adjusted_col(raw_col: usize, cells: &[String], column_widths: &[usize]) -> usize { +fn calc_table_adjusted_col(raw_col: usize, cells: &[String], column_widths: &[usize], alignments: &[crate::app::Alignment]) -> usize { + use crate::app::Alignment; let mut rendered_pos = 3; let mut raw_pos = 0; @@ -667,7 +669,13 @@ fn calc_table_adjusted_col(raw_col: usize, cells: &[String], column_widths: &[us .take(char_offset_in_raw_cell.min(cell_char_len)) .map(|c| c.width().unwrap_or(1)) .sum(); - let content_padding = (col_width.saturating_sub(cell_display_width)) / 2; + let pad = col_width.saturating_sub(cell_display_width); + let alignment = alignments.get(cell_idx).copied().unwrap_or(Alignment::Left); + let content_padding = match alignment { + Alignment::Left => 0, + Alignment::Right => pad, + Alignment::Center => pad / 2, + }; let rendered_content_start = rendered_pos + 1 + content_padding; // +1 for leading space return rendered_content_start + display_offset; @@ -715,11 +723,11 @@ fn apply_content_search_highlights( // Calculate the rendered column position based on content type // Use display width for CJK character support let adjusted_col = match &app.content_items.get(item_idx) { - Some(ContentItem::TableRow { cells, column_widths, is_separator, .. }) => { + Some(ContentItem::TableRow { cells, column_widths, alignments, is_separator, .. }) => { if *is_separator { continue; } - calc_table_adjusted_col(m.start_col, cells, column_widths) + calc_table_adjusted_col(m.start_col, cells, column_widths, alignments) } Some(ContentItem::TextLine(line)) => { let line = normalize_whitespace(line); @@ -1799,6 +1807,7 @@ fn render_table_row( is_separator: bool, is_header: bool, column_widths: &[usize], + alignments: &[crate::app::Alignment], area: Rect, is_cursor: bool, has_link: bool, @@ -1812,12 +1821,24 @@ fn render_table_row( ]; if is_separator { + use crate::app::Alignment; for (i, &width) in column_widths.iter().enumerate() { if i > 0 { spans.push(Span::styled("┼", Style::default().fg(border_color))); } - let dashes = "─".repeat(width + 2); + // Reflect alignment visually in the separator row (`:` markers where colons would + // appear in the source). Width accounts for two padding spaces either side. + let total = width + 2; + let alignment = alignments.get(i).copied().unwrap_or(Alignment::Left); + let (left_mark, right_mark) = match alignment { + Alignment::Left => (":", "─"), + Alignment::Right => ("─", ":"), + Alignment::Center => (":", ":"), + }; + let dashes = "─".repeat(total.saturating_sub(2)); + spans.push(Span::styled(left_mark.to_string(), Style::default().fg(border_color))); spans.push(Span::styled(dashes, Style::default().fg(border_color))); + spans.push(Span::styled(right_mark.to_string(), Style::default().fg(border_color))); } } else { let default_style = if is_header { @@ -1836,8 +1857,12 @@ fn render_table_row( let visible = cell_visible_width(&expanded_cell); let width = column_widths.get(i).copied().unwrap_or(visible); let pad = width.saturating_sub(visible); - let left_pad = pad / 2; - let right_pad = pad - left_pad; + let alignment = alignments.get(i).copied().unwrap_or(crate::app::Alignment::Left); + let (left_pad, right_pad) = match alignment { + crate::app::Alignment::Left => (0, pad), + crate::app::Alignment::Right => (pad, 0), + crate::app::Alignment::Center => (pad / 2, pad - pad / 2), + }; spans.push(Span::styled(format!(" {}", " ".repeat(left_pad)), default_style)); @@ -2230,3 +2255,41 @@ fn render_frontmatter_line( let paragraph = Paragraph::new(Line::from(spans)).style(style); f.render_widget(paragraph, area); } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn cell_visible_width_plain_text() { + assert_eq!(cell_visible_width("Plain URL"), 9); + } + + #[test] + fn cell_visible_width_strips_markdown_link() { + // `[label](url)` -> `label`. A prior off-by-one counted the closing `)` toward visible. + assert_eq!(cell_visible_width("[Top 5](https://x.test)"), 5); + } + + #[test] + fn cell_visible_width_strips_bold_italic_code() { + assert_eq!(cell_visible_width("**bold text**"), 9); + assert_eq!(cell_visible_width("*em*"), 2); + assert_eq!(cell_visible_width("`code`"), 4); + } + + #[test] + fn cell_visible_width_mixed_text_and_link() { + // "one [a](u) two" renders as "one a two" = 9 visible chars. + assert_eq!(cell_visible_width("one [a](https://u.test) two"), 9); + } + + #[test] + fn cell_visible_width_multiple_links_same_cell() { + // Pins the off-by-one fix: before the fix, each link inflated visible by 1, + // so a 2-link cell miscounted by 2 and tables with uneven link counts + // misaligned their borders. + // "[a](u1) [b](u2)" -> "a b" = 3 visible chars. + assert_eq!(cell_visible_width("[a](https://u1.test) [b](https://u2.test)"), 3); + } +} From 63dc410a2b4b7cf1c4b0aa0e698ddf561f9a26cc Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 02:05:20 -0700 Subject: [PATCH 3/8] feat: autolink bare http(s):// URLs Render bare URLs as styled links everywhere and emit them to item_links_at so Enter opens them. Strips trailing sentence punctuation per GFM; skips URLs that fall inside an existing [label](url) so they don't double-emit. Table cells fall back to a bare-URL scan when no bracket link is present. Unit tests for detect_bare_url_len (scheme, delimiters, punctuation, no-match) and for extract_simple_table_links bare-URL + precedence. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/state.rs | 125 ++++++++++++++++++++++++++++++++++++++-------- src/ui/content.rs | 114 ++++++++++++++++++++++++++++++++++++++++++ src/ui/mod.rs | 2 +- 3 files changed, 219 insertions(+), 22 deletions(-) diff --git a/src/app/state.rs b/src/app/state.rs index 65eecc7..a417d01 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -2674,9 +2674,40 @@ impl App { !self.item_all_links_at(self.content_cursor).is_empty() } - /// Extract `[text](url)` links from each table cell and map positions into the - /// row's rendered column space. Simple scenarios only: at most one markdown link - /// per cell, and the link's pre-prefix can contain inline formatting but no other links. + /// Parse the first `[label](url)` at any position in `s`, skipping wiki-link form `[[...]]`. + /// Returns `(display, url, raw_start_byte)` — display is the label, or the URL if the label is empty. + fn find_first_bracket_link(s: &str) -> Option<(String, String, usize)> { + let br_start = match s.find('[') { + Some(p) => p, + None => return None, + }; + if s[br_start..].starts_with("[[") { + return None; + } + let br_end_rel = match s[br_start + 1..].find(']') { + Some(p) => p, + None => return None, + }; + let br_end = br_start + 1 + br_end_rel; + if !s[br_end..].starts_with("](") { + return None; + } + let pr_end_rel = match s[br_end + 2..].find(')') { + Some(p) => p, + None => return None, + }; + let pr_end = br_end + 2 + pr_end_rel; + let label = &s[br_start + 1..br_end]; + let url = &s[br_end + 2..pr_end]; + if url.is_empty() { + return None; + } + let display = if label.is_empty() { url.to_string() } else { label.to_string() }; + Some((display, url.to_string(), br_start)) + } + + /// Extract `[text](url)` and bare URL links from each table cell and map positions into the + /// row's rendered column space. Simple scenarios only: one link per cell. fn extract_simple_table_links(cells: &[String], column_widths: &[usize], alignments: &[Alignment]) -> Vec<(String, String, usize, usize)> { let mut links = Vec::new(); let mut col_cursor = 0usize; // column within content area (after ` │` prefix) @@ -2692,27 +2723,26 @@ impl App { }; let cell_start = col_cursor + 1 /* leading space */ + left_pad; - if let Some(br_start) = cell.find('[') { - if !cell[br_start..].starts_with("[[") { - if let Some(br_end_rel) = cell[br_start + 1..].find(']') { - let br_end = br_start + 1 + br_end_rel; - if cell[br_end..].starts_with("](") { - if let Some(pr_end_rel) = cell[br_end + 2..].find(')') { - let pr_end = br_end + 2 + pr_end_rel; - let label = &cell[br_start + 1..br_end]; - let url = &cell[br_end + 2..pr_end]; - if !url.is_empty() { - let display = if label.is_empty() { url.to_string() } else { label.to_string() }; - let pre_visible = crate::ui::cell_visible_width(&cell[..br_start]); - let start = cell_start + pre_visible; - let end = start + display.chars().count(); - links.push((display, url.to_string(), start, end)); - } - } - } + // Find the first link in the cell — bracket form `[label](url)` takes priority, + // else fall back to a bare `http(s)://` scan. One link per cell (simple scope). + let mut found: Option<(String, String, usize)> = Self::find_first_bracket_link(cell); + if found.is_none() { + let mut scan = 0; + while scan < cell.len() { + if let Some(url_len) = crate::ui::detect_bare_url_len(cell, scan) { + let url = cell[scan..scan + url_len].to_string(); + found = Some((url.clone(), url, scan)); + break; } + scan += 1; } } + if let Some((display, url, raw_start)) = found { + let pre_visible = crate::ui::cell_visible_width(&cell[..raw_start]); + let start = cell_start + pre_visible; + let end = start + display.chars().count(); + links.push((display, url, start, end)); + } col_cursor += 1 + width + 1; // " " + width + " " if i + 1 < cells.len() { @@ -2739,6 +2769,9 @@ impl App { let mut links = Vec::new(); let mut search_start = 0; + // Raw byte ranges claimed by bracket-style links/images. Used to skip bare URLs + // that fall inside a `(url)` portion so we don't double-emit. + let mut claimed: Vec<(usize, usize)> = Vec::new(); while search_start < text.len() { let remaining = &text[search_start..]; @@ -2779,6 +2812,7 @@ impl App { } search_start = abs_img_pos + 2 + bracket_end + 2 + paren_end + 1; + claimed.push((abs_img_pos, search_start)); continue; } } @@ -2823,6 +2857,7 @@ impl App { } search_start = abs_img_pos + 1 + bracket_end + 2 + paren_end + 1; + claimed.push((abs_img_pos, search_start)); continue; } } @@ -2866,6 +2901,7 @@ impl App { } search_start = abs_bracket_pos + bracket_end + 2 + paren_end + 1; + claimed.push((abs_bracket_pos, search_start)); continue; } } @@ -2873,6 +2909,25 @@ impl App { break; } + // Bare URL autolink pass. Skips URLs that fall inside already-claimed bracket-link + // ranges so e.g. `[click](https://x)` doesn't double-emit the URL inside the parens. + let mut pos = 0; + while pos < text.len() { + if let Some(url_len) = crate::ui::detect_bare_url_len(text, pos) { + let end = pos + url_len; + let overlaps = claimed.iter().any(|(s, e)| pos < *e && end > *s); + if !overlaps { + let url = text[pos..end].to_string(); + let rendered_start = Self::calc_rendered_pos(text, pos); + let rendered_end = rendered_start + url.chars().count(); + links.push((url.clone(), url, rendered_start, rendered_end)); + } + pos = end; + } else { + pos += 1; + } + } + links } @@ -6026,4 +6081,32 @@ mod tests { let links = App::extract_simple_table_links(&cells, &widths, &alignments); assert!(links.is_empty()); } + + #[test] + fn extract_simple_table_links_bare_url_in_cell() { + // Cell 0 occupies " X " + "│" -> col_cursor=6. + // Cell 1 (Left): URL starts at 6 + 1 + 0 = 7, ends at 7 + 19 = 26. + let cells = vec!["X".to_string(), "https://example.com".to_string()]; + let widths = vec![3, 19]; + let alignments = vec![Alignment::Left, Alignment::Left]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + assert_eq!(links.len(), 1); + assert_eq!(links[0].0, "https://example.com"); + assert_eq!(links[0].1, "https://example.com"); + assert_eq!(links[0].2, 7); + assert_eq!(links[0].3, 26); + } + + #[test] + fn extract_simple_table_links_prefers_bracket_over_bare_in_same_cell() { + // One-link-per-cell scope: a bracket link in the cell claims it; the trailing + // bare URL is not emitted. + let cells = vec!["[label](https://a) https://b.test".to_string()]; + let widths = vec![33]; + let alignments = vec![Alignment::Left]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + assert_eq!(links.len(), 1); + assert_eq!(links[0].0, "label"); + assert_eq!(links[0].1, "https://a"); + } } diff --git a/src/ui/content.rs b/src/ui/content.rs index 73b0a91..504bdbf 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -474,6 +474,48 @@ pub(crate) fn cell_visible_width(cell: &str) -> usize { total.saturating_sub(calc_formatting_shrinkage(cell, total)) } +/// If `text[start..]` begins with a bare `http://` or `https://` URL, return the +/// byte length of the URL (trailing sentence punctuation stripped). Used for +/// GFM-style autolinking both in rendering and in the Enter-to-open path. +pub(crate) fn detect_bare_url_len(text: &str, start: usize) -> Option { + let rest = match text.get(start..) { + Some(s) => s, + None => return None, + }; + let scheme_len = if rest.starts_with("https://") { + 8 + } else if rest.starts_with("http://") { + 7 + } else { + return None; + }; + + // Walk from the scheme end until we hit a terminator or the string end. + let mut end = rest.len(); + for (idx, ch) in rest[scheme_len..].char_indices() { + if ch.is_whitespace() || matches!(ch, ')' | ']' | '>' | '<' | '"' | '\'' | '|') { + end = scheme_len + idx; + break; + } + } + + // Strip trailing sentence punctuation so `https://x.test.` -> `https://x.test`. + while end > scheme_len { + let last = rest[..end].chars().last().unwrap(); + if matches!(last, '.' | ',' | ';' | ':' | '!' | '?') { + end -= last.len_utf8(); + } else { + break; + } + } + + if end > scheme_len { + Some(end) + } else { + None + } +} + /// Calculate how many characters are removed by inline formatting before a given position /// This accounts for **bold**, *italic*, ~~strikethrough~~, `code`, [[wiki links]], and [markdown](links) fn calc_formatting_shrinkage(text: &str, up_to_pos: usize) -> usize { @@ -572,6 +614,16 @@ fn calc_formatting_shrinkage(text: &str, up_to_pos: usize) -> usize { continue; } } + // Bare URL: rendered 1:1 (no shrinkage), but skip so inner chars aren't reprocessed. + if chars[pos] == 'h' { + let byte_pos: usize = chars[..pos].iter().map(|c| c.len_utf8()).sum(); + if let Some(url_len) = detect_bare_url_len(text, byte_pos) { + // `pos` is a char index, `url_len` is bytes — convert by counting chars in the slice. + let url_char_count = text[byte_pos..byte_pos + url_len].chars().count(); + pos += url_char_count; + continue; + } + } pos += 1; } @@ -838,6 +890,37 @@ where let content_theme = &theme.content; while let Some((i, c)) = chars.next() { + // Bare URL autolink (http:// or https://). Must run before the char-dispatch branches + // so `h` starting a URL is recognised and consumed as a single link span. + if c == 'h' { + if let Some(url_len) = detect_bare_url_len(text, i) { + if i > current_start { + spans.push(Span::styled(&text[current_start..i], Style::default().fg(content_theme.text))); + } + let is_selected = selected_link == Some(link_index); + let style = if is_selected { + Style::default() + .fg(theme.background) + .bg(theme.warning) + .add_modifier(Modifier::BOLD) + } else { + Style::default() + .fg(content_theme.link) + .add_modifier(Modifier::UNDERLINED) + }; + spans.push(Span::styled(&text[i..i + url_len], style)); + link_index += 1; + // Advance the char iterator past the URL. Count chars (not bytes) in case + // the URL contains non-ASCII (e.g. IDN host). + let url_chars = text[i..i + url_len].chars().count(); + for _ in 1..url_chars { + chars.next(); + } + current_start = i + url_len; + continue; + } + } + // Check for **bold** or *italic* if c == '*' { if let Some(&(_, '*')) = chars.peek() { @@ -2292,4 +2375,35 @@ mod tests { // "[a](u1) [b](u2)" -> "a b" = 3 visible chars. assert_eq!(cell_visible_width("[a](https://u1.test) [b](https://u2.test)"), 3); } + + #[test] + fn detect_bare_url_basic() { + assert_eq!(detect_bare_url_len("see https://example.com now", 4), Some(19)); + assert_eq!(detect_bare_url_len("http://a.test", 0), Some(13)); + } + + #[test] + fn detect_bare_url_strips_trailing_punctuation() { + // GFM: the trailing `.` should not be part of the URL. + assert_eq!(detect_bare_url_len("visit https://example.com.", 6), Some(19)); + } + + #[test] + fn detect_bare_url_stops_at_delimiters() { + // "https://x.test" = 14 chars; the `)` / `>` terminator is not included. + assert_eq!(detect_bare_url_len("(https://x.test)", 1), Some(14)); + assert_eq!(detect_bare_url_len("", 1), Some(14)); + } + + #[test] + fn detect_bare_url_no_match_returns_none() { + assert_eq!(detect_bare_url_len("nothing here", 0), None); + assert_eq!(detect_bare_url_len("http:/broken", 0), None); // missing second slash + } + + #[test] + fn cell_visible_width_counts_bare_url_one_to_one() { + // Bare URL is not shrunk — visible width equals its character count. + assert_eq!(cell_visible_width("visit https://x.test"), 20); + } } diff --git a/src/ui/mod.rs b/src/ui/mod.rs index a6b17d4..7a37453 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -20,7 +20,7 @@ use ratatui::{ use crate::app::{App, ContextMenuState, DialogState, SearchPickerState, Mode, WikiAutocompleteState}; pub use content::render_content; -pub(crate) use content::cell_visible_width; +pub(crate) use content::{cell_visible_width, detect_bare_url_len}; pub use dialogs::{ render_create_folder_dialog, render_create_note_dialog, render_create_note_in_folder_dialog, render_create_wiki_note_dialog, render_delete_confirm_dialog, render_delete_folder_confirm_dialog, From 1ee200ee8d1de4badd231c3d1a86a3408e9c8000 Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 02:25:04 -0700 Subject: [PATCH 4/8] fix: plain dashes in rendered separator row Alignment is already visible in how data rows are padded, so showing `:---` / `---:` / `:---:` in the rendered separator duplicates source syntax that most markdown renderers strip. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ui/content.rs | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/src/ui/content.rs b/src/ui/content.rs index 504bdbf..de06500 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -1904,24 +1904,14 @@ fn render_table_row( ]; if is_separator { - use crate::app::Alignment; + // Plain horizontal rule — alignment is already visible in how data rows are padded, + // so we don't render colon markers from the source. for (i, &width) in column_widths.iter().enumerate() { if i > 0 { spans.push(Span::styled("┼", Style::default().fg(border_color))); } - // Reflect alignment visually in the separator row (`:` markers where colons would - // appear in the source). Width accounts for two padding spaces either side. - let total = width + 2; - let alignment = alignments.get(i).copied().unwrap_or(Alignment::Left); - let (left_mark, right_mark) = match alignment { - Alignment::Left => (":", "─"), - Alignment::Right => ("─", ":"), - Alignment::Center => (":", ":"), - }; - let dashes = "─".repeat(total.saturating_sub(2)); - spans.push(Span::styled(left_mark.to_string(), Style::default().fg(border_color))); + let dashes = "─".repeat(width + 2); spans.push(Span::styled(dashes, Style::default().fg(border_color))); - spans.push(Span::styled(right_mark.to_string(), Style::default().fg(border_color))); } } else { let default_style = if is_header { From 47e4084bed179692bf71970bfb5705d42373031a Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 06:16:08 -0700 Subject: [PATCH 5/8] feat: wide tables wrap cells, drop one-link-per-cell limit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wide tables no longer push later columns off-screen. Column widths cap against the terminal area (shrink-widest-first with a floor), and cells wrap to multiple visual lines at whitespace boundaries. Row height grows to match the tallest cell. Wrapping preserves inline markdown: parse_inline_formatting runs once per cell, then spans are distributed across visual lines — atomic spans (links, bold, italic, code, wiki) stay intact, plain-text spans break at whitespace. Avoids corrupting constructs that span wrap boundaries. Drop the one-link-per-cell limit in extract_simple_table_links so every [text](url) and bare URL in a row is navigable via arrow-key cycling. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/app/state.rs | 136 +++++++----- src/ui/content.rs | 513 +++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 543 insertions(+), 106 deletions(-) diff --git a/src/app/state.rs b/src/app/state.rs index a417d01..1ee3aa1 100644 --- a/src/app/state.rs +++ b/src/app/state.rs @@ -2674,40 +2674,14 @@ impl App { !self.item_all_links_at(self.content_cursor).is_empty() } - /// Parse the first `[label](url)` at any position in `s`, skipping wiki-link form `[[...]]`. - /// Returns `(display, url, raw_start_byte)` — display is the label, or the URL if the label is empty. - fn find_first_bracket_link(s: &str) -> Option<(String, String, usize)> { - let br_start = match s.find('[') { - Some(p) => p, - None => return None, - }; - if s[br_start..].starts_with("[[") { - return None; - } - let br_end_rel = match s[br_start + 1..].find(']') { - Some(p) => p, - None => return None, - }; - let br_end = br_start + 1 + br_end_rel; - if !s[br_end..].starts_with("](") { - return None; - } - let pr_end_rel = match s[br_end + 2..].find(')') { - Some(p) => p, - None => return None, - }; - let pr_end = br_end + 2 + pr_end_rel; - let label = &s[br_start + 1..br_end]; - let url = &s[br_end + 2..pr_end]; - if url.is_empty() { - return None; - } - let display = if label.is_empty() { url.to_string() } else { label.to_string() }; - Some((display, url.to_string(), br_start)) - } - - /// Extract `[text](url)` and bare URL links from each table cell and map positions into the - /// row's rendered column space. Simple scenarios only: one link per cell. + /// Extract all `[text](url)` and bare URL links from each table cell, mapping positions + /// into the row's rendered column space. Walks every cell end-to-end so multiple links + /// per cell are all navigable. + /// + /// Rendered positions assume natural column widths and a single-line row. When a table + /// wraps (capped widths, multi-line rows), keyboard Enter-to-open still works because it + /// only uses the URL; mouse click accuracy on wrapped lines is not guaranteed by this + /// method's output. fn extract_simple_table_links(cells: &[String], column_widths: &[usize], alignments: &[Alignment]) -> Vec<(String, String, usize, usize)> { let mut links = Vec::new(); let mut col_cursor = 0usize; // column within content area (after ` │` prefix) @@ -2723,25 +2697,28 @@ impl App { }; let cell_start = col_cursor + 1 /* leading space */ + left_pad; - // Find the first link in the cell — bracket form `[label](url)` takes priority, - // else fall back to a bare `http(s)://` scan. One link per cell (simple scope). - let mut found: Option<(String, String, usize)> = Self::find_first_bracket_link(cell); - if found.is_none() { - let mut scan = 0; - while scan < cell.len() { - if let Some(url_len) = crate::ui::detect_bare_url_len(cell, scan) { - let url = cell[scan..scan + url_len].to_string(); - found = Some((url.clone(), url, scan)); - break; - } - scan += 1; + // Walk the cell: at each position, try to recognise a bracket link first (so a + // bare URL inside its `(url)` portion is not double-emitted), then a bare URL. + let mut scan = 0; + while scan < cell.len() { + if let Some((display, url, raw_start, raw_end)) = Self::bracket_link_at(cell, scan) { + let pre_visible = crate::ui::cell_visible_width(&cell[..raw_start]); + let start = cell_start + pre_visible; + let end = start + display.chars().count(); + links.push((display, url, start, end)); + scan = raw_end; + continue; } - } - if let Some((display, url, raw_start)) = found { - let pre_visible = crate::ui::cell_visible_width(&cell[..raw_start]); - let start = cell_start + pre_visible; - let end = start + display.chars().count(); - links.push((display, url, start, end)); + if let Some(url_len) = crate::ui::detect_bare_url_len(cell, scan) { + let url = cell[scan..scan + url_len].to_string(); + let pre_visible = crate::ui::cell_visible_width(&cell[..scan]); + let start = cell_start + pre_visible; + let end = start + url.chars().count(); + links.push((url.clone(), url, start, end)); + scan += url_len; + continue; + } + scan += 1; } col_cursor += 1 + width + 1; // " " + width + " " @@ -2752,6 +2729,39 @@ impl App { links } + /// Parse `[label](url)` anchored at byte offset `at` in `s`, skipping wiki-link form `[[...]]`. + /// Returns `(display, url, raw_start, raw_end_exclusive)` where display is the label (or url + /// if label is empty). Returns None if no bracket link starts exactly at `at`. + fn bracket_link_at(s: &str, at: usize) -> Option<(String, String, usize, usize)> { + let rest = match s.get(at..) { + Some(r) => r, + None => return None, + }; + if !rest.starts_with('[') || rest.starts_with("[[") { + return None; + } + let br_end_rel = match rest[1..].find(']') { + Some(p) => p, + None => return None, + }; + let br_end = 1 + br_end_rel; + if !rest[br_end..].starts_with("](") { + return None; + } + let pr_end_rel = match rest[br_end + 2..].find(')') { + Some(p) => p, + None => return None, + }; + let pr_end = br_end + 2 + pr_end_rel; + let label = &rest[1..br_end]; + let url = &rest[br_end + 2..pr_end]; + if url.is_empty() { + return None; + } + let display = if label.is_empty() { url.to_string() } else { label.to_string() }; + Some((display, url.to_string(), at, at + pr_end + 1)) + } + /// Extract all links and images from a specific content item as (text, url, start_col, end_col) tuples /// The columns are character positions in the rendered line (after prefix like "▶ " or "• ") pub fn item_links_at(&self, index: usize) -> Vec<(String, String, usize, usize)> { @@ -6098,15 +6108,29 @@ mod tests { } #[test] - fn extract_simple_table_links_prefers_bracket_over_bare_in_same_cell() { - // One-link-per-cell scope: a bracket link in the cell claims it; the trailing - // bare URL is not emitted. + fn extract_simple_table_links_emits_both_bracket_and_bare_in_same_cell() { + // A cell with both a bracket link and a trailing bare URL emits both, + // in source order. Bracket link's URL is not re-emitted as a bare URL. let cells = vec!["[label](https://a) https://b.test".to_string()]; let widths = vec![33]; let alignments = vec![Alignment::Left]; let links = App::extract_simple_table_links(&cells, &widths, &alignments); - assert_eq!(links.len(), 1); + assert_eq!(links.len(), 2); assert_eq!(links[0].0, "label"); assert_eq!(links[0].1, "https://a"); + assert_eq!(links[1].0, "https://b.test"); + assert_eq!(links[1].1, "https://b.test"); + } + + #[test] + fn extract_simple_table_links_multiple_bracket_links_in_same_cell() { + // Multiple `[text](url)` in one cell should all be emitted. + let cells = vec!["[alpha](u1) and [beta](u2)".to_string()]; + let widths = vec![16]; + let alignments = vec![Alignment::Left]; + let links = App::extract_simple_table_links(&cells, &widths, &alignments); + assert_eq!(links.len(), 2); + assert_eq!(links[0].0, "alpha"); + assert_eq!(links[1].0, "beta"); } } diff --git a/src/ui/content.rs b/src/ui/content.rs index de06500..bd3e6d6 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -186,7 +186,27 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { base_height + (inline_images.len() as u16 * INLINE_THUMBNAIL_HEIGHT) } } - ContentItem::TableRow { .. } => 1u16, + ContentItem::TableRow { cells, is_separator, column_widths, .. } => { + if *is_separator { + 1u16 + } else { + // Budget must match render_table_row exactly. render uses area.width + // (= inner_area.width after chunk split), not `available_width`, which + // carries a 4-char list-prefix margin that tables don't need. + let n = column_widths.len(); + let overhead = 3 + 3 * n; + let budget = (inner_area.width as usize).saturating_sub(overhead); + let capped = cap_column_widths(column_widths, budget); + let text_color = theme.content.text; + let row_lines = cells.iter().enumerate().map(|(i, cell)| { + let w = capped.get(i).copied().unwrap_or(0); + let expanded = expand_tabs(cell); + let spans = parse_inline_formatting:: bool>(&expanded, theme, None, None); + distribute_spans_across_lines(spans, w, text_color).len() + }).max().unwrap_or(1).max(1); + (row_lines as u16).min(max_item_height) + } + } ContentItem::Details { content_lines, id, .. } => { let is_open = details_states.get(id).copied().unwrap_or(false); if is_open { @@ -474,6 +494,207 @@ pub(crate) fn cell_visible_width(cell: &str) -> usize { total.saturating_sub(calc_formatting_shrinkage(cell, total)) } +/// Per-column minimum width when shrinking a wide table to fit the terminal. +const TABLE_COLUMN_MIN_WIDTH: usize = 8; + +/// Given the "natural" width of each column (max content width) and the available +/// budget for content (= terminal area minus borders/padding), return capped widths +/// that sum to at most `available`. Shrinks the widest column(s) first so narrow +/// columns keep their full width whenever possible. Each column stays at or above +/// `TABLE_COLUMN_MIN_WIDTH` unless its natural width is already below that. +pub(crate) fn cap_column_widths(natural: &[usize], available: usize) -> Vec { + let mut widths: Vec = natural.to_vec(); + if widths.is_empty() { + return widths; + } + loop { + let total: usize = widths.iter().sum(); + if total <= available { + return widths; + } + // Pick the widest column that can still shrink. + let mut target: Option = None; + let mut max_w: usize = 0; + for (i, &w) in widths.iter().enumerate() { + let floor = TABLE_COLUMN_MIN_WIDTH.min(natural[i]); + if w > floor && w > max_w { + max_w = w; + target = Some(i); + } + } + match target { + Some(i) => widths[i] -= 1, + None => return widths, // every column already at its floor; can't shrink further + } + } +} + +/// Distribute a pre-parsed list of inline spans across visual lines of at most +/// `width` display columns each. +/// +/// Original span structure is preserved — each span carries its own whitespace +/// (a plain-text span that reads `" then "` keeps its leading and trailing +/// space, so adjacent styled spans sit against punctuation without any injected +/// space). Plain-text spans can be broken at internal whitespace if needed; +/// styled spans (links, bold, italic, code, wiki) are atomic — they fit on one +/// line or start a new line, overflowing as a single span if wider than `width`. +/// +/// Use this downstream of `parse_inline_formatting` so the parser stays the +/// single source of truth for what counts as a markdown construct: +/// ```ignore +/// let spans = parse_inline_formatting(cell, theme, None, None:: bool>); +/// let lines = distribute_spans_across_lines(spans, width, theme.content.text); +/// ``` +/// +/// The returned lines own their content (`Span<'static>`). +pub(crate) fn distribute_spans_across_lines( + spans: Vec>, + width: usize, + plain_text_color: ratatui::style::Color, +) -> Vec>> { + if width == 0 { + let owned: Vec> = spans + .into_iter() + .map(|s| Span::styled(s.content.into_owned(), s.style)) + .collect(); + return vec![owned]; + } + + let mut lines: Vec>> = Vec::new(); + let mut current: Vec> = Vec::new(); + let mut current_visible: usize = 0; + + for span in spans { + let style = span.style; + let span_visible = UnicodeWidthStr::width(span.content.as_ref()); + let is_plain = is_plain_text_span(&style, plain_text_color); + + if !is_plain { + // Atomic span: must stay together. + if current_visible > 0 && current_visible + span_visible > width { + lines.push(std::mem::take(&mut current)); + current_visible = 0; + } + current.push(Span::styled(span.content.into_owned(), style)); + current_visible += span_visible; + continue; + } + + // Plain-text span: may need breaking at internal whitespace. + let mut rest: &str = span.content.as_ref(); + while !rest.is_empty() { + // If we're at the start of a fresh line, discard leading whitespace + // (lines shouldn't start with a space, unless the content IS just spaces). + if current_visible == 0 { + let trimmed = rest.trim_start(); + if trimmed.is_empty() { + break; + } + rest = trimmed; + } + + let rest_visible = UnicodeWidthStr::width(rest); + if current_visible + rest_visible <= width { + // Whole remainder fits on current line. + current.push(Span::styled(rest.to_string(), style)); + current_visible += rest_visible; + break; + } + + // Need to break within `rest`. Find the longest prefix that fits AND ends at + // a whitespace boundary. + let remaining_budget = width.saturating_sub(current_visible); + let (head, tail) = split_plain_at_whitespace(rest, remaining_budget); + + if !head.is_empty() { + current.push(Span::styled(head.to_string(), style)); + lines.push(std::mem::take(&mut current)); + current_visible = 0; + rest = tail; + continue; + } + + // No whitespace break fits in the budget. If there's content on the current + // line, flush it so the next iteration tries with a fresh full-width line. + if !current.is_empty() { + lines.push(std::mem::take(&mut current)); + current_visible = 0; + continue; + } + + // Empty line and no whitespace break — hard-break the first word at + // display-width boundaries. + let (forced_head, forced_tail) = take_width(rest, width); + if forced_head.is_empty() { + // Degenerate: push the first char and move on. + let first_char = rest.chars().next().unwrap(); + let first_len = first_char.len_utf8(); + current.push(Span::styled(rest[..first_len].to_string(), style)); + current_visible += UnicodeWidthChar::width(first_char).unwrap_or(1); + rest = &rest[first_len..]; + } else { + lines.push(vec![Span::styled(forced_head.to_string(), style)]); + rest = forced_tail; + } + } + } + + if !current.is_empty() || lines.is_empty() { + lines.push(current); + } + lines +} + +/// Return `(head, tail)` where `head` is the longest prefix of `s` whose display +/// width does not exceed `max_width` AND which ends at a whitespace boundary. +/// `tail` has leading whitespace stripped. Returns `("", s)` if no such prefix +/// exists. +fn split_plain_at_whitespace(s: &str, max_width: usize) -> (&str, &str) { + let mut best_end: Option = None; + let mut width_before_pos: usize = 0; + + for (pos, ch) in s.char_indices() { + if ch.is_whitespace() { + if width_before_pos <= max_width { + best_end = Some(pos); + } else { + break; + } + } + width_before_pos += UnicodeWidthChar::width(ch).unwrap_or(0); + } + + match best_end { + Some(end) => (&s[..end], s[end..].trim_start()), + None => ("", s), + } +} + +/// Spans emitted by `parse_inline_formatting` for ordinary text carry only the +/// default content colour (no modifiers, no background). Use that as the "is +/// this plain text?" fingerprint so we know which spans can be broken at +/// whitespace during wrapping. +fn is_plain_text_span(style: &Style, plain_color: ratatui::style::Color) -> bool { + style.bg.is_none() + && style.add_modifier.is_empty() + && style.sub_modifier.is_empty() + && (style.fg.is_none() || style.fg == Some(plain_color.into())) +} + +/// Split a string into a `(head, tail)` pair where `head` has display width `<= width`. +/// Used by `wrap_cell` for hard-breaking over-width words. +fn take_width(s: &str, width: usize) -> (&str, &str) { + let mut w = 0usize; + for (i, ch) in s.char_indices() { + let cw = UnicodeWidthChar::width(ch).unwrap_or(1); + if w + cw > width { + return (&s[..i], &s[i..]); + } + w += cw; + } + (s, "") +} + /// If `text[start..]` begins with a bare `http://` or `https://` URL, return the /// byte length of the URL (trailing sentence punctuation stripped). Used for /// GFM-style autolinking both in rendering and in the Enter-to-open path. @@ -1889,46 +2110,97 @@ fn render_table_row( cells: &[String], is_separator: bool, is_header: bool, - column_widths: &[usize], + natural_widths: &[usize], alignments: &[crate::app::Alignment], area: Rect, is_cursor: bool, has_link: bool, ) { - let cursor_indicator = if is_cursor { "▶ " } else { " " }; let border_color = theme.border; + let row_bg = if is_cursor { + Style::default().bg(theme.selection) + } else { + Style::default() + }; - let mut spans = vec![ - Span::styled(cursor_indicator, Style::default().fg(theme.warning)), - Span::styled("│", Style::default().fg(border_color)), - ]; + // Cap widths against the row's available render width. + // Row overhead: " " (2) + leading │ (1) + per cell " content " (+2) + per-cell │ (N-1 between + 1 trailing) = 3 + 3N. + let n = natural_widths.len(); + let overhead = 3 + 3 * n; + let budget = (area.width as usize).saturating_sub(overhead); + let widths = cap_column_widths(natural_widths, budget); if is_separator { - // Plain horizontal rule — alignment is already visible in how data rows are padded, - // so we don't render colon markers from the source. - for (i, &width) in column_widths.iter().enumerate() { + // Separator is always a single line. + let mut spans = vec![ + Span::styled(if is_cursor { "▶ " } else { " " }, Style::default().fg(theme.warning)), + Span::styled("│", Style::default().fg(border_color)), + ]; + for (i, &width) in widths.iter().enumerate() { if i > 0 { spans.push(Span::styled("┼", Style::default().fg(border_color))); } let dashes = "─".repeat(width + 2); spans.push(Span::styled(dashes, Style::default().fg(border_color))); } + spans.push(Span::styled("│", Style::default().fg(border_color))); + let line_area = Rect { x: area.x, y: area.y, width: area.width, height: 1 }; + let paragraph = Paragraph::new(Line::from(spans)).style(row_bg); + f.render_widget(paragraph, line_area); + return; + } + + let text_color = theme.content.text; + + // Parse each cell as inline markdown ONCE, then distribute the resulting spans + // across visual lines. Parsing the whole cell keeps `parse_inline_formatting` as + // the single source of truth for what counts as a construct — multi-word atoms + // like `**warp decode**` or `[Top 5 Things](url)` are recognised correctly, no + // matter how the wrap boundary falls. + let per_cell_lines: Vec>>> = cells + .iter() + .enumerate() + .map(|(i, c)| { + let w = widths.get(i).copied().unwrap_or(0); + let expanded = expand_tabs(c); + let spans = parse_inline_formatting:: bool>(&expanded, theme, None, None); + distribute_spans_across_lines(spans, w, text_color) + }) + .collect(); + let row_height = per_cell_lines + .iter() + .map(|lines| lines.len()) + .max() + .unwrap_or(1) + .max(1); + + let default_style = if is_header { + Style::default().fg(theme.info).add_modifier(Modifier::BOLD) } else { - let default_style = if is_header { - Style::default().fg(theme.info).add_modifier(Modifier::BOLD) - } else { - Style::default().fg(theme.foreground) - }; - let text_color = theme.content.text; + Style::default().fg(theme.foreground) + }; + + for line_idx in 0..row_height { + // Cursor indicator shows only on the first visual line of the row. + let cursor_indicator = if is_cursor && line_idx == 0 { "▶ " } else { " " }; + let mut spans: Vec> = vec![ + Span::styled(cursor_indicator, Style::default().fg(theme.warning)), + Span::styled("│", Style::default().fg(border_color)), + ]; - for (i, cell) in cells.iter().enumerate() { + for (i, cell_lines) in per_cell_lines.iter().enumerate() { if i > 0 { spans.push(Span::styled("│", Style::default().fg(border_color))); } - - let expanded_cell = expand_tabs(cell); - let visible = cell_visible_width(&expanded_cell); - let width = column_widths.get(i).copied().unwrap_or(visible); + let line_spans_slice: &[Span<'static>] = cell_lines + .get(line_idx) + .map(|v| v.as_slice()) + .unwrap_or(&[]); + let width = widths.get(i).copied().unwrap_or(0); + let visible: usize = line_spans_slice + .iter() + .map(|s| UnicodeWidthStr::width(s.content.as_ref())) + .sum(); let pad = width.saturating_sub(visible); let alignment = alignments.get(i).copied().unwrap_or(crate::app::Alignment::Left); let (left_pad, right_pad) = match alignment { @@ -1939,42 +2211,36 @@ fn render_table_row( spans.push(Span::styled(format!(" {}", " ".repeat(left_pad)), default_style)); - // `parse_inline_formatting` is generic over the validator's closure type. - // We don't need wiki-link validation in tables — a fn-pointer type satisfies `F: Fn(&str) -> bool`. - let inline = parse_inline_formatting:: bool>(&expanded_cell, theme, None, None); - for sp in inline { - // Restyle plain-text spans (those the inline parser emits with the default body color) - // so headers get bold+info. Links/code/wiki keep their own styles. - let is_plain = sp.style.bg.is_none() - && sp.style.add_modifier.is_empty() - && sp.style.sub_modifier.is_empty() - && (sp.style.fg.is_none() || sp.style.fg == Some(text_color.into())); - let style = if is_plain { default_style } else { sp.style }; - spans.push(Span::styled(sp.content.into_owned(), style)); + for sp in line_spans_slice.iter().cloned() { + let style = if is_plain_text_span(&sp.style, text_color) { + default_style + } else { + sp.style + }; + spans.push(Span::styled(sp.content, style)); } spans.push(Span::styled(format!("{} ", " ".repeat(right_pad)), default_style)); } - } - spans.push(Span::styled("│", Style::default().fg(border_color))); + spans.push(Span::styled("│", Style::default().fg(border_color))); + // "Open ↗" hint only on the first line, same as the cursor indicator. + if has_link && line_idx == 0 { + spans.push(Span::styled(" Open ↗", Style::default().fg(theme.content.link))); + } - if has_link { - spans.push(Span::styled(" Open ↗", Style::default().fg(theme.content.link))); + if (area.y + line_idx as u16) >= area.y + area.height { + break; + } + let line_area = Rect { + x: area.x, + y: area.y + line_idx as u16, + width: area.width, + height: 1, + }; + let paragraph = Paragraph::new(Line::from(spans)).style(row_bg); + f.render_widget(paragraph, line_area); } - - let styled_line = Line::from(spans); - - let style = if is_cursor { - Style::default().bg(theme.selection) - } else { - Style::default() - }; - - let paragraph = Paragraph::new(styled_line) - .style(style) - .wrap(Wrap { trim: false }); - f.render_widget(paragraph, area); } fn render_inline_image_with_cursor(f: &mut Frame, app: &mut App, path: &str, area: Rect, is_cursor: bool, is_hovered: bool) { @@ -2396,4 +2662,151 @@ mod tests { // Bare URL is not shrunk — visible width equals its character count. assert_eq!(cell_visible_width("visit https://x.test"), 20); } + + #[test] + fn cap_column_widths_leaves_narrow_columns_alone() { + // Natural sum = 7 + 12 + 500 = 519; budget = 107 (like a ~120-col terminal). + // Expect narrow columns untouched, Description shrunk to fill what's left. + let natural = vec![7, 12, 500]; + let capped = cap_column_widths(&natural, 107); + assert_eq!(capped[0], 7); + assert_eq!(capped[1], 12); + assert_eq!(capped[0] + capped[1] + capped[2], 107); + } + + #[test] + fn cap_column_widths_no_shrink_when_it_fits() { + let natural = vec![4, 6, 10]; + assert_eq!(cap_column_widths(&natural, 50), vec![4, 6, 10]); + } + + #[test] + fn cap_column_widths_respects_min_floor() { + // If available is absurdly small, columns bottom out at TABLE_COLUMN_MIN_WIDTH + // (unless their natural width is already below that — those stay at natural). + let natural = vec![3, 50, 50]; // 3 is below the floor; leave it alone + let capped = cap_column_widths(&natural, 5); + assert_eq!(capped[0], 3); + assert_eq!(capped[1], TABLE_COLUMN_MIN_WIDTH); + assert_eq!(capped[2], TABLE_COLUMN_MIN_WIDTH); + } + + // --- distribute_spans_across_lines --- + // Tests use Style::default() for "plain" spans and a non-default modifier + // (BOLD) as a proxy for any styled span (links/bold/code/etc.), matching + // how `parse_inline_formatting` emits them. + + fn plain_color() -> ratatui::style::Color { + ratatui::style::Color::Reset + } + + fn plain(content: &'static str) -> Span<'static> { + Span::styled(content.to_string(), Style::default()) + } + + fn atomic(content: &'static str) -> Span<'static> { + // Any non-default style qualifies the span as "atomic" to our logic. + Span::styled(content.to_string(), Style::default().add_modifier(Modifier::BOLD)) + } + + fn line_text(line: &[Span<'static>]) -> String { + line.iter().map(|s| s.content.as_ref()).collect() + } + + #[test] + fn distribute_plain_text_wraps_at_word_boundary() { + // "alpha beta gamma delta" at width 10: "alpha beta" = 10 fits, "gamma delta" = 11 + // does not, so "gamma" and "delta" each get their own line. + let lines = distribute_spans_across_lines( + vec![plain("alpha beta gamma delta")], + 10, + plain_color(), + ); + let texts: Vec = lines.iter().map(|l| line_text(l)).collect(); + assert_eq!( + texts, + vec!["alpha beta".to_string(), "gamma".to_string(), "delta".to_string()] + ); + } + + #[test] + fn distribute_hard_breaks_over_wide_plain_word() { + let lines = distribute_spans_across_lines( + vec![plain("supercalifragilisticexpialidocious")], + 10, + plain_color(), + ); + assert!(lines.len() >= 4); + for l in &lines { + let width: usize = l.iter().map(|s| UnicodeWidthStr::width(s.content.as_ref())).sum(); + assert!(width <= 10, "line {:?} exceeds width", line_text(l)); + } + } + + #[test] + fn distribute_keeps_atomic_span_on_one_line_even_when_wider_than_column() { + // A styled span wider than the column is accepted as overflow — splitting its + // content would corrupt the rendered markdown construct. + let lines = distribute_spans_across_lines(vec![atomic("VeryLongStyledContent")], 10, plain_color()); + assert_eq!(lines.len(), 1); + assert_eq!(line_text(&lines[0]), "VeryLongStyledContent"); + } + + #[test] + fn distribute_packs_plain_then_atomic_on_same_line_when_it_fits() { + // "see" (plain, 3) + "blog" (atomic, 4) -> "see blog" on one line, width 20. + let lines = distribute_spans_across_lines( + vec![plain("see "), atomic("blog")], + 20, + plain_color(), + ); + assert_eq!(lines.len(), 1); + assert_eq!(line_text(&lines[0]), "see blog"); + } + + #[test] + fn distribute_breaks_to_new_line_when_atomic_would_overflow() { + // "a short prefix " (plain, 15 incl. trailing space) + "XXXXXXX" atomic (7): + // 15+7=22 > 18 budget, so atomic starts on a new line. The plain span's + // trailing space is preserved on line 1 (invisible when rendered). + let lines = distribute_spans_across_lines( + vec![plain("a short prefix "), atomic("XXXXXXX")], + 18, + plain_color(), + ); + assert_eq!(lines.len(), 2); + assert_eq!(line_text(&lines[0]).trim_end(), "a short prefix"); + assert_eq!(line_text(&lines[1]), "XXXXXXX"); + } + + #[test] + fn distribute_empty_input_returns_one_empty_line() { + let lines = distribute_spans_across_lines(Vec::new(), 10, plain_color()); + assert_eq!(lines.len(), 1); + assert!(lines[0].is_empty()); + } + + #[test] + fn distribute_width_zero_returns_one_line_owned() { + // When width is 0 we don't wrap — caller decides how to handle. + let lines = distribute_spans_across_lines(vec![plain("hello world")], 0, plain_color()); + assert_eq!(lines.len(), 1); + } + + #[test] + fn distribute_does_not_inject_space_between_atomic_and_adjacent_punctuation() { + // Reproduces the `two kernels: \`gate+up\`, then \`down\`` case. Previously + // the flatten-to-words step lost the fact that "," had no leading space, + // and we injected one, bumping the visible width and forcing an extra wrap. + // With span-preserving distribution, no space is injected. + let spans = vec![ + plain("two kernels: "), + atomic("gate+up"), + plain(", then "), + atomic("down"), + ]; + let lines = distribute_spans_across_lines(spans, 31, plain_color()); + assert_eq!(lines.len(), 1, "got {:?}", lines.iter().map(|l| line_text(l)).collect::>()); + assert_eq!(line_text(&lines[0]), "two kernels: gate+up, then down"); + } } From 95ddd55b8535c61167a37bdbc12941897800829d Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 18:04:39 -0700 Subject: [PATCH 6/8] fix: count wide chars (emoji, CJK) in table column-width sizing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit cell_visible_width measured by chars().count(), so a cell like "🟡In-Progress" was sized at 12 cols when it actually renders in 13. Column widths under-reserved, causing emoji/CJK cells to overflow and wrap unnecessarily even when the terminal had room. Switch to UnicodeWidthStr::width and subtract the markdown-marker char count (markers are all ASCII = 1 col each). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ui/content.rs | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-) diff --git a/src/ui/content.rs b/src/ui/content.rs index bd3e6d6..3914f51 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -488,10 +488,16 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { } /// Visible width of a table cell after inline markdown shrinks -/// (e.g. `[label](url)` -> `label`). Used both for column layout and render padding. +/// (e.g. `[label](url)` -> `label`). Measured in *display columns*, so wide +/// characters (CJK, emoji) contribute their full terminal width — not just 1 +/// char each. Markdown markers stripped by `calc_formatting_shrinkage` are all +/// ASCII (1 col each), so subtracting their char-count from the display width +/// gives the visible-content's display width. pub(crate) fn cell_visible_width(cell: &str) -> usize { - let total = cell.chars().count(); - total.saturating_sub(calc_formatting_shrinkage(cell, total)) + let display_width = UnicodeWidthStr::width(cell); + let total_chars = cell.chars().count(); + let marker_chars = calc_formatting_shrinkage(cell, total_chars); + display_width.saturating_sub(marker_chars) } /// Per-column minimum width when shrinking a wide table to fit the terminal. @@ -2663,6 +2669,18 @@ mod tests { assert_eq!(cell_visible_width("visit https://x.test"), 20); } + #[test] + fn cell_visible_width_counts_emoji_as_two_columns() { + // 🟡 is one char but displays as 2 columns in a terminal. The char-count + // version under-counted: 1 (emoji) + 11 ("In-Progress") = 12, so column + // widths were reserved at 12 cols while the cell actually renders in 13. + // That off-by-one forced an unnecessary wrap. + assert_eq!(cell_visible_width("🟡In-Progress"), 13); + assert_eq!(cell_visible_width("🟡"), 2); + // ASCII control: still matches char count. + assert_eq!(cell_visible_width("In-Progress"), 11); + } + #[test] fn cap_column_widths_leaves_narrow_columns_alone() { // Natural sum = 7 + 12 + 500 = 519; budget = 107 (like a ~120-col terminal). From dff736bf5363615b48a07c12d4afc0543a0b04f6 Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Fri, 24 Apr 2026 19:53:31 -0700 Subject: [PATCH 7/8] feat: render
as line breaks inside table cells Table cells can now contain GFM-style
,
,
(case-insensitive) to split content across multiple visual lines. Each logical line parses and wraps independently; they stack vertically within the cell, and row height grows to fit. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ui/content.rs | 117 ++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 108 insertions(+), 9 deletions(-) diff --git a/src/ui/content.rs b/src/ui/content.rs index 3914f51..22056e9 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -201,8 +201,14 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { let row_lines = cells.iter().enumerate().map(|(i, cell)| { let w = capped.get(i).copied().unwrap_or(0); let expanded = expand_tabs(cell); - let spans = parse_inline_formatting:: bool>(&expanded, theme, None, None); - distribute_spans_across_lines(spans, w, text_color).len() + // `
` inside a cell opens a new logical line; each logical line + // wraps independently and stacks vertically within the cell. + let mut total: usize = 0; + for logical in split_cell_by_br(&expanded) { + let spans = parse_inline_formatting:: bool>(logical, theme, None, None); + total += distribute_spans_across_lines(spans, w, text_color).len(); + } + total.max(1) }).max().unwrap_or(1).max(1); (row_lines as u16).min(max_item_height) } @@ -701,6 +707,63 @@ fn take_width(s: &str, width: usize) -> (&str, &str) { (s, "") } +/// Split a table cell on GFM-style line-break tags (`
`, `
`, `
`, +/// case-insensitive). Returns one slice per logical line — at least one slice, +/// even for an empty cell. +/// +/// Tag recognition is deliberately narrow: only the three common forms with +/// optional single-space and trailing slash. Anything else (attributes, unusual +/// whitespace, non-ASCII case folding) is passed through as literal text. +pub(crate) fn split_cell_by_br(cell: &str) -> Vec<&str> { + let mut parts: Vec<&str> = Vec::new(); + let bytes = cell.as_bytes(); + let mut start = 0; + let mut i = 0; + while i < cell.len() { + if bytes[i] == b'<' { + if let Some(end) = try_match_br(bytes, i) { + parts.push(&cell[start..i]); + start = end; + i = end; + continue; + } + } + i += 1; + } + parts.push(&cell[start..]); + parts +} + +/// Try to match a `
` / `
` / `
` tag starting at byte offset `at`. +/// Returns the byte offset just past the closing `>` if matched, else `None`. +fn try_match_br(bytes: &[u8], at: usize) -> Option { + let b = bytes; + if b.get(at) != Some(&b'<') { + return None; + } + if !matches!(b.get(at + 1), Some(b'b' | b'B')) { + return None; + } + if !matches!(b.get(at + 2), Some(b'r' | b'R')) { + return None; + } + let mut i = at + 3; + // Optional single space ("
" form). + if b.get(i) == Some(&b' ') { + i += 1; + } + // Optional self-closing slash. + if b.get(i) == Some(&b'/') { + i += 1; + } + // Must end in `>`. + if b.get(i) == Some(&b'>') { + Some(i + 1) + } else { + None + } +} + /// If `text[start..]` begins with a bare `http://` or `https://` URL, return the /// byte length of the URL (trailing sentence punctuation stripped). Used for /// GFM-style autolinking both in rendering and in the Enter-to-open path. @@ -2158,19 +2221,28 @@ fn render_table_row( let text_color = theme.content.text; - // Parse each cell as inline markdown ONCE, then distribute the resulting spans - // across visual lines. Parsing the whole cell keeps `parse_inline_formatting` as - // the single source of truth for what counts as a construct — multi-word atoms - // like `**warp decode**` or `[Top 5 Things](url)` are recognised correctly, no - // matter how the wrap boundary falls. + // Parse each cell as inline markdown ONCE per logical line, then distribute the + // resulting spans across visual lines. `
` tags open a new logical line — + // each one wraps independently; their visual lines stack within the cell. + // Parsing the whole logical line keeps `parse_inline_formatting` as the single + // source of truth for what counts as a construct (multi-word atoms like + // `**warp decode**` or `[Top 5 Things](url)` are recognised regardless of + // where wrap boundaries fall). let per_cell_lines: Vec>>> = cells .iter() .enumerate() .map(|(i, c)| { let w = widths.get(i).copied().unwrap_or(0); let expanded = expand_tabs(c); - let spans = parse_inline_formatting:: bool>(&expanded, theme, None, None); - distribute_spans_across_lines(spans, w, text_color) + let mut all_visual_lines: Vec>> = Vec::new(); + for logical in split_cell_by_br(&expanded) { + let spans = parse_inline_formatting:: bool>(logical, theme, None, None); + all_visual_lines.extend(distribute_spans_across_lines(spans, w, text_color)); + } + if all_visual_lines.is_empty() { + all_visual_lines.push(Vec::new()); + } + all_visual_lines }) .collect(); let row_height = per_cell_lines @@ -2811,6 +2883,33 @@ mod tests { assert_eq!(lines.len(), 1); } + #[test] + fn split_cell_by_br_basic_variants() { + assert_eq!(split_cell_by_br("no break"), vec!["no break"]); + assert_eq!(split_cell_by_br("a
b"), vec!["a", "b"]); + assert_eq!(split_cell_by_br("a
b"), vec!["a", "b"]); + assert_eq!(split_cell_by_br("a
b"), vec!["a", "b"]); + } + + #[test] + fn split_cell_by_br_case_insensitive() { + assert_eq!(split_cell_by_br("A
B"), vec!["A", "B"]); + assert_eq!(split_cell_by_br("A
B"), vec!["A", "B"]); + } + + #[test] + fn split_cell_by_br_multiple_and_empty_segments() { + assert_eq!(split_cell_by_br("
head
mid
"), vec!["", "head", "mid", ""]); + } + + #[test] + fn split_cell_by_br_malformed_tag_passes_through() { + // No closing `>` — treat literally. + assert_eq!(split_cell_by_br("a
b"), vec!["ab"]); + } + #[test] fn distribute_does_not_inject_space_between_atomic_and_adjacent_punctuation() { // Reproduces the `two kernels: \`gate+up\`, then \`down\`` case. Previously From dfe51f89b050aca0e78c42a8bddefc8ca2aa0caf Mon Sep 17 00:00:00 2001 From: Jinze Xue Date: Sat, 25 Apr 2026 14:04:26 -0700 Subject: [PATCH 8/8] fix: use visible width in calc_wrapped_height for non-table lines MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit calc_wrapped_height measured each word with raw width — including markdown markers — so a token like [#cdd-rl-post-training](https://...) counted ~80 raw cols when it actually renders as ~21. Lines with long markdown links over-reserved height, leaving blank padding rows below the visible content. Switch to cell_visible_width per word so the strip-shrinkage already used for tables applies here too. Handles the common single-word link case (multi-word constructs with internal spaces are a separate, rarer case in non-table content). Co-Authored-By: Claude Opus 4.7 (1M context) --- src/ui/content.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ui/content.rs b/src/ui/content.rs index 22056e9..a5783c5 100644 --- a/src/ui/content.rs +++ b/src/ui/content.rs @@ -141,7 +141,11 @@ pub fn render_content(f: &mut Frame, app: &mut App, area: Rect) { let mut current_line_width = 0usize; for word in text.split_whitespace() { - let word_width = unicode_width::UnicodeWidthStr::width(word); + // Use the *visible* width so a single-word markdown atom like + // `[label](https://very-long-url)` counts as its rendered label width + // (~ "label") instead of its raw source. Otherwise the height calc + // over-reserves lines and the layout shows blank padding rows. + let word_width = cell_visible_width(word); if current_line_width == 0 { if word_width > content_width {