From 7eac105c0005ba1e151795fb8031baed23b4751b Mon Sep 17 00:00:00 2001 From: Marco Arnulfo <103679419+marcoarnulfo@users.noreply.github.com> Date: Tue, 13 Oct 2026 06:36:44 +0200 Subject: [PATCH] fix(ui): Combine flags the same converted file added twice (#1303) (#1458) Follow-up to #1420. #1420 compares the bytes of each entry to tell a file added twice from a different file with the same name and size. An image or text file is converted to PDF each time it is added, with its own date, so those bytes never match and the same .txt or image added twice was no longer flagged. Each entry now keeps the file as it was added (the same bytes for a PDF, the image or text otherwise), and that is what is compared. Adds the_same_image_or_text_file_added_twice_is_a_duplicate in combine_ui.rs, which fails before this change. --- crates/ui-egui/src/combine_ui.rs | 31 +++++++++++++++++++++++++++++-- 1 file changed, 29 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/combine_ui.rs b/crates/ui-egui/src/combine_ui.rs index 22aa16ff..467a040b 100644 --- a/crates/ui-egui/src/combine_ui.rs +++ b/crates/ui-egui/src/combine_ui.rs @@ -402,6 +402,9 @@ pub struct CombineFile { pub(crate) group: u64, pub name: String, pub bytes: Arc>, + /// The file as it was added: `bytes` itself for a PDF, the image or text it was made from + /// otherwise (converted anew, with its own date, each time it is added). + source: Arc>, pub pages: usize, /// Each page's displayed size in points; shared, so undo snapshots don't copy it. pub(crate) sizes: Arc<[(f32, f32)]>, @@ -440,7 +443,7 @@ impl CombineTab { // unrelated files, and never count a source split into parts as several additions. let mut candidates: HashMap<(&str, usize), Vec> = HashMap::new(); for (i, f) in files.iter().enumerate() { - candidates.entry((&f.name, f.bytes.len())).or_default().push(i); + candidates.entry((&f.name, f.source.len())).or_default().push(i); } let mut twice = vec![false; files.len()]; for indices in candidates.values() { @@ -452,7 +455,7 @@ impl CombineTab { let mut contents: HashMap<&[u8], Vec> = HashMap::new(); for &i in indices { if let Some(f) = files.get(i) { - contents.entry(f.bytes.as_slice()).or_default().push(i); + contents.entry(f.source.as_slice()).or_default().push(i); } } for copies in contents.values() { @@ -2405,6 +2408,7 @@ impl PdfCraftApp { lock: None, notes: Vec::new(), }; + let source = bytes.clone(); let (bytes, assessed) = match self.session.as_pdf(&name, &bytes) { Ok(pdf) => { let assessed = assess(&pdf, None); @@ -2433,6 +2437,7 @@ impl PdfCraftApp { group: 0, name, bytes, + source, pages: 0, range: String::new(), modified, @@ -2646,6 +2651,28 @@ mod tests { assert!(app.combine_tab.duplicate_warnings(&app.combine_draft).is_empty()); } + /// #1303: an image or text file is converted anew, with its own date, each time it is added, + /// so it is the file as added that is compared, not the PDF made from it. + #[test] + fn the_same_image_or_text_file_added_twice_is_a_duplicate() { + static NOW: std::sync::atomic::AtomicI64 = std::sync::atomic::AtomicI64::new(1_700_000_000); + let mut png = std::io::Cursor::new(Vec::new()); + image::RgbaImage::from_pixel(12, 4, image::Rgba([20, 30, 40, 255])).write_to(&mut png, image::ImageFormat::Png).unwrap(); + let png = png.into_inner(); + let warnings = |first: (&str, &[u8]), second: (&str, &[u8])| { + let mut app = PdfCraftApp::new(); + app.session = std::mem::take(&mut app.session).with_clock(|| NOW.fetch_add(1, std::sync::atomic::Ordering::Relaxed)); + for (name, bytes) in [first, second] { + app.stage_combine(vec![(name.into(), bytes.to_vec())]); + } + assert_ne!(app.combine_draft[0].bytes, app.combine_draft[1].bytes, "each addition is converted with its own date"); + app.combine_tab.duplicate_warnings(&app.combine_draft).to_vec() + }; + assert_eq!(warnings(("notes.txt", b"Hello"), ("notes.txt", b"Hello")), [true, true], "the same text file"); + assert_eq!(warnings(("photo.png", &png), ("photo.png", &png)), [true, true], "the same image"); + assert_eq!(warnings(("notes.txt", b"Hello"), ("notes.txt", b"World")), [false, false], "different text, same name and size"); + } + #[test] fn every_read_of_a_file_is_new_even_after_undo() { // Regression: the read counter lived in the undo history, so unlock → undo → unlock (with