diff --git a/nbd/.nbd/tickets/e14172.md b/nbd/.nbd/tickets/e14172.md new file mode 100644 index 0000000..a8843a4 --- /dev/null +++ b/nbd/.nbd/tickets/e14172.md @@ -0,0 +1,68 @@ ++++ +title = "ASCII graph rendering in display.rs" +priority = 5 +status = "done" +ticket_type = "feature" +dependencies = ["9c9ebe"] ++++ +Add `format_graph` and `print_graph` to `src/display.rs` to render a ticket dependency DAG as an ASCII tree. + +## Motivation + +The `nbd graph` command needs to convert the `TicketGraph` data structure (from `src/graph.rs`) into a human-readable ASCII representation. Rendering belongs in `display.rs` next to `format_list` and `format_ticket`. + +## Output format + +The graph is a forest of trees. Each root ticket (no in-graph dependencies) starts at column 0. Its dependents (tickets that depend on it) are indented below it using box-drawing characters. + +``` +a3f9c2 [todo] Fix login bug +├── b7d41e [todo] Add rate limiting +│ └── c9e823 [in_progress] Write tests +└── d1f302 [done] Update docs +e4a781 [todo] New feature (no deps) +``` + +**Node format per line:** +``` +{prefix}{id} [{status}] {title} +``` + +Where `{prefix}` is built from indentation characters (`│ `, `├── `, `└── `). + +**Blocked indicator (optional):** Consider marking tickets that have unresolved (non-done/closed) dependencies with a `[blocked]` tag or `!` prefix so the graph visually distinguishes ready from blocked tickets. + +## Signatures + +```rust +/// Render the full dependency forest as an ASCII tree string. +pub fn format_graph(graph: &TicketGraph) -> String + +/// Print the full dependency forest. +pub fn print_graph(graph: &TicketGraph) + +/// Render the subtree rooted at `root_id` as an ASCII tree string. +pub fn format_subtree(graph: &TicketGraph, root_id: &str) -> String + +/// Print the subtree rooted at `root_id`. +pub fn print_subtree(graph: &TicketGraph, root_id: &str) +``` + +## Implementation notes + +- Use a recursive helper that tracks a `prefix: String` carrying the accumulated indentation characters. +- For each node's children (its dependents in the graph), iterate: + - If not the last child: prefix extension is `│ `; connector is `├── `. + - If the last child: prefix extension is ` `; connector is `└── `. +- Cycle guard: track a `visited: HashSet<&str>` across the recursion; if a node ID is already visited, render `{prefix}{connector}{id} [cycle]` and stop descending. +- Status is shown as the serde string: `todo`, `in_progress`, `done`, `closed`. + +## Files touched +- `src/display.rs` — `format_graph`, `print_graph`, `format_subtree`, `print_subtree` + +## Tests (unit, in `src/tests.rs`) +- `format_graph` on a single ticket with no deps produces a single line. +- `format_graph` on a two-ticket chain shows the child indented with `└──`. +- `format_graph` with a branching parent shows `├──` for all but the last child and `└──` for the last. +- `format_subtree` only shows the specified root's subtree. +- A cycle in the graph does not cause infinite recursion; the repeated node is labelled `[cycle]`. \ No newline at end of file diff --git a/nbd/src/display.rs b/nbd/src/display.rs index 176476d..7c2b8f2 100644 --- a/nbd/src/display.rs +++ b/nbd/src/display.rs @@ -7,6 +7,9 @@ //! in command handlers. The corresponding `format_*` functions return a //! `String` and are provided primarily for testing and composition. +use std::collections::HashSet; + +use crate::graph::TicketGraph; use crate::store::MigrateReport; use crate::ticket::{Status, Ticket, TicketType}; @@ -262,3 +265,113 @@ pub fn format_migrate_report_json(report: &MigrateReport) -> String { pub fn print_migrate_report_json(report: &MigrateReport) { println!("{}", format_migrate_report_json(report)); } + +// ── Graph rendering ─────────────────────────────────────────────────────────── + +/// Format the full dependency forest as an ASCII tree string. +/// +/// Roots (tickets with no in-graph dependencies) appear at column 0, sorted +/// by priority descending. Each root's dependents (tickets that list it as a +/// dependency) are indented below using box-drawing characters: +/// +/// ```text +/// a3f9c2 [todo] Fix login bug +/// ├── b7d41e [in_progress] Add rate limiting +/// │ └── c9e823 [todo] Write tests +/// └── d1f302 [done] Update docs +/// e4a781 [todo] New feature (no deps) +/// ``` +/// +/// Cycles are detected and labelled `[cycle]` rather than looping forever. +pub fn format_graph(graph: &TicketGraph<'_>) -> String { + let mut out = String::new(); + let mut visited: HashSet = HashSet::new(); + for root in graph.roots() { + render_node(graph, &root.id, "", "", &mut visited, &mut out); + } + out +} + +/// Print the full dependency forest to stdout. +pub fn print_graph(graph: &TicketGraph<'_>) { + println!("{}", format_graph(graph)); +} + +/// Format the subtree rooted at `root_id` as an ASCII tree string. +/// +/// Renders `root_id` and every ticket that transitively depends on it +/// (via dependent edges), using the same box-drawing format as +/// [`format_graph`]. Returns an empty string when `root_id` is not in the +/// graph. +pub fn format_subtree(graph: &TicketGraph<'_>, root_id: &str) -> String { + let mut out = String::new(); + let mut visited: HashSet = HashSet::new(); + if graph.get_node(root_id).is_some() { + render_node(graph, root_id, "", "", &mut visited, &mut out); + } + out +} + +/// Print the subtree rooted at `root_id` to stdout. +pub fn print_subtree(graph: &TicketGraph<'_>, root_id: &str) { + println!("{}", format_subtree(graph, root_id)); +} + +// ── Internal graph helpers ──────────────────────────────────────────────────── + +/// Recursively render a single node and its dependents into `out`. +/// +/// `prefix` is the accumulated indentation from ancestor levels. +/// `connector` is the box-drawing characters connecting this node to its +/// parent (`""` for roots, `"├── "` or `"└── "` for children). +/// +/// When a node ID is already in `visited`, it is rendered as `[cycle]` and +/// the recursion stops, preventing infinite loops in cyclic data. +fn render_node( + graph: &TicketGraph<'_>, + id: &str, + prefix: &str, + connector: &str, + visited: &mut HashSet, + out: &mut String, +) { + // Cycle detection. + if visited.contains(id) { + append_line(out, &format!("{prefix}{connector}{id} [cycle]")); + return; + } + visited.insert(id.to_string()); + + // Extract the node data we need, cloning so we can drop the borrow before + // recursing (the recursive call needs a mutable `visited`). + let (status_s, title, dependents) = match graph.get_node(id) { + Some(node) => { + let s = status_str(&node.ticket.status); + let t = node.ticket.title.clone(); + let d: Vec = node.dependents.iter().map(|s| s.to_string()).collect(); + (s, t, d) + } + None => return, + }; + + append_line( + out, + &format!("{prefix}{connector}{id} [{status_s}] {title}"), + ); + + let n = dependents.len(); + for (i, dep_id) in dependents.iter().enumerate() { + let is_last = i == n - 1; + let child_connector = if is_last { "└── " } else { "├── " }; + let child_prefix = format!("{prefix}{}", if is_last { " " } else { "│ " }); + render_node(graph, dep_id, &child_prefix, child_connector, visited, out); + } +} + +/// Append `line` to `out`, preceded by a newline if `out` is non-empty. +fn append_line(out: &mut String, line: &str) { + if !out.is_empty() { + out.push('\n'); + } + out.push_str(line); +} diff --git a/nbd/src/tests.rs b/nbd/src/tests.rs index fe9830b..0893a6d 100644 --- a/nbd/src/tests.rs +++ b/nbd/src/tests.rs @@ -1197,6 +1197,122 @@ mod graph { } } +// ── display graph rendering ─────────────────────────────────────────────────── + +/// Tests for the graph rendering functions in [`crate::display`]. +mod display_graph { + use crate::display::{format_graph, format_subtree}; + use crate::graph::TicketGraph; + use crate::ticket::Ticket; + + fn make_ticket(id: &str, deps: &[&str]) -> Ticket { + let mut t = Ticket::new(id.to_string(), format!("Ticket {id}")); + t.dependencies = deps.iter().map(|d| d.to_string()).collect(); + t + } + + /// A single ticket with no deps renders as one line containing its ID, + /// status, and title, with no box-drawing characters. + #[test] + fn single_ticket_no_deps() { + let tickets = vec![make_ticket("aaaaaa", &[])]; + let graph = TicketGraph::build(&tickets); + let out = format_graph(&graph); + assert!(out.contains("aaaaaa"), "should contain ID"); + assert!(out.contains("[todo]"), "should contain status"); + assert!(out.contains("Ticket aaaaaa"), "should contain title"); + assert!(!out.contains("├──"), "should have no branch connectors"); + assert!(!out.contains("└──"), "should have no branch connectors"); + } + + /// A two-ticket chain (B depends on A) renders A at the top level and B + /// indented below it with `└──`. + #[test] + fn two_ticket_chain() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let tickets = vec![a, b]; + let graph = TicketGraph::build(&tickets); + let out = format_graph(&graph); + + // A should appear before B. + let pos_a = out.find("aaaaaa").expect("aaaaaa should appear"); + let pos_b = out.find("bbbbbb").expect("bbbbbb should appear"); + assert!( + pos_a < pos_b, + "root (aaaaaa) should appear before dependent (bbbbbb)" + ); + + // B's line should use the └── connector. + assert!(out.contains("└──"), "last (only) child should use └──"); + assert!(!out.contains("├──"), "only child should not use ├──"); + } + + /// When a root has two dependents, the first uses `├──` and the last `└──`. + #[test] + fn branching_parent() { + let root = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let c = make_ticket("cccccc", &["aaaaaa"]); + let tickets = vec![root, b, c]; + let graph = TicketGraph::build(&tickets); + let out = format_graph(&graph); + + assert!(out.contains("├──"), "non-last child should use ├──"); + assert!(out.contains("└──"), "last child should use └──"); + } + + /// `format_subtree` for a root only includes that root and its dependents, + /// not unrelated tickets. + #[test] + fn subtree_excludes_unrelated() { + let a = make_ticket("aaaaaa", &[]); + let b = make_ticket("bbbbbb", &["aaaaaa"]); + let c = make_ticket("cccccc", &[]); // unrelated + let tickets = vec![a, b, c]; + let graph = TicketGraph::build(&tickets); + + let out = format_subtree(&graph, "aaaaaa"); + assert!(out.contains("aaaaaa"), "root should be present"); + assert!(out.contains("bbbbbb"), "dependent should be present"); + assert!(!out.contains("cccccc"), "unrelated ticket should be absent"); + } + + /// `format_subtree` on an unknown ID returns an empty string. + #[test] + fn subtree_unknown_id_empty() { + let tickets = vec![make_ticket("aaaaaa", &[])]; + let graph = TicketGraph::build(&tickets); + assert!(format_subtree(&graph, "ffffff").is_empty()); + } + + /// When the data contains a cycle, the repeated node is labelled `[cycle]` + /// and the output is finite (no infinite loop). + /// + /// A pure cycle (A depends on B, B depends on A) has no roots, so we use + /// `format_subtree` to trigger rendering from one of the cyclic nodes. + #[test] + fn cycle_labelled() { + let mut a = make_ticket("aaaaaa", &[]); + a.dependencies = vec!["bbbbbb".to_string()]; + let mut b = make_ticket("bbbbbb", &[]); + b.dependencies = vec!["aaaaaa".to_string()]; + let tickets = vec![a, b]; + let graph = TicketGraph::build(&tickets); + // format_subtree drives rendering from "aaaaaa"; when it tries to + // revisit "aaaaaa" via bbbbbb's dependent edge, it should hit [cycle]. + let out = format_subtree(&graph, "aaaaaa"); + assert!(out.contains("[cycle]"), "cycle should be labelled: {out}"); + } + + /// An empty graph renders as an empty string. + #[test] + fn empty_graph() { + let graph = TicketGraph::build(&[]); + assert!(format_graph(&graph).is_empty()); + } +} + // ── display module ──────────────────────────────────────────────────────────── /// Tests for [`crate::display`].