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.
This commit is contained in:
Marco Arnulfo authored and GitHub committed 2026-10-13 00:36:44 -04:00
1 parent 929ae1155b
commit 7eac105c00
1 file changed
+29 -2
+29 -2
View File
@@ -402,6 +402,9 @@ pub struct CombineFile {
pub(crate) group: u64,
pub name: String,
pub bytes: Arc<Vec<u8>>,
/// 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<Vec<u8>>,
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<usize>> = 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<usize>> = 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