From 9ad059aee4fec9c97b20149d6b1f9bbad3993bbf Mon Sep 17 00:00:00 2001 From: Rock Boynton Date: Wed, 28 Oct 2026 15:15:06 -0700 Subject: [PATCH 1/2] renderdag: Tag prefix line parts with their column A prefix line renderer joins all the columns of a link, term, or pad line into one text part. So a caller cannot tell which column drew each character, and it cannot style each edge, for example with one color for each column. Add a ColumnText part that carries the column of its edge, and make the three renderers give one for each column. A connector between two columns goes to the column whose line ends in the link line, or else to the column farther from the node. That way a fork or a merge takes the color of its branch. The rendered text does not change. --- eden/scm/lib/renderdag/src/pipeline.rs | 3 +- .../src/pipeline/prefix_lines_to_text.rs | 4 +- .../src/pipeline/row_shape_to_prefix_lines.rs | 97 ++++++++++++ .../row_shape_to_prefix_lines/ascii.rs | 129 ++++++++-------- .../row_shape_to_prefix_lines/ascii_large.rs | 117 ++++++++------- .../row_shape_to_prefix_lines/box_drawing.rs | 139 +++++++++--------- eden/scm/lib/renderdag/src/pipeline/types.rs | 8 +- 7 files changed, 309 insertions(+), 188 deletions(-) diff --git a/eden/scm/lib/renderdag/src/pipeline.rs b/eden/scm/lib/renderdag/src/pipeline.rs index 649f6347abb30..cbc0596f961d8 100644 --- a/eden/scm/lib/renderdag/src/pipeline.rs +++ b/eden/scm/lib/renderdag/src/pipeline.rs @@ -189,7 +189,8 @@ mod tests { .parts .iter() .map(|part| match part { - PrefixLinePart::Text(text) => text.as_str(), + PrefixLinePart::Text(text) + | PrefixLinePart::ColumnText { text, .. } => text.as_str(), PrefixLinePart::NodeGlyph => "o", }) .collect(); diff --git a/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs b/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs index e5d595f5bc39a..42f4dd48d28c6 100644 --- a/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs +++ b/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs @@ -124,7 +124,9 @@ fn render_prefix_line(prefix_line: &PrefixLine, glyph: &str) -> String { let mut out = String::new(); for part in prefix_line.parts.iter() { match part { - PrefixLinePart::Text(text) => out.push_str(text), + PrefixLinePart::Text(text) | PrefixLinePart::ColumnText { text, .. } => { + out.push_str(text) + } PrefixLinePart::NodeGlyph => out.push_str(glyph), } } diff --git a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines.rs b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines.rs index c146c14e96d11..8234941e290c2 100644 --- a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines.rs +++ b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines.rs @@ -11,8 +11,11 @@ pub mod box_drawing; use crate::pipeline::types::DashStyle; use crate::pipeline::types::GraphRowShape; +use crate::pipeline::types::LinkLine; use crate::pipeline::types::PadLine; +use crate::pipeline::types::PrefixLine; use crate::pipeline::types::PrefixLineKind; +use crate::pipeline::types::PrefixLinePart; fn ancestry_lines( row: &GraphRowShape, @@ -36,3 +39,97 @@ fn ancestry_lines( (kind, columns) }) } + +/// True if a link line cell only carries a horizontal line past its column. +fn is_horizontal_run(cell: LinkLine) -> bool { + cell.intersects(LinkLine::HORIZONTAL) + && !cell.intersects(LinkLine::CHILD | LinkLine::ANY_FORK_OR_MERGE) +} + +/// Find the owning column of each link line connector, scanning each horizontal run only once. +/// +/// A connector joins two columns. It belongs to the column whose line ends in this link line. If +/// neither line or both lines end here, it belongs to the column that is farther from the node, +/// which is the branch that forks or merges. +fn connector_columns( + node_column: usize, + link_row: &[LinkLine], +) -> impl Iterator + '_ { + let mut right = 0; + let mut owner = 0; + (0..link_row.len()).map(move |segment| { + if segment + 1 == link_row.len() { + return segment; + } + // Every connector before the right endpoint has the same owner. + if right <= segment { + let left = segment; + right = segment + 1; + while right + 1 < link_row.len() && is_horizontal_run(link_row[right]) { + right += 1; + } + owner = match ( + link_row[left].intersects(LinkLine::VERTICAL), + link_row[right].intersects(LinkLine::VERTICAL), + ) { + (true, false) => right, + (false, true) => left, + _ if node_column <= left => right, + _ => left, + }; + } + owner + }) +} + +/// Make a part for graph text that draws part of the edge in `column`. +fn column_part(column: usize, text: impl Into) -> PrefixLinePart { + let text = text.into(); + if text.trim().is_empty() { + PrefixLinePart::Text(text) + } else { + PrefixLinePart::ColumnText { column, text } + } +} + +/// Append graph text, reusing the last part when its column ownership matches. +#[inline] +fn push_column_part(parts: &mut Vec, column: usize, text: &str) { + let column = (!text.trim().is_empty()).then_some(column); + match (parts.last_mut(), column) { + (Some(PrefixLinePart::Text(last)), None) => last.push_str(text), + ( + Some(PrefixLinePart::ColumnText { + column: last_column, + text: last, + }), + Some(column), + ) if *last_column == column => last.push_str(text), + (_, None) => parts.push(PrefixLinePart::Text(text.to_owned())), + (_, Some(column)) => parts.push(PrefixLinePart::ColumnText { + column, + text: text.to_owned(), + }), + } +} + +/// Render one cell per column, coalescing adjacent parts with the same ownership. +#[inline] +fn column_line<'a>(kind: PrefixLineKind, cells: impl IntoIterator) -> PrefixLine { + let cells = cells.into_iter(); + let mut parts = Vec::with_capacity(cells.size_hint().0.min(4)); + for (column, text) in cells.enumerate() { + push_column_part(&mut parts, column, text); + } + PrefixLine { parts, kind } +} + +/// Make the parts for graph text that has one character for each of `columns`. +fn column_parts( + text: &str, + columns: impl IntoIterator, +) -> impl Iterator { + text.chars() + .zip(columns) + .map(|(c, column)| column_part(column, c)) +} diff --git a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii.rs b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii.rs index 170d52f7ef05e..10406e45836d1 100644 --- a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii.rs +++ b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii.rs @@ -6,6 +6,10 @@ */ use super::ancestry_lines; +use super::column_line; +use super::connector_columns; +use super::is_horizontal_run; +use super::push_column_part; use crate::pipeline::types::DashStyle; use crate::pipeline::types::GraphRowShape; use crate::pipeline::types::LinkLine; @@ -32,8 +36,8 @@ impl PrefixLineRenderer for AsciiPrefixLineRenderer { fn next_prefix_lines(&mut self, line: &GraphRowShape) -> Vec { let mut lines = Vec::new(); let (ancestor, ancestor_line) = match line.style.dash_style { - DashStyle::Persistent => ('.', ". "), - DashStyle::Once => (':', ": "), + DashStyle::Persistent => (".", ". "), + DashStyle::Once => (":", ": "), }; lines.push(render_pad_line( @@ -47,7 +51,7 @@ impl PrefixLineRenderer for AsciiPrefixLineRenderer { parts: Vec::new(), kind: PrefixLineKind::Node, }; - for entry in line.node_line.iter() { + for (i, entry) in line.node_line.iter().enumerate() { match entry { NodeLine::Node => { node_line.parts.push(PrefixLinePart::NodeGlyph); @@ -55,9 +59,9 @@ impl PrefixLineRenderer for AsciiPrefixLineRenderer { .parts .push(PrefixLinePart::Text(String::from(" "))); } - NodeLine::Parent => push_text(&mut node_line, "| "), - NodeLine::Ancestor => push_text(&mut node_line, ancestor_line), - NodeLine::Blank => push_text(&mut node_line, " "), + NodeLine::Parent => push_column_part(&mut node_line.parts, i, "| "), + NodeLine::Ancestor => push_column_part(&mut node_line.parts, i, ancestor_line), + NodeLine::Blank => push_column_part(&mut node_line.parts, i, " "), } } lines.push(node_line); @@ -70,64 +74,69 @@ impl PrefixLineRenderer for AsciiPrefixLineRenderer { // Render the link line if let Some(link_row) = line.link_line.as_ref() { - let mut link_line = String::new(); + let mut parts = Vec::new(); let any_horizontal = link_row .iter() .any(|cur| cur.intersects(LinkLine::HORIZONTAL)); - let mut iter = link_row + for (i, (cur, connector_column)) in link_row .iter() .copied() - .chain(std::iter::once(LinkLine::empty())) - .peekable(); - while let Some(cur) = iter.next() { - let next = match iter.peek() { - Some(&v) => v, - None => break, - }; + .zip(connector_columns(line.node_column, link_row)) + .enumerate() + { + let next = link_row.get(i + 1).copied().unwrap_or_default(); // Draw the parent/ancestor line. - if cur.intersects(LinkLine::HORIZONTAL) { + let column_text = if cur.intersects(LinkLine::HORIZONTAL) { if cur.intersects(LinkLine::CHILD | LinkLine::ANY_FORK_OR_MERGE) { - link_line.push('+'); + "+" } else { - link_line.push('-'); + "-" } } else if cur.intersects(LinkLine::VERTICAL) { if cur.intersects(LinkLine::ANY_FORK_OR_MERGE) && any_horizontal { - link_line.push('+'); + "+" } else if cur.intersects(LinkLine::VERT_PARENT) { - link_line.push('|'); + "|" } else { - link_line.push(ancestor); + ancestor } } else if cur.intersects(LinkLine::ANY_MERGE) && any_horizontal { - link_line.push('\''); + "'" } else if cur.intersects(LinkLine::ANY_FORK) && any_horizontal { - link_line.push('.'); + "." } else { - link_line.push(' '); - } + " " + }; // Draw the connecting line. - if cur.intersects(LinkLine::HORIZONTAL) { - link_line.push('-'); + let connector = if cur.intersects(LinkLine::HORIZONTAL) { + "-" } else if cur.intersects(LinkLine::RIGHT_MERGE) { if next.intersects(LinkLine::LEFT_FORK) && !any_horizontal { - link_line.push('\\'); + "\\" } else { - link_line.push('-'); + "-" } } else if cur.intersects(LinkLine::RIGHT_FORK) { if next.intersects(LinkLine::LEFT_MERGE) && !any_horizontal { - link_line.push('/'); + "/" } else { - link_line.push('-'); + "-" } } else { - link_line.push(' '); - } + " " + }; + + let column = if is_horizontal_run(cur) { + connector_column + } else { + i + }; + push_column_part(&mut parts, column, column_text); + push_column_part(&mut parts, connector_column, connector); } lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(link_line)], + parts, kind: PrefixLineKind::Link, }); } @@ -135,23 +144,17 @@ impl PrefixLineRenderer for AsciiPrefixLineRenderer { // Render the term line if let Some(term_row) = line.term_line.as_ref() { let term_strs = ["| ", "~ "]; - for term_str in term_strs.iter() { - let mut term_line = String::new(); - for (i, term) in term_row.iter().enumerate() { - if *term { - term_line.push_str(term_str); - } else { - term_line.push_str(match line.ancestry_line[i] { - PadLine::Parent => "| ", - PadLine::Ancestor => ancestor_line, - PadLine::Blank => " ", - }); - } - } - lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(term_line)], - kind: PrefixLineKind::Term, - }); + for &term_str in &term_strs { + lines.push(column_line( + PrefixLineKind::Term, + term_row.iter().enumerate().map(|(i, term)| { + if *term { + term_str + } else { + pad_cell(ancestor_line, line.ancestry_line[i]) + } + }), + )); } } @@ -168,24 +171,16 @@ fn render_pad_line( line: impl IntoIterator, kind: PrefixLineKind, ) -> PrefixLine { - PrefixLine { - parts: vec![PrefixLinePart::Text(render_pad_text(ancestor, line))], + column_line( kind, - } + line.into_iter().map(|entry| pad_cell(ancestor, entry)), + ) } -fn render_pad_text(ancestor: &str, line: impl IntoIterator) -> String { - let mut out = String::new(); - for entry in line { - out.push_str(match entry { - PadLine::Parent => "| ", - PadLine::Ancestor => ancestor, - PadLine::Blank => " ", - }); +fn pad_cell(ancestor: &str, entry: PadLine) -> &str { + match entry { + PadLine::Parent => "| ", + PadLine::Ancestor => ancestor, + PadLine::Blank => " ", } - out -} - -fn push_text(line: &mut PrefixLine, text: &str) { - line.parts.push(PrefixLinePart::Text(text.to_owned())); } diff --git a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii_large.rs b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii_large.rs index b7a74d5340d3b..fb63588c0de41 100644 --- a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii_large.rs +++ b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/ascii_large.rs @@ -6,6 +6,10 @@ */ use super::ancestry_lines; +use super::column_part; +use super::column_parts; +use super::connector_columns; +use super::is_horizontal_run; use crate::pipeline::types::DashStyle; use crate::pipeline::types::GraphRowShape; use crate::pipeline::types::LinkLine; @@ -50,24 +54,15 @@ impl PrefixLineRenderer for AsciiLargePrefixLineRenderer { for (i, entry) in line.node_line.iter().enumerate() { match entry { NodeLine::Node => { - if i > 0 { - node_line - .parts - .push(PrefixLinePart::Text(String::from(" "))); - } + node_line.parts.extend(gap(i)); node_line.parts.push(PrefixLinePart::NodeGlyph); node_line .parts .push(PrefixLinePart::Text(String::from(" "))); } - NodeLine::Parent => push_text(&mut node_line, if i > 0 { " | " } else { "| " }), - NodeLine::Ancestor => { - if i > 0 { - push_text(&mut node_line, " "); - } - push_text(&mut node_line, ancestor_line); - } - NodeLine::Blank => push_text(&mut node_line, if i > 0 { " " } else { " " }), + NodeLine::Parent => node_line.parts.extend(cell(i, "| ")), + NodeLine::Ancestor => node_line.parts.extend(cell(i, ancestor_line)), + NodeLine::Blank => node_line.parts.extend(cell(i, " ")), } } lines.push(node_line); @@ -80,9 +75,24 @@ impl PrefixLineRenderer for AsciiLargePrefixLineRenderer { // Render the link line if let Some(link_row) = line.link_line.as_ref() { - let mut top_link_line = String::new(); - let mut bot_link_line = String::new(); - for (i, cur) in link_row.iter().enumerate() { + let mut top_parts = Vec::new(); + let mut bot_parts = Vec::new(); + let mut previous_column = None; + for (i, (cur, right_column)) in link_row + .iter() + .zip(connector_columns(line.node_column, link_row)) + .enumerate() + { + let mut top_link_line = String::new(); + let mut bot_link_line = String::new(); + // The first column has no left slot. + let left_column = previous_column.replace(right_column); + let center_column = + if !cur.intersects(LinkLine::VERTICAL) && is_horizontal_run(*cur) { + right_column + } else { + i + }; // Top left if i > 0 { if cur.intersects(LinkLine::LEFT_MERGE_PARENT) { @@ -154,13 +164,18 @@ impl PrefixLineRenderer for AsciiLargePrefixLineRenderer { } else { bot_link_line.push(' '); } + + let top_columns = left_column.into_iter().chain([center_column, right_column]); + top_parts.extend(column_parts(&top_link_line, top_columns)); + let bot_columns = left_column.into_iter().chain([i, right_column]); + bot_parts.extend(column_parts(&bot_link_line, bot_columns)); } lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(top_link_line)], + parts: top_parts, kind: PrefixLineKind::Link, }); lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(bot_link_line)], + parts: bot_parts, kind: PrefixLineKind::Link, }); } @@ -169,23 +184,20 @@ impl PrefixLineRenderer for AsciiLargePrefixLineRenderer { if let Some(term_row) = line.term_line.as_ref() { let term_strs = ["| ", "~ "]; for term_str in term_strs.iter() { - let mut term_line = String::new(); - for (i, term) in term_row.iter().enumerate() { - if i > 0 { - term_line.push(' '); - } - if *term { - term_line.push_str(term_str); - } else { - term_line.push_str(match line.ancestry_line[i] { - PadLine::Parent => "| ", - PadLine::Ancestor => ancestor_line, - PadLine::Blank => " ", - }); - } - } + let parts = term_row + .iter() + .enumerate() + .flat_map(|(i, term)| { + let text = if *term { + term_str + } else { + pad_cell(ancestor_line, line.ancestry_line[i]) + }; + cell(i, text) + }) + .collect(); lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(term_line)], + parts, kind: PrefixLineKind::Term, }); } @@ -204,27 +216,28 @@ fn render_pad_line( line: impl IntoIterator, kind: PrefixLineKind, ) -> PrefixLine { - PrefixLine { - parts: vec![PrefixLinePart::Text(render_pad_text(ancestor, line))], - kind, - } + let parts = line + .into_iter() + .enumerate() + .flat_map(|(i, entry)| cell(i, pad_cell(ancestor, entry))) + .collect(); + PrefixLine { parts, kind } } -fn render_pad_text(ancestor: &str, line: impl IntoIterator) -> String { - let mut out = String::new(); - for (i, entry) in line.into_iter().enumerate() { - if i > 0 { - out.push(' '); - } - out.push_str(match entry { - PadLine::Parent => "| ", - PadLine::Ancestor => ancestor, - PadLine::Blank => " ", - }); +fn pad_cell(ancestor: &str, entry: PadLine) -> &str { + match entry { + PadLine::Parent => "| ", + PadLine::Ancestor => ancestor, + PadLine::Blank => " ", } - out } -fn push_text(line: &mut PrefixLine, text: &str) { - line.parts.push(PrefixLinePart::Text(text.to_owned())); +/// The space before every column but the first. The first column is only 2 characters wide. +fn gap(column: usize) -> Option { + (column > 0).then(|| PrefixLinePart::Text(String::from(" "))) +} + +/// A 2-character cell for `column`, after its gap. +fn cell(column: usize, text: &str) -> impl Iterator { + gap(column).into_iter().chain([column_part(column, text)]) } diff --git a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/box_drawing.rs b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/box_drawing.rs index 869d7d88e0af1..a9a261473148f 100644 --- a/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/box_drawing.rs +++ b/eden/scm/lib/renderdag/src/pipeline/row_shape_to_prefix_lines/box_drawing.rs @@ -8,6 +8,10 @@ use std::marker::PhantomData; use super::ancestry_lines; +use super::column_line; +use super::connector_columns; +use super::is_horizontal_run; +use super::push_column_part; use crate::pipeline::types::DashStyle; use crate::pipeline::types::GraphRowShape; use crate::pipeline::types::LinkLine; @@ -140,7 +144,7 @@ impl PrefixLineRenderer for BoxDrawingPrefixLineRen parts: Vec::new(), kind: PrefixLineKind::Node, }; - for entry in line.node_line.iter() { + for (i, entry) in line.node_line.iter().enumerate() { match entry { NodeLine::Node => { node_line.parts.push(PrefixLinePart::NodeGlyph); @@ -148,9 +152,11 @@ impl PrefixLineRenderer for BoxDrawingPrefixLineRen .parts .push(PrefixLinePart::Text(String::from(" "))); } - NodeLine::Parent => push_text(&mut node_line, glyphs[glyph::PARENT]), - NodeLine::Ancestor => push_text(&mut node_line, glyphs[ancestor]), - NodeLine::Blank => push_text(&mut node_line, glyphs[glyph::SPACE]), + NodeLine::Parent => { + push_column_part(&mut node_line.parts, i, glyphs[glyph::PARENT]) + } + NodeLine::Ancestor => push_column_part(&mut node_line.parts, i, glyphs[ancestor]), + NodeLine::Blank => push_column_part(&mut node_line.parts, i, glyphs[glyph::SPACE]), } } lines.push(node_line); @@ -165,35 +171,39 @@ impl PrefixLineRenderer for BoxDrawingPrefixLineRen // Render the link line #[allow(clippy::if_same_then_else)] if let Some(link_row) = line.link_line.as_ref() { - let mut link_line = String::new(); - for cur in link_row.iter() { - if cur.intersects(LinkLine::HORIZONTAL) { + let mut parts = Vec::new(); + for (i, (cur, connector_column)) in link_row + .iter() + .zip(connector_columns(line.node_column, link_row)) + .enumerate() + { + let link_glyph = if cur.intersects(LinkLine::HORIZONTAL) { if cur.intersects(LinkLine::CHILD) { - link_line.push_str(glyphs[glyph::JOIN_BOTH]); + glyph::JOIN_BOTH } else if cur.intersects(LinkLine::ANY_FORK) && cur.intersects(LinkLine::ANY_MERGE) { - link_line.push_str(glyphs[glyph::JOIN_BOTH]); + glyph::JOIN_BOTH } else if cur.intersects(LinkLine::ANY_FORK) && cur.intersects(LinkLine::VERT_PARENT) && !line.merge { - link_line.push_str(glyphs[glyph::JOIN_BOTH]); + glyph::JOIN_BOTH } else if cur.intersects(LinkLine::ANY_FORK) { - link_line.push_str(glyphs[glyph::FORK_BOTH]); + glyph::FORK_BOTH } else if cur.intersects(LinkLine::ANY_MERGE) { - link_line.push_str(glyphs[glyph::MERGE_BOTH]); + glyph::MERGE_BOTH } else { - link_line.push_str(glyphs[glyph::HORIZONTAL]); + glyph::HORIZONTAL } } else if cur.intersects(LinkLine::VERT_PARENT) && !line.merge { let left = cur.intersects(LinkLine::LEFT_MERGE | LinkLine::LEFT_FORK); let right = cur.intersects(LinkLine::RIGHT_MERGE | LinkLine::RIGHT_FORK); match (left, right) { - (true, true) => link_line.push_str(glyphs[glyph::JOIN_BOTH]), - (true, false) => link_line.push_str(glyphs[glyph::JOIN_LEFT]), - (false, true) => link_line.push_str(glyphs[glyph::JOIN_RIGHT]), - (false, false) => link_line.push_str(glyphs[glyph::PARENT]), + (true, true) => glyph::JOIN_BOTH, + (true, false) => glyph::JOIN_LEFT, + (false, true) => glyph::JOIN_RIGHT, + (false, false) => glyph::PARENT, } } else if cur.intersects(LinkLine::VERT_PARENT | LinkLine::VERT_ANCESTOR) && !cur.intersects(LinkLine::LEFT_FORK | LinkLine::RIGHT_FORK) @@ -201,47 +211,55 @@ impl PrefixLineRenderer for BoxDrawingPrefixLineRen let left = cur.intersects(LinkLine::LEFT_MERGE); let right = cur.intersects(LinkLine::RIGHT_MERGE); match (left, right) { - (true, true) => link_line.push_str(glyphs[glyph::JOIN_BOTH]), - (true, false) => link_line.push_str(glyphs[glyph::JOIN_LEFT]), - (false, true) => link_line.push_str(glyphs[glyph::JOIN_RIGHT]), + (true, true) => glyph::JOIN_BOTH, + (true, false) => glyph::JOIN_LEFT, + (false, true) => glyph::JOIN_RIGHT, (false, false) => { if cur.intersects(LinkLine::VERT_ANCESTOR) { - link_line.push_str(glyphs[ancestor]); + ancestor } else { - link_line.push_str(glyphs[glyph::PARENT]); + glyph::PARENT } } } } else if cur.intersects(LinkLine::LEFT_FORK) && cur.intersects(LinkLine::LEFT_MERGE | LinkLine::CHILD) { - link_line.push_str(glyphs[glyph::JOIN_LEFT]); + glyph::JOIN_LEFT } else if cur.intersects(LinkLine::RIGHT_FORK) && cur.intersects(LinkLine::RIGHT_MERGE | LinkLine::CHILD) { - link_line.push_str(glyphs[glyph::JOIN_RIGHT]); + glyph::JOIN_RIGHT } else if cur.intersects(LinkLine::LEFT_MERGE) && cur.intersects(LinkLine::RIGHT_MERGE) { - link_line.push_str(glyphs[glyph::MERGE_BOTH]); + glyph::MERGE_BOTH } else if cur.intersects(LinkLine::LEFT_FORK) && cur.intersects(LinkLine::RIGHT_FORK) { - link_line.push_str(glyphs[glyph::FORK_BOTH]); + glyph::FORK_BOTH } else if cur.intersects(LinkLine::LEFT_FORK) { - link_line.push_str(glyphs[glyph::FORK_LEFT]); + glyph::FORK_LEFT } else if cur.intersects(LinkLine::LEFT_MERGE) { - link_line.push_str(glyphs[glyph::MERGE_LEFT]); + glyph::MERGE_LEFT } else if cur.intersects(LinkLine::RIGHT_FORK) { - link_line.push_str(glyphs[glyph::FORK_RIGHT]); + glyph::FORK_RIGHT } else if cur.intersects(LinkLine::RIGHT_MERGE) { - link_line.push_str(glyphs[glyph::MERGE_RIGHT]); + glyph::MERGE_RIGHT } else { - link_line.push_str(glyphs[glyph::SPACE]); - } + glyph::SPACE + }; + let (column_text, connector) = split_glyph(glyphs[link_glyph]); + let column = if is_horizontal_run(*cur) { + connector_column + } else { + i + }; + push_column_part(&mut parts, column, column_text); + push_column_part(&mut parts, connector_column, connector); } lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(link_line)], + parts, kind: PrefixLineKind::Link, }); } @@ -249,20 +267,17 @@ impl PrefixLineRenderer for BoxDrawingPrefixLineRen // Render the term line if let Some(term_row) = line.term_line.as_ref() { let term_strs = [glyphs[glyph::PARENT], glyphs[glyph::TERMINATION]]; - for term_str in term_strs.iter() { - let mut term_line = String::new(); - for (i, term) in term_row.iter().enumerate() { - if *term { - term_line.push_str(term_str); - } else { - term_line - .push_str(glyphs[pad_line_to_glyph(line.ancestry_line[i], ancestor)]); - } - } - lines.push(PrefixLine { - parts: vec![PrefixLinePart::Text(term_line)], - kind: PrefixLineKind::Term, - }); + for &term_str in &term_strs { + lines.push(column_line( + PrefixLineKind::Term, + term_row.iter().enumerate().map(|(i, term)| { + if *term { + term_str + } else { + glyphs[pad_line_to_glyph(line.ancestry_line[i], ancestor)] + } + }), + )); } } @@ -283,8 +298,13 @@ where } } -fn push_text(line: &mut PrefixLine, text: &str) { - line.parts.push(PrefixLinePart::Text(text.to_owned())); +/// Split a glyph into the text in its own column and the connector to the next column. +fn split_glyph(glyph: &str) -> (&str, &str) { + // A DEC special graphics glyph starts with an escape sequence. Keep it with the first + // character, so that the two halves join to the same bytes. + let start = if glyph.starts_with("\x1B(0") { 3 } else { 0 }; + let end = start + glyph[start..].chars().next().map_or(0, char::len_utf8); + glyph.split_at(end) } fn render_pad_line( @@ -293,24 +313,11 @@ fn render_pad_line( line: impl IntoIterator, kind: PrefixLineKind, ) -> PrefixLine { - PrefixLine { - parts: vec![PrefixLinePart::Text(render_pad_text( - glyphs, ancestor, line, - ))], + column_line( kind, - } -} - -fn render_pad_text( - glyphs: &[&str; glyph::COUNT], - ancestor: usize, - line: impl IntoIterator, -) -> String { - let mut out = String::new(); - for entry in line { - out.push_str(glyphs[pad_line_to_glyph(entry, ancestor)]); - } - out + line.into_iter() + .map(|entry| glyphs[pad_line_to_glyph(entry, ancestor)]), + ) } fn pad_line_to_glyph(line: PadLine, ancestor: usize) -> usize { diff --git a/eden/scm/lib/renderdag/src/pipeline/types.rs b/eden/scm/lib/renderdag/src/pipeline/types.rs index 5f46b4902280d..9813c0007aae7 100644 --- a/eden/scm/lib/renderdag/src/pipeline/types.rs +++ b/eden/scm/lib/renderdag/src/pipeline/types.rs @@ -255,9 +255,15 @@ pub struct PrefixLine { /// A piece of a graph prefix line. #[derive(Clone, Debug, PartialEq, Eq)] pub enum PrefixLinePart { - /// Literal graph text. + /// Graph text with no column, such as blank cells and the spacing around a node glyph. Text(String), + /// Graph text that draws part of the edge in `column`. + /// + /// Callers can use the column to style each edge, for example to give each column its own + /// color. A connector that joins two columns belongs to the branch that forks or merges there. + ColumnText { column: usize, text: String }, + /// Placeholder for the rendered node glyph. /// /// Keeping the glyph as a slot lets callers cache prefix lines and fill in From f9c1a0c5d3670dcd9f5ea28537864bb4698363f9 Mon Sep 17 00:00:00 2001 From: Rock Boynton Date: Wed, 28 Oct 2026 15:28:02 -0700 Subject: [PATCH 2/2] renderdag: Pass each rendered line to a callback The text output stage writes each line into a String, which drops the column of each piece. A caller that styles each column through its own writer, such as a CLI formatter, cannot use the stage. It also cannot copy the stage, because the padding and message rules are private. Add write_next_lines, which gives each line to a callback as a list of pieces that keep their columns. The stage still trims trailing whitespace, now across pieces. write_next_text keeps its signature and calls the new method, so current callers see no change. --- eden/scm/lib/renderdag/src/output.rs | 69 -------- .../src/pipeline/prefix_lines_to_text.rs | 153 +++++++++++++++--- eden/scm/lib/renderdag/src/pipeline/types.rs | 12 ++ 3 files changed, 142 insertions(+), 92 deletions(-) diff --git a/eden/scm/lib/renderdag/src/output.rs b/eden/scm/lib/renderdag/src/output.rs index b85cf7520a8ab..79bf5cbc95b8b 100644 --- a/eden/scm/lib/renderdag/src/output.rs +++ b/eden/scm/lib/renderdag/src/output.rs @@ -22,75 +22,6 @@ pub struct OutputRendererOptions { pub graph_row_shape_style: crate::pipeline::types::GraphRowShapeStyle, } -/// Common stateful string line output utilities shared by ASCII and box-drawing -/// renderers. -#[derive(Default)] -pub(crate) struct OutputRendererState { - queued_pad_line: Option, - /// If > 1, no need for an extra separator for the next row. - /// Separator is only needed for adjacent single-line node lines. - /// If there are link, pad, term, or other kinds of node lines, - /// no need to draw separator. - row_height: usize, -} - -impl OutputRendererState { - pub(crate) fn begin_row(&mut self, out: &mut String, separator_line: bool) { - self.flush_queued_pad_line(out); - if separator_line && self.row_height == 1 { - self.push_line(out, ""); - } - self.row_height = 0; - } - - pub(crate) fn push_line_with_message( - &mut self, - out: &mut String, - mut line: String, - message: Option<&str>, - ) { - if let Some(message) = message { - line.push(' '); - line.push_str(message); - } - self.push_line(out, &line); - } - - pub(crate) fn push_pad_lines<'a>( - &mut self, - out: &mut String, - base_pad_line: &str, - message_lines: impl Iterator, - ) -> bool { - let mut emitted = false; - for message in message_lines { - let mut pad_line = String::with_capacity(base_pad_line.len() + message.len() + 1); - pad_line.push_str(base_pad_line); - pad_line.push(' '); - pad_line.push_str(message); - self.push_line(out, &pad_line); - emitted = true; - } - emitted - } - - pub(crate) fn queue_pad_line(&mut self, pad_line: String) { - self.queued_pad_line = Some(pad_line); - } - - fn push_line(&mut self, out: &mut String, line: &str) { - out.push_str(line.trim_end()); - out.push('\n'); - self.row_height = self.row_height.saturating_add(1); - } - - fn flush_queued_pad_line(&mut self, out: &mut String) { - if let Some(pad_line) = self.queued_pad_line.take() { - self.push_line(out, &pad_line); - } - } -} - pub struct OutputRendererBuilder where R: Renderer> + Sized, diff --git a/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs b/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs index 42f4dd48d28c6..f38cc25e249e6 100644 --- a/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs +++ b/eden/scm/lib/renderdag/src/pipeline/prefix_lines_to_text.rs @@ -5,8 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -use crate::output::OutputRendererState; use crate::pad::pad_lines; +use crate::pipeline::types::LinePiece; use crate::pipeline::types::LinkLinePlacement; use crate::pipeline::types::PrefixLine; use crate::pipeline::types::PrefixLineKind; @@ -14,11 +14,16 @@ use crate::pipeline::types::PrefixLinePart; /// Stateful renderer for the final pipeline stage. /// -/// It fills node glyph slots, attaches message lines, and writes complete text -/// lines for one rendered graph row at a time. +/// It fills node glyph slots, attaches message lines, and writes complete text lines for one +/// rendered graph row at a time. #[derive(Default)] pub struct PrefixLinesToText { - state: OutputRendererState, + /// The queued pad line, and the node glyph to render it with. + queued_pad_line: Option<(PrefixLine, String)>, + /// If > 1, no need for an extra separator for the next row. Separator is only needed for + /// adjacent single-line node lines. If there are link, pad, term, or other kinds of node lines, + /// no need to draw separator. + row_height: usize, link_line_placement: LinkLinePlacement, } @@ -65,6 +70,35 @@ impl PrefixLinesToText { message: &str, min_row_height: usize, ) { + self.write_next_lines( + |line| { + for piece in line { + out.push_str(piece.text); + } + out.push('\n'); + }, + prefix_lines, + separator_line, + glyph, + message, + min_row_height, + ); + } + + /// Give each line of the next row to `out`, as a list of pieces. + /// + /// A caller can use this to style each piece, for example to give each column its own color. + /// The last piece of a line has no trailing whitespace. + pub fn write_next_lines( + &mut self, + mut out: impl FnMut(&[LinePiece<'_>]), + prefix_lines: Vec, + separator_line: bool, + glyph: &str, + message: &str, + min_row_height: usize, + ) { + let out = &mut out; let mut message_lines = pad_lines(message.lines(), min_row_height); let extra_message_lines = if self.link_line_placement == LinkLinePlacement::Bottom { let fixed_lines = prefix_lines @@ -82,7 +116,7 @@ impl PrefixLinesToText { let mut repeatable_line = None; let mut has_term_row = false; - self.state.begin_row(out, separator_line); + self.begin_row(out, separator_line); for prefix_line in prefix_lines { // Text starts at the node; extra message lines precede links only for Bottom. @@ -91,9 +125,9 @@ impl PrefixLinesToText { } if prefix_line.kind == PrefixLineKind::PostNode { if extra_message_lines > 0 { - let base_line = render_prefix_line(&prefix_line, glyph); + let base_line = line_pieces(&prefix_line, glyph); // Reserve message lines for the mandatory Link, Term, and Ancestry lines. - self.state.push_pad_lines( + self.push_pad_lines( out, &base_line, message_lines.by_ref().take(extra_message_lines), @@ -106,29 +140,102 @@ impl PrefixLinesToText { continue; } has_term_row |= prefix_line.kind == PrefixLineKind::Term; - let text = render_prefix_line(&prefix_line, glyph); - self.state - .push_line_with_message(out, text, message_lines.next()); + let line = line_pieces(&prefix_line, glyph); + self.push_line_with_message(out, line, message_lines.next()); } if let Some(repeatable_line) = repeatable_line { - let base_line = render_prefix_line(&repeatable_line, glyph); - if !self.state.push_pad_lines(out, &base_line, message_lines) && has_term_row { - self.state.queue_pad_line(base_line); + let base_line = line_pieces(&repeatable_line, glyph); + if !self.push_pad_lines(out, &base_line, message_lines) && has_term_row { + self.queue_pad_line(repeatable_line, glyph); } } } -} -fn render_prefix_line(prefix_line: &PrefixLine, glyph: &str) -> String { - let mut out = String::new(); - for part in prefix_line.parts.iter() { - match part { - PrefixLinePart::Text(text) | PrefixLinePart::ColumnText { text, .. } => { - out.push_str(text) - } - PrefixLinePart::NodeGlyph => out.push_str(glyph), + fn begin_row(&mut self, out: &mut impl FnMut(&[LinePiece<'_>]), separator_line: bool) { + self.flush_queued_pad_line(out); + if separator_line && self.row_height == 1 { + self.push_line(out, &mut []); + } + self.row_height = 0; + } + + fn push_line_with_message<'a>( + &mut self, + out: &mut impl FnMut(&[LinePiece<'_>]), + mut line: Vec>, + message: Option<&'a str>, + ) { + if let Some(message) = message { + line.push(LinePiece { + column: None, + text: " ", + }); + line.push(LinePiece { + column: None, + text: message, + }); + } + self.push_line(out, &mut line); + } + + fn push_pad_lines<'a>( + &mut self, + out: &mut impl FnMut(&[LinePiece<'_>]), + base_pad_line: &[LinePiece<'_>], + message_lines: impl Iterator, + ) -> bool { + let mut emitted = false; + for message in message_lines { + self.push_line_with_message(out, base_pad_line.to_vec(), Some(message)); + emitted = true; } + emitted + } + + fn queue_pad_line(&mut self, pad_line: PrefixLine, glyph: &str) { + self.queued_pad_line = Some((pad_line, glyph.to_owned())); } - out + + fn push_line(&mut self, out: &mut impl FnMut(&[LinePiece<'_>]), line: &mut [LinePiece<'_>]) { + out(trim_end(line)); + self.row_height = self.row_height.saturating_add(1); + } + + fn flush_queued_pad_line(&mut self, out: &mut impl FnMut(&[LinePiece<'_>])) { + if let Some((pad_line, glyph)) = self.queued_pad_line.take() { + self.push_line(out, &mut line_pieces(&pad_line, &glyph)); + } + } +} + +/// Remove trailing whitespace from a line, across pieces. +fn trim_end<'s, 'a>(line: &'s mut [LinePiece<'a>]) -> &'s [LinePiece<'a>] { + let end = line + .iter_mut() + .rposition(|piece| { + piece.text = piece.text.trim_end(); + !piece.text.is_empty() + }) + .map_or(0, |index| index + 1); + &line[..end] +} + +/// Convert a prefix line into line pieces, with `glyph` in the node glyph slot. +fn line_pieces<'a>(prefix_line: &'a PrefixLine, glyph: &'a str) -> Vec> { + prefix_line + .parts + .iter() + .map(|part| match part { + PrefixLinePart::Text(text) => LinePiece { column: None, text }, + PrefixLinePart::ColumnText { column, text } => LinePiece { + column: Some(*column), + text, + }, + PrefixLinePart::NodeGlyph => LinePiece { + column: None, + text: glyph, + }, + }) + .collect() } diff --git a/eden/scm/lib/renderdag/src/pipeline/types.rs b/eden/scm/lib/renderdag/src/pipeline/types.rs index 9813c0007aae7..835340244bf9c 100644 --- a/eden/scm/lib/renderdag/src/pipeline/types.rs +++ b/eden/scm/lib/renderdag/src/pipeline/types.rs @@ -319,3 +319,15 @@ pub trait PrefixLineRenderer { /// Convert the next graph row shape into prefix lines. fn next_prefix_lines(&mut self, row_shape: &GraphRowShape) -> Vec; } + +/// A piece of a rendered graph line. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[non_exhaustive] +pub struct LinePiece<'a> { + /// The column of the edge that this text draws. It is `None` for text that is not part of an + /// edge: a node glyph, message text, or spacing. + pub column: Option, + + /// The text of this piece. + pub text: &'a str, +}