feat(nbd): show diff output on nbd update without --json [5f1495]

When `nbd update` is run without `--json`, print a git-diff-style
+/- summary of changed fields instead of the full ticket.

- Add `format_diff` and `print_diff` to display.rs
- Capture `old` ticket snapshot in `cmd_update` before mutations
- Use `print_diff` for the non-JSON branch of `cmd_update`
- Add unit tests (4) and integration tests (3) covering diff output,
  JSON fallback, and the no-changes case

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
quotesdb
Elijah Voigt 3 months ago
parent 6358579025
commit 7aa547633f

@ -3,7 +3,9 @@
"allow": [ "allow": [
"Bash(cargo:*)", "Bash(cargo:*)",
"Bash(git:*)", "Bash(git:*)",
"Bash(ls:*)" "Bash(ls:*)",
"Edit",
"Write"
] ]
} }
} }

@ -1,7 +1,7 @@
+++ +++
title = "nbd update diff output" title = "nbd update diff output"
priority = 5 priority = 5
status = "todo" status = "done"
ticket_type = "feature" ticket_type = "feature"
dependencies = [] dependencies = []
+++ +++

@ -266,6 +266,69 @@ pub fn print_migrate_report_json(report: &MigrateReport) {
println!("{}", format_migrate_report_json(report)); println!("{}", format_migrate_report_json(report));
} }
/// Format a git-diff-style `- old / + new` summary of what changed between
/// two [`Ticket`] snapshots.
///
/// Each field that differs produces two lines:
///
/// ```text
/// - status: todo
/// + status: in_progress
/// ```
///
/// Fields compared: `title`, `body`, `priority`, `status`, `ticket_type`, and
/// `dependencies`. The `id` field is never shown because it cannot change.
///
/// Returns `"(no changes)"` when every compared field is identical.
pub fn format_diff(old: &Ticket, new: &Ticket) -> String {
let mut lines: Vec<String> = Vec::new();
macro_rules! diff_field {
($label:expr, $old_val:expr, $new_val:expr) => {{
let old_s: String = $old_val;
let new_s: String = $new_val;
if old_s != new_s {
lines.push(format!("- {:<LABEL_WIDTH$}{}", $label, old_s));
lines.push(format!("+ {:<LABEL_WIDTH$}{}", $label, new_s));
}
}};
}
diff_field!("title:", old.title.clone(), new.title.clone());
diff_field!("body:", old.body.clone(), new.body.clone());
diff_field!(
"priority:",
old.priority.to_string(),
new.priority.to_string()
);
diff_field!(
"status:",
status_str(&old.status).to_string(),
status_str(&new.status).to_string()
);
diff_field!(
"type:",
ticket_type_str(&old.ticket_type).to_string(),
ticket_type_str(&new.ticket_type).to_string()
);
diff_field!(
"dependencies:",
old.dependencies.join(", "),
new.dependencies.join(", ")
);
if lines.is_empty() {
"(no changes)".to_string()
} else {
lines.join("\n")
}
}
/// Print a git-diff-style summary of what changed between two tickets to stdout.
pub fn print_diff(old: &Ticket, new: &Ticket) {
println!("{}", format_diff(old, new));
}
// ── Graph rendering ─────────────────────────────────────────────────────────── // ── Graph rendering ───────────────────────────────────────────────────────────
/// Format the full dependency forest as an ASCII tree string. /// Format the full dependency forest as an ASCII tree string.

@ -778,6 +778,7 @@ async fn cmd_update(
let old_format = detect_format(&existing_path); let old_format = detect_format(&existing_path);
let mut ticket = read_ticket(&root, &id).await?; let mut ticket = read_ticket(&root, &id).await?;
let old = ticket.clone();
if let Some(t) = title { if let Some(t) = title {
ticket.title = t; ticket.title = t;
@ -817,7 +818,7 @@ async fn cmd_update(
if json { if json {
display::print_ticket_json(&ticket); display::print_ticket_json(&ticket);
} else { } else {
display::print_ticket(&ticket); display::print_diff(&old, &ticket);
} }
Ok(()) Ok(())

@ -1318,8 +1318,8 @@ mod display_graph {
/// Tests for [`crate::display`]. /// Tests for [`crate::display`].
mod display { mod display {
use crate::display::{ use crate::display::{
format_list, format_list_json, format_migrate_report, format_migrate_report_json, format_diff, format_list, format_list_json, format_migrate_report,
format_ticket, format_ticket_json, format_migrate_report_json, format_ticket, format_ticket_json,
}; };
use crate::store::MigrateReport; use crate::store::MigrateReport;
use crate::ticket::{Status, Ticket, TicketType}; use crate::ticket::{Status, Ticket, TicketType};
@ -1572,4 +1572,84 @@ mod display {
assert_eq!(parsed["errors"][0]["filename"], "err.json"); assert_eq!(parsed["errors"][0]["filename"], "err.json");
assert_eq!(parsed["errors"][0]["message"], "bad"); assert_eq!(parsed["errors"][0]["message"], "bad");
} }
// ── format_diff ───────────────────────────────────────────────────────────
/// `format_diff` returns `"(no changes)"` when old and new are identical.
#[test]
fn format_diff_no_changes() {
let t = sample_ticket();
let output = format_diff(&t, &t);
assert_eq!(output, "(no changes)");
}
/// `format_diff` shows `- old` and `+ new` lines only for changed fields.
#[test]
fn format_diff_shows_changed_fields_only() {
let old = sample_ticket(); // status: in_progress
let mut new = sample_ticket();
new.status = Status::Done;
let output = format_diff(&old, &new);
assert!(
output.contains("- status:"),
"should show old status line: {output}"
);
assert!(
output.contains("+ status:"),
"should show new status line: {output}"
);
assert!(
output.contains("in_progress"),
"should show old status value: {output}"
);
assert!(
output.contains("done"),
"should show new status value: {output}"
);
// Unchanged fields must not appear.
assert!(
!output.contains("title:"),
"unchanged title should not appear: {output}"
);
assert!(
!output.contains("priority:"),
"unchanged priority should not appear: {output}"
);
}
/// `format_diff` shows multiple changed fields when several differ.
#[test]
fn format_diff_multiple_changed_fields() {
let old = sample_ticket();
let mut new = sample_ticket();
new.status = Status::Done;
new.priority = 3;
let output = format_diff(&old, &new);
assert!(output.contains("status:"), "status diff should appear");
assert!(output.contains("priority:"), "priority diff should appear");
// title unchanged — must not appear.
assert!(
!output.contains("title:"),
"unchanged title should not appear: {output}"
);
}
/// `format_diff` renders dependency changes as comma-separated lists.
#[test]
fn format_diff_dependencies_comma_separated() {
let mut old = sample_ticket(); // deps: b7d41e, c9e823
let mut new = sample_ticket();
old.dependencies = vec!["aaaaaa".to_string()];
new.dependencies = vec!["bbbbbb".to_string(), "cccccc".to_string()];
let output = format_diff(&old, &new);
assert!(
output.contains("dependencies:"),
"dependencies diff should appear: {output}"
);
assert!(output.contains("aaaaaa"), "old dep should appear: {output}");
assert!(
output.contains("bbbbbb, cccccc"),
"new deps should be comma-separated: {output}"
);
}
} }

@ -1696,7 +1696,10 @@ fn graph_chain() {
// B's line should use `└──`. // B's line should use `└──`.
let line_b = stdout.lines().find(|l| l.contains(&id_b)).unwrap(); let line_b = stdout.lines().find(|l| l.contains(&id_b)).unwrap();
assert!(line_b.contains("└──"), "child should use └── connector: {line_b}"); assert!(
line_b.contains("└──"),
"child should use └── connector: {line_b}"
);
} }
/// `nbd graph <id>` for ticket A shows A and its dependents but not unrelated /// `nbd graph <id>` for ticket A shows A and its dependents but not unrelated
@ -1714,7 +1717,10 @@ fn graph_single_ticket_subtree() {
assert!(stdout.contains(&id_a), "root should be in subtree"); assert!(stdout.contains(&id_a), "root should be in subtree");
assert!(stdout.contains(&id_b), "dependent should be in subtree"); assert!(stdout.contains(&id_b), "dependent should be in subtree");
assert!(!stdout.contains(&id_c), "unrelated ticket should be absent: {stdout}"); assert!(
!stdout.contains(&id_c),
"unrelated ticket should be absent: {stdout}"
);
} }
/// `nbd graph --json` produces valid JSON with `nodes` and `edges` arrays. /// `nbd graph --json` produces valid JSON with `nodes` and `edges` arrays.
@ -1730,12 +1736,22 @@ fn graph_json_output() {
let parsed: serde_json::Value = let parsed: serde_json::Value =
serde_json::from_str(&stdout).expect("--json should produce valid JSON"); serde_json::from_str(&stdout).expect("--json should produce valid JSON");
let nodes = parsed["nodes"].as_array().expect("nodes should be an array"); let nodes = parsed["nodes"]
let edges = parsed["edges"].as_array().expect("edges should be an array"); .as_array()
.expect("nodes should be an array");
let edges = parsed["edges"]
.as_array()
.expect("edges should be an array");
let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect(); let node_ids: Vec<&str> = nodes.iter().map(|n| n["id"].as_str().unwrap()).collect();
assert!(node_ids.contains(&id_a.as_str()), "nodes should include id_a"); assert!(
assert!(node_ids.contains(&id_b.as_str()), "nodes should include id_b"); node_ids.contains(&id_a.as_str()),
"nodes should include id_a"
);
assert!(
node_ids.contains(&id_b.as_str()),
"nodes should include id_b"
);
assert_eq!(edges.len(), 1, "should have exactly one edge (A blocks B)"); assert_eq!(edges.len(), 1, "should have exactly one edge (A blocks B)");
assert_eq!(edges[0]["from"].as_str().unwrap(), id_a); assert_eq!(edges[0]["from"].as_str().unwrap(), id_a);
assert_eq!(edges[0]["to"].as_str().unwrap(), id_b); assert_eq!(edges[0]["to"].as_str().unwrap(), id_b);
@ -1776,7 +1792,10 @@ fn graph_filter() {
let stdout = String::from_utf8(output.stdout).unwrap(); let stdout = String::from_utf8(output.stdout).unwrap();
assert!(stdout.contains(&id_bug), "bug ticket should be visible"); assert!(stdout.contains(&id_bug), "bug ticket should be visible");
assert!(!stdout.contains(&id_task), "task ticket should be excluded: {stdout}"); assert!(
!stdout.contains(&id_task),
"task ticket should be excluded: {stdout}"
);
} }
/// `nbd graph <3-char-prefix>` resolves to the correct ticket using prefix /// `nbd graph <3-char-prefix>` resolves to the correct ticket using prefix
@ -1794,5 +1813,80 @@ fn graph_partial_id() {
String::from_utf8_lossy(&output.stderr) String::from_utf8_lossy(&output.stderr)
); );
let stdout = String::from_utf8(output.stdout).unwrap(); let stdout = String::from_utf8(output.stdout).unwrap();
assert!(stdout.contains(&id_a), "resolved ticket should appear in output"); assert!(
stdout.contains(&id_a),
"resolved ticket should appear in output"
);
}
// ── update diff output tests ──────────────────────────────────────────────────
/// `nbd update <id> --status in_progress` (no `--json`) prints `- status:` and
/// `+ status:` lines showing what changed.
#[test]
fn update_no_json_prints_diff() {
let env = TestEnv::new();
let id = env.create(&["--title", "Diff test"]);
let output = env.run(&["update", &id, "--status", "in_progress"]);
assert!(
output.status.success(),
"update failed: {}",
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8(output.stdout).unwrap();
assert!(
stdout.contains("- status:"),
"should show old status line: {stdout}"
);
assert!(
stdout.contains("+ status:"),
"should show new status line: {stdout}"
);
assert!(stdout.contains("todo"), "should show old value: {stdout}");
assert!(
stdout.contains("in_progress"),
"should show new value: {stdout}"
);
}
/// `nbd update <id> --json` still prints full JSON (no diff).
#[test]
fn update_with_json_flag_prints_full_ticket() {
let env = TestEnv::new();
let id = env.create(&["--title", "JSON update test"]);
let output = env.run(&["update", &id, "--status", "in_progress", "--json"]);
assert!(
output.status.success(),
"update --json failed: {}",
String::from_utf8_lossy(&output.stderr)
);
let stdout = String::from_utf8(output.stdout).unwrap();
// Must be valid JSON containing the updated ticket.
let parsed: serde_json::Value =
serde_json::from_str(&stdout).expect("--json output should be valid JSON");
assert_eq!(parsed["status"], "in_progress");
assert_eq!(parsed["title"], "JSON update test");
// Should NOT contain diff markers.
assert!(
!stdout.contains("- status:"),
"JSON output should not contain diff markers: {stdout}"
);
}
/// `nbd update` with no changes prints `(no changes)`.
#[test]
fn update_no_changes_prints_no_changes() {
let env = TestEnv::new();
let id = env.create(&["--title", "Unchanged"]);
// Update with no field changes (supply same status).
let output = env.run(&["update", &id, "--status", "todo"]);
assert!(output.status.success());
let stdout = String::from_utf8(output.stdout).unwrap();
assert!(
stdout.contains("(no changes)"),
"should print '(no changes)' when nothing changed: {stdout}"
);
} }

Loading…
Cancel
Save