diff --git a/crates/ui-egui/src/combine_grid.rs b/crates/ui-egui/src/combine_grid.rs index 7fadf78cb..e0336d969 100644 --- a/crates/ui-egui/src/combine_grid.rs +++ b/crates/ui-egui/src/combine_grid.rs @@ -557,7 +557,14 @@ fn file_gap(cells: &[Cell], at: usize, files: usize) -> usize { } /// The grid of files, in the card that holds the list. Returns what the user did. -pub(crate) fn grid(app: &mut PdfCraftApp, ui: &mut egui::Ui, t: &Tokens, checks: &[Result], selected: &[bool]) -> Option { +pub(crate) fn grid( + app: &mut PdfCraftApp, + ui: &mut egui::Ui, + t: &Tokens, + checks: &[Result], + selected: &[bool], + twice: &[bool], +) -> Option { let ctx = ui.ctx().clone(); let ppp = ctx.pixels_per_point(); let mut action = None; @@ -566,13 +573,6 @@ pub(crate) fn grid(app: &mut PdfCraftApp, ui: &mut egui::Ui, t: &Tokens, checks: // The card dragged may scroll out of the rows drawn, so its release is watched here. let released = drag.is_some() && ctx.input(|i| i.pointer.any_released()); let n = app.combine_draft.len(); - // The same file twice is allowed (e.g. a cover sheet), but probably a mistake. - // (A file's parts are the one file, not copies: counted by file.) - let mut copies: HashMap<(&str, usize), std::collections::BTreeSet> = HashMap::new(); - for f in &app.combine_draft { - copies.entry((f.name.as_str(), f.bytes.len())).or_default().insert(f.group); - } - let twice: Vec = app.combine_draft.iter().map(|f| copies.get(&(f.name.as_str(), f.bytes.len())).is_some_and(|g| g.len() > 1)).collect(); let places = layout(app); // Each file's first place (for the keys and to scroll to it). let mut first_place = vec![0usize; n]; diff --git a/crates/ui-egui/src/combine_ui.rs b/crates/ui-egui/src/combine_ui.rs index f7dc21835..2d2bb173a 100644 --- a/crates/ui-egui/src/combine_ui.rs +++ b/crates/ui-egui/src/combine_ui.rs @@ -180,6 +180,8 @@ pub struct CombineTab { unlock: Option, /// Bumped on every change to the list, so a grid drag that began before one is dropped. pub(crate) revision: u64, + /// Exact-content duplicate warnings, shared by both views until the list changes. + duplicates: Option<(u64, Arc<[bool]>)>, /// The file the keyboard moved to, for the grid to scroll into view. pub(crate) reveal: Option, /// Files per grid row as last drawn (Up/Down move a row in the grid). @@ -423,6 +425,49 @@ pub struct CombineFile { checked: Option<(String, RangeCheck)>, } +impl CombineTab { + fn duplicate_warnings(&mut self, files: &[CombineFile]) -> Arc<[bool]> { + if let Some((revision, flags)) = &self.duplicates + && *revision == self.revision + { + return flags.clone(); + } + // Only same-named, same-sized files can be repeats. Avoid reading the bytes of + // 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); + } + let mut twice = vec![false; files.len()]; + for indices in candidates.values() { + let Some(first) = indices.first().and_then(|i| files.get(*i)) else { continue }; + if indices.iter().all(|i| files.get(*i).is_some_and(|f| f.group == first.group)) { + continue; + } + // Slice keys check exact bytes even when their hashes collide. + 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); + } + } + for copies in contents.values() { + let Some(first) = copies.first().and_then(|i| files.get(*i)) else { continue }; + if copies.iter().any(|i| files.get(*i).is_some_and(|f| f.group != first.group)) { + for &i in copies { + if let Some(flag) = twice.get_mut(i) { + *flag = true; + } + } + } + } + } + let flags: Arc<[bool]> = twice.into(); + self.duplicates = Some((self.revision, flags.clone())); + flags + } +} + impl CombineFile { fn take(&mut self, a: Assessed, password: Option) { self.pages = a.pages; @@ -591,6 +636,7 @@ pub(crate) fn page(app: &mut PdfCraftApp, ui: &mut egui::Ui) { // list, so the Warnings column can say so before anything is combined. app.combine_conflicts.refresh(&app.session, &app.combine_draft); let conflicts = app.combine_conflicts.per_file.clone(); + let twice = app.combine_tab.duplicate_warnings(&app.combine_draft); let n = app.combine_draft.len(); let ids: Vec = app.combine_draft.iter().map(|f| f.id).collect(); app.combine_tab.selected.retain(|id| ids.contains(id)); @@ -778,11 +824,11 @@ pub(crate) fn page(app: &mut PdfCraftApp, ui: &mut egui::Ui) { // The view as the toolbar left it: a click on List view this frame starts no // thumbnail renders. } else if app.combine_view == CombineView::Grid { - if let Some(action) = crate::combine_grid::grid(app, ui, &t, &checks, &selected) { + if let Some(action) = crate::combine_grid::grid(app, ui, &t, &checks, &selected, &twice) { events.action = Some(action); } } else { - let table_events = table(app, ui, &t, &checks, &selected, &conflicts); + let table_events = table(app, ui, &t, &checks, &selected, &conflicts, &twice); events.range_focused |= table_events.range_focused; events.range_changed |= table_events.range_changed; if table_events.action.is_some() { @@ -1226,15 +1272,9 @@ fn table( checks: &[Result], selected: &[bool], conflicts: &[Vec<(String, String)>], + twice: &[bool], ) -> TableEvents { let mut events = TableEvents::default(); - // The same file twice is allowed (e.g. a cover sheet), but probably a mistake. - // (A file's parts are the one file, not copies.) - let twice: Vec = app - .combine_draft - .iter() - .map(|f| app.combine_draft.iter().filter(|g| g.group != f.group && g.name == f.name && g.bytes.len() == f.bytes.len()).count() > 0) - .collect(); let dragging = egui::DragAndDrop::has_payload_of_type::(ui.ctx()); // Until the user resizes a column, the widths follow the window. let gap = ui.spacing().item_spacing.x; @@ -1244,7 +1284,7 @@ fn table( app.combine_tab.layout = app.combine_tab.layout.wrapping_add(1); } let widths = app.combine_columns.resolved(available); - let rows = Rows { checks, selected, twice: &twice, conflicts, dragging, sort: app.combine_tab.sort }; + let rows = Rows { checks, selected, twice, conflicts, dragging, sort: app.combine_tab.sort }; let mut heading_event = None; let mut shown = None; egui::ScrollArea::horizontal() @@ -2573,6 +2613,29 @@ mod tests { pdfcraft_cos::write_full(&doc, &Default::default()).unwrap() } + #[test] + fn duplicate_warnings_reuse_results_and_track_removal_and_parts() { + let bytes = locked(); + let mut app = PdfCraftApp::new(); + app.stage_combine(vec![("same.pdf".into(), bytes.clone())]); + let mut part = app.combine_draft[0].clone(); + part.id = 99; + app.combine_draft.push(part); + let first = app.combine_tab.duplicate_warnings(&app.combine_draft); + assert_eq!(&*first, &[false, false], "parts of one source are not duplicate additions"); + let again = app.combine_tab.duplicate_warnings(&app.combine_draft); + assert!(Arc::ptr_eq(&first, &again), "unchanged frames reuse the result without scanning files"); + app.stage_combine(vec![("same.pdf".into(), bytes)]); + assert_eq!(&*app.combine_tab.duplicate_warnings(&app.combine_draft), &[true, true, true]); + let duplicate = app.combine_draft.last().unwrap().id; + app.combine_apply(RowAction::RemoveFile(duplicate)); + assert_eq!(&*app.combine_tab.duplicate_warnings(&app.combine_draft), &[false, false]); + app.combine_history_step(true); + assert_eq!(&*app.combine_tab.duplicate_warnings(&app.combine_draft), &[true, true, true]); + app.close_combine_tab(); + assert!(app.combine_tab.duplicate_warnings(&app.combine_draft).is_empty()); + } + #[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 diff --git a/crates/ui-egui/tests/combine_grid.rs b/crates/ui-egui/tests/combine_grid.rs index 8e7ab6bc4..e0d2668b8 100644 --- a/crates/ui-egui/tests/combine_grid.rs +++ b/crates/ui-egui/tests/combine_grid.rs @@ -126,6 +126,44 @@ fn names(h: &Harness<'static, PdfCraftApp>) -> Vec { h.state().combine_draft.iter().map(|f| f.name.clone()).collect() } +fn check_duplicate_warnings(view: &str) { + let first = fixture_sized(1, 200.0, 300.0); + let other = fixture_sized(1, 300.0, 200.0); + assert_eq!(first.len(), other.len()); + assert_ne!(first, other); + let mut h = grid_of(vec![("report.pdf", first.clone()), ("report.pdf", other)], vec2(1400.0, 900.0)); + h.state_mut().set_option("combine-view", view).unwrap(); + h.run_steps(2); + let warning_count = |h: &mut Harness<'static, PdfCraftApp>| { + if view == "grid" { + let at = h.get_all_by_label("report.pdf").next().unwrap().rect().center(); + h.hover_at(at); + h.run_steps(60); + } + h.query_all_by_label_contains("Added more than once").count() + }; + assert_eq!(warning_count(&mut h), 0, "different equal-sized PDFs are not duplicates in {view}"); + h.state_mut().use_files(FilePurpose::Combine, vec![("report.pdf".into(), first)]); + h.run_steps(3); + assert_eq!(warning_count(&mut h), if view == "grid" { 1 } else { 2 }, "only the two identical files are warned"); + h.key_press_modifiers(Modifiers::COMMAND, Key::Z); + h.run_steps(3); + assert_eq!(warning_count(&mut h), 0, "undo removes the duplicate warning"); + h.key_press_modifiers(Modifiers::COMMAND | Modifiers::SHIFT, Key::Z); + h.run_steps(3); + assert_eq!(warning_count(&mut h), if view == "grid" { 1 } else { 2 }, "redo restores the duplicate warning"); +} + +#[test] +fn list_duplicate_warnings_compare_contents_and_follow_undo() { + check_duplicate_warnings("list"); +} + +#[test] +fn grid_duplicate_warnings_compare_contents_and_follow_undo() { + check_duplicate_warnings("grid"); +} + /// Run frames until `done`, giving render threads time; whether it happened. fn settle(h: &mut Harness<'static, PdfCraftApp>, done: impl Fn(&PdfCraftApp) -> bool) -> bool { for _ in 0..400 {