74b65fbe
Add inline comments on patches
a73x 2026-03-20 20:22
Closes 45da1336. Adds PatchInlineComment action with file path and line number. CLI: `patch comment <id> --file <path> --line <n> -b "text"`. If --file and --line are omitted, creates a regular comment. In the TUI diff view, inline comments are interleaved at the correct position with box-drawing decoration (magenta). The detail view shows them grouped under "Inline Comments" with file:line labels. Comments on files not in the diff are shown at the bottom. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
diff --git a/src/cli.rs b/src/cli.rs index 058896d..a676fc7 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -107,13 +107,19 @@ pub enum PatchCmd { /// Patch ID (prefix match) id: String, }, /// Comment on a patch /// Comment on a patch (use --file and --line for inline comments) Comment { /// Patch ID (prefix match) id: String, /// Comment body #[arg(short, long)] body: String, /// File path for inline comment #[arg(short, long)] file: Option<String>, /// Line number for inline comment #[arg(short, long)] line: Option<u32>, }, /// Review a patch Review { diff --git a/src/dag.rs b/src/dag.rs index 0255be7..2cc2507 100644 --- a/src/dag.rs +++ b/src/dag.rs @@ -137,6 +137,9 @@ fn commit_message(action: &Action) -> String { Action::PatchRevise { .. } => "patch: revise".to_string(), Action::PatchReview { verdict, .. } => format!("patch: review ({:?})", verdict), Action::PatchComment { .. } => "patch: comment".to_string(), Action::PatchInlineComment { ref file, line, .. } => { format!("patch: inline comment on {}:{}", file, line) } Action::PatchClose { .. } => "patch: close".to_string(), Action::PatchMerge => "patch: merge".to_string(), Action::Merge => "collab: merge".to_string(), diff --git a/src/event.rs b/src/event.rs index 26f3b31..c26d86b 100644 --- a/src/event.rs +++ b/src/event.rs @@ -44,6 +44,11 @@ pub enum Action { PatchComment { body: String, }, PatchInlineComment { file: String, line: u32, body: String, }, PatchClose { reason: Option<String>, }, diff --git a/src/main.rs b/src/main.rs index b372679..81f1568 100644 --- a/src/main.rs +++ b/src/main.rs @@ -54,7 +54,12 @@ fn run(cli: Cli) -> Result<(), error::Error> { } PatchCmd::List { all } => patch::list(&repo, all), PatchCmd::Show { id } => patch::show(&repo, &id), PatchCmd::Comment { id, body } => patch::comment(&repo, &id, &body), PatchCmd::Comment { id, body, file, line, } => patch::comment(&repo, &id, &body, file.as_deref(), line), PatchCmd::Review { id, verdict, body } => { let v = match verdict.as_str() { "approve" => ReviewVerdict::Approve, diff --git a/src/patch.rs b/src/patch.rs index 098e3e8..fca1add 100644 --- a/src/patch.rs +++ b/src/patch.rs @@ -82,6 +82,15 @@ pub fn show(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Erro ); } } if !p.inline_comments.is_empty() { println!("\n--- Inline Comments ---"); for c in &p.inline_comments { println!( "\n{} on {}:{} ({}):\n {}", c.author.name, c.file, c.line, c.timestamp, c.body ); } } if !p.comments.is_empty() { println!("\n--- Comments ---"); for c in &p.comments { @@ -91,15 +100,37 @@ pub fn show(repo: &Repository, id_prefix: &str) -> Result<(), crate::error::Erro Ok(()) } pub fn comment(repo: &Repository, id_prefix: &str, body: &str) -> Result<(), crate::error::Error> { pub fn comment( repo: &Repository, id_prefix: &str, body: &str, file: Option<&str>, line: Option<u32>, ) -> Result<(), crate::error::Error> { let (ref_name, _id) = state::resolve_patch_ref(repo, id_prefix)?; let author = get_author(repo)?; let action = match (file, line) { (Some(f), Some(l)) => Action::PatchInlineComment { file: f.to_string(), line: l, body: body.to_string(), }, (Some(_), None) | (None, Some(_)) => { return Err(git2::Error::from_str( "--file and --line must both be provided for inline comments", ) .into()); } (None, None) => Action::PatchComment { body: body.to_string(), }, }; let event = Event { timestamp: chrono::Utc::now().to_rfc3339(), author, action: Action::PatchComment { body: body.to_string(), }, action, }; dag::append_event(repo, &ref_name, &event)?; println!("Comment added."); diff --git a/src/state.rs b/src/state.rs index f1a374e..e2a02c0 100644 --- a/src/state.rs +++ b/src/state.rs @@ -47,6 +47,15 @@ pub enum PatchStatus { } #[derive(Debug, Clone)] pub struct InlineComment { pub author: Author, pub file: String, pub line: u32, pub body: String, pub timestamp: String, } #[derive(Debug, Clone)] pub struct PatchState { pub id: String, pub title: String, @@ -55,6 +64,7 @@ pub struct PatchState { pub base_ref: String, pub head_commit: String, pub comments: Vec<Comment>, pub inline_comments: Vec<InlineComment>, pub reviews: Vec<Review>, pub created_at: String, pub author: Author, @@ -151,6 +161,7 @@ impl PatchState { base_ref, head_commit, comments: Vec::new(), inline_comments: Vec::new(), reviews: Vec::new(), created_at: event.timestamp.clone(), author: event.author.clone(), @@ -184,6 +195,17 @@ impl PatchState { }); } } Action::PatchInlineComment { file, line, body } => { if let Some(ref mut s) = state { s.inline_comments.push(InlineComment { author: event.author.clone(), file, line, body, timestamp: event.timestamp.clone(), }); } } Action::PatchClose { .. } => { if let Some(ref mut s) = state { if status_ts.as_ref().is_none_or(|ts| event.timestamp >= *ts) { diff --git a/src/tui.rs b/src/tui.rs index 510defe..05d05dd 100644 --- a/src/tui.rs +++ b/src/tui.rs @@ -474,7 +474,7 @@ fn render_detail(frame: &mut Frame, app: &App, area: Rect) { .get(&patch.id) .map(|s| s.as_str()) .unwrap_or("Loading..."); colorize_diff(diff_text) colorize_diff(diff_text, &patch.inline_comments) } }, None => Text::raw("No patches to display."), @@ -647,6 +647,33 @@ fn build_patch_detail(patch: &PatchState) -> Text<'static> { } } if !patch.inline_comments.is_empty() { lines.push(Line::raw("")); lines.push(Line::styled( "--- Inline Comments ---", Style::default() .fg(Color::Magenta) .add_modifier(Modifier::BOLD), )); for c in &patch.inline_comments { lines.push(Line::raw("")); lines.push(Line::from(vec![ Span::styled( format!("{}:{}", c.file, c.line), Style::default().fg(Color::Cyan), ), Span::raw(format!(" - {} ", c.author.name)), Span::styled( format!("({})", c.timestamp), Style::default().fg(Color::DarkGray), ), ])); for l in c.body.lines() { lines.push(Line::raw(format!(" {}", l))); } } } if !patch.comments.is_empty() { lines.push(Line::raw("")); lines.push(Line::styled( @@ -676,26 +703,128 @@ fn build_patch_detail(patch: &PatchState) -> Text<'static> { Text::from(lines) } fn colorize_diff(diff: &str) -> Text<'static> { let lines: Vec<Line> = diff fn colorize_diff(diff: &str, inline_comments: &[state::InlineComment]) -> Text<'static> { // Build a lookup: (file, line) -> vec of comments let mut comment_map: HashMap<(String, u32), Vec<&state::InlineComment>> = HashMap::new(); for c in inline_comments { comment_map .entry((c.file.clone(), c.line)) .or_default() .push(c); } let mut lines: Vec<Line> = Vec::new(); let mut current_file = String::new(); let mut current_new_line: u32 = 0; for diff_line in diff.lines() { // Track which file we're in from +++ headers if let Some(file) = diff_line.strip_prefix("+++ b/") { current_file = file.to_string(); } else if diff_line.starts_with("+++ ") { // +++ /dev/null or similar current_file.clear(); } // Track line numbers from @@ hunk headers if diff_line.starts_with("@@") { // Parse @@ -old,len +new,len @@ if let Some(plus_part) = diff_line.split('+').nth(1) { if let Some(line_str) = plus_part.split(',').next().or(plus_part.split(' ').next()) { current_new_line = line_str.parse().unwrap_or(0); } } } // Colorize the diff line let style = if diff_line.starts_with('+') && !diff_line.starts_with("+++") { Style::default().fg(Color::Green) } else if diff_line.starts_with('-') && !diff_line.starts_with("---") { Style::default().fg(Color::Red) } else if diff_line.starts_with("@@") { Style::default().fg(Color::Cyan) } else if diff_line.starts_with("diff ") || diff_line.starts_with("index ") { Style::default().fg(Color::DarkGray) } else if diff_line.starts_with("+++") || diff_line.starts_with("---") { Style::default().fg(Color::Yellow) } else { Style::default() }; lines.push(Line::styled(diff_line.to_string(), style)); // After rendering this line, check for inline comments at current position if !current_file.is_empty() && current_new_line > 0 && (diff_line.starts_with('+') || diff_line.starts_with(' ') || diff_line.starts_with("@@")) { if let Some(comments) = comment_map.get(&(current_file.clone(), current_new_line)) { for c in comments { lines.push(Line::styled( format!(" ┌─ {} ({})", c.author.name, c.timestamp), Style::default() .fg(Color::Magenta) .add_modifier(Modifier::BOLD), )); for body_line in c.body.lines() { lines.push(Line::styled( format!(" │ {}", body_line), Style::default().fg(Color::Magenta), )); } lines.push(Line::styled( " └─".to_string(), Style::default().fg(Color::Magenta), )); } } } // Advance line counter for added/context lines (not removed or header) if (diff_line.starts_with('+') && !diff_line.starts_with("+++")) || diff_line.starts_with(' ') { current_new_line += 1; } } // Show any inline comments for files not covered by the diff let files_in_diff: std::collections::HashSet<&str> = diff .lines() .map(|line| { let style = if line.starts_with('+') && !line.starts_with("+++") { Style::default().fg(Color::Green) } else if line.starts_with('-') && !line.starts_with("---") { Style::default().fg(Color::Red) } else if line.starts_with("@@") { Style::default().fg(Color::Cyan) } else if line.starts_with("diff ") || line.starts_with("index ") { Style::default().fg(Color::DarkGray) } else if line.starts_with("+++") || line.starts_with("---") { Style::default().fg(Color::Yellow) } else { Style::default() }; Line::styled(line.to_string(), style) }) .filter_map(|l| l.strip_prefix("+++ b/")) .collect(); let mut orphan_comments: Vec<&state::InlineComment> = inline_comments .iter() .filter(|c| !files_in_diff.contains(c.file.as_str())) .collect(); if !orphan_comments.is_empty() { orphan_comments.sort_by(|a, b| (&a.file, a.line).cmp(&(&b.file, b.line))); lines.push(Line::raw("")); lines.push(Line::styled( "--- Comments on files not in diff ---", Style::default() .fg(Color::Magenta) .add_modifier(Modifier::BOLD), )); for c in orphan_comments { lines.push(Line::styled( format!( " {}:{} - {} ({}):", c.file, c.line, c.author.name, c.timestamp ), Style::default().fg(Color::Magenta), )); for body_line in c.body.lines() { lines.push(Line::styled( format!(" {}", body_line), Style::default().fg(Color::Magenta), )); } } } Text::from(lines) }