fix(comments): include replies to replies in the comment summary (#820) (#840)

The comment summary gathered a comment's direct replies only, matching /IRT against the comment itself. A reply to a reply points at the direct reply, so its text never reached the generated summary even though the stored discussion kept it.

Replies are now walked to any depth, with a bound, and indented one level per reply. The existing output for direct replies is unchanged.

The new test builds a rectangle, a reply and a reply to that reply, then checks the summary carries both texts with the nested one indented under its parent. It omitted the nested reply before the fix.

Co-authored-by: Big Pickle <[email protected]>
This commit is contained in:
muraa-pandBig Pickle authored and GitHub committed 2026-10-10 15:08:18 -04:00
1 parent c7fcee4ed9
commit 5ab928d529
2 files changed
+34 -8

No files matched your search

+21 -8
View File
@@ -3190,9 +3190,25 @@ pub fn comment_kind(a: &pdfcraft_render::Annotation) -> &str {
pub fn comment_summary(name: &str, all: &[pdfcraft_render::Annotation], sort: SummarySort) -> String {
use std::fmt::Write;
let top: Vec<&pdfcraft_render::Annotation> = all.iter().filter(|a| a.in_reply_to.is_none() && a.state.is_none()).collect();
let replies_of = |a: &pdfcraft_render::Annotation| -> Vec<&pdfcraft_render::Annotation> {
all.iter().filter(|r| r.state.is_none() && r.in_reply_to.is_some() && r.in_reply_to == a.name && a.name.is_some()).collect()
};
// Write a comment's replies, indenting each level. A reply to a reply keeps its `/IRT` on the
// direct parent, so following one level would leave the deeper replies out of the summary.
fn write_replies(out: &mut String, all: &[pdfcraft_render::Annotation], parent: &str, depth: usize) {
use std::fmt::Write;
if depth > 64 {
return;
}
for r in all.iter().filter(|r| r.state.is_none() && r.in_reply_to.as_deref() == Some(parent)) {
let indent = " ".repeat(depth + 1);
let _ =
writeln!(out, "{indent}Author: {} Subject: Reply Date: {}", r.author.as_deref().unwrap_or(""), r.modified.as_deref().unwrap_or(""));
for line in r.contents.as_deref().unwrap_or("").lines() {
let _ = writeln!(out, "{indent}{line}");
}
if let Some(nm) = r.name.as_deref() {
write_replies(out, all, nm, depth + 1);
}
}
}
let mut numbered: Vec<(usize, &pdfcraft_render::Annotation)> = Vec::new();
let mut last = usize::MAX;
let mut n = 0;
@@ -3229,11 +3245,8 @@ pub fn comment_summary(name: &str, all: &[pdfcraft_render::Annotation], sort: Su
if let Some(c) = a.contents.as_deref().filter(|c| !c.is_empty()) {
let _ = writeln!(out, "{c}");
}
for r in replies_of(a) {
let _ = writeln!(out, " Author: {} Subject: Reply Date: {}", r.author.as_deref().unwrap_or(""), r.modified.as_deref().unwrap_or(""));
for line in r.contents.as_deref().unwrap_or("").lines() {
let _ = writeln!(out, " {line}");
}
if let Some(nm) = a.name.as_deref() {
write_replies(&mut out, all, nm, 0);
}
out.push('\n');
}
+13
View File
@@ -1553,6 +1553,19 @@ fn comments_lock_take_checkmarks_hide_and_summarize() {
assert_eq!(comment_summary("x", &[], SummarySort::Page), "Summary of Comments on x\n\nThis document has no comments.\n");
}
/// #820: the generated summary includes replies to replies, indented one level deeper.
#[test]
fn comment_summary_includes_replies_to_replies() {
let (mut s, id) = session_with(1);
s.apply(id, rect_comment(0, [40.0, 40.0, 90.0, 90.0])).unwrap();
s.apply(id, Edit::ReplyToAnnotation { page: 0, index: 0, text: "FIRST_REPLY".into(), author: "A".into() }).unwrap();
s.apply(id, Edit::ReplyToAnnotation { page: 0, index: 1, text: "NESTED_REPLY".into(), author: "B".into() }).unwrap();
let text = comment_summary("doc.pdf", &s.get(id).unwrap().info.annotations, SummarySort::Page);
assert!(text.contains("FIRST_REPLY"), "{text}");
assert!(text.contains("NESTED_REPLY"), "replies to replies are summarized: {text}");
assert!(text.contains(" NESTED_REPLY"), "the nested reply is indented under its parent: {text}");
}
#[test]
fn signing_saving_trusting_and_commenting_afterwards() {
let p12 = std::fs::read(concat!(env!("CARGO_MANIFEST_DIR"), "/../sign/tests/data/ec-p256.p12")).unwrap();