From 8486a35207655ec1261547cd334e25225ecd763e Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Fri, 2 Oct 2026 05:11:25 -0400 Subject: [PATCH] M4: Combine files lists the files first: reorder, remove, add more, and choose each file's pages (doc_combine pages) Co-Authored-By: Claude Opus 5.5 --- crates/automation/src/lib.rs | 11 ++- crates/automation/src/tools.rs | 12 ++- crates/automation/tests/automation.rs | 5 ++ crates/engine/src/lib.rs | 25 ++++++ crates/organize/src/import.rs | 20 ++++- crates/organize/src/lib.rs | 2 +- crates/ui-egui/src/combine_ui.rs | 120 ++++++++++++++++++++++++++ crates/ui-egui/src/dialogs.rs | 14 +++ crates/ui-egui/src/files.rs | 19 +--- crates/ui-egui/src/lib.rs | 6 ++ crates/ui-egui/tests/editing.rs | 24 ++++++ parity/acrobat-features.toml | 10 +-- 12 files changed, 239 insertions(+), 29 deletions(-) create mode 100644 crates/ui-egui/src/combine_ui.rs diff --git a/crates/automation/src/lib.rs b/crates/automation/src/lib.rs index c06397f4..cfe29187 100644 --- a/crates/automation/src/lib.rs +++ b/crates/automation/src/lib.rs @@ -894,14 +894,19 @@ impl Automation { if paths.len() < 2 { return Err(ToolError::InvalidArgs("combine needs at least two files".into())); } + let ranges: Vec> = match a.get("pages") { + None | Some(Value::Null) => vec![None; paths.len()], + Some(Value::Array(v)) if v.len() == paths.len() => v.iter().map(|x| x.as_str().map(str::to_owned)).collect(), + Some(_) => return Err(ToolError::InvalidArgs("pages must list a range (or null) for each path".into())), + }; let mut sources = Vec::new(); - for p in paths { + for (p, range) in paths.into_iter().zip(ranges) { let path = self.resolve(p, false)?; let bytes = std::fs::read(&path).map_err(|e| failed(format!("{}: {e}", path.display())))?; let name = path.file_stem().map(|n| n.to_string_lossy().into_owned()).unwrap_or_default(); - sources.push((name, Arc::new(bytes))); + sources.push((name, Arc::new(bytes), range)); } - let bytes = self.session.combine(&sources).map_err(failed)?; + let bytes = self.session.combine_ranges(&sources).map_err(failed)?; self.deliver(a, "Combined", bytes) } diff --git a/crates/automation/src/tools.rs b/crates/automation/src/tools.rs index b4c319ed..05816b5b 100644 --- a/crates/automation/src/tools.rs +++ b/crates/automation/src/tools.rs @@ -171,9 +171,17 @@ pub fn tools() -> Vec { json!({ "doc": doc(), "pages": pages("to extract"), "out": save_out.clone(), "open": open.clone(), "separate": { "type": "boolean" }, "out_dir": { "type": "string" }, "delete": { "type": "boolean" } }), &["doc", "pages"], )), - t("doc_combine", "Combine files", "Combine PDFs, in order, into one (bookmarks are kept under one entry per file).") + t("doc_combine", "Combine files", "Combine PDFs, in order, into one (bookmarks are kept under one entry per file). pages optionally chooses each file's pages, in step with paths: a range such as \"1-3, 6\" or null for all pages.") .cmd("page.combine") - .with(schema(json!({ "paths": { "type": "array", "items": { "type": "string" }, "minItems": 2 }, "out": save_out, "open": open }), &["paths"])), + .with(schema( + json!({ + "paths": { "type": "array", "items": { "type": "string" }, "minItems": 2 }, + "pages": { "type": "array", "items": { "type": ["string", "null"] } }, + "out": save_out, + "open": open, + }), + &["paths"], + )), t("doc_split", "Split a document", "Split into several files written to out_dir: every N pages, before given pages, at top-level bookmarks (bookmarks: true; files named after them), or by file size (max_mb). Files are -partK.pdf.") .cmd("page.split") .with(schema( diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index d7ed9933..c2db787a 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -164,6 +164,11 @@ fn combine_extract_and_split() { let doc = combined["document"]["doc"].as_u64().unwrap(); assert_eq!(combined["document"]["pages"], 5); assert_eq!(page_text(&mut a, doc), ["Page 1", "Page 2", "Page 3", "Page 1", "Page 2"]); + // Chosen pages per file, in the order given. + let some = ok(&mut a, "doc_combine", json!({ "paths": ["a.pdf", "b.pdf"], "pages": ["3, 1", null], "open": true })); + assert_eq!(page_text(&mut a, some["document"]["doc"].as_u64().unwrap()), ["Page 3", "Page 1", "Page 1", "Page 2"]); + assert!(matches!(a.call("doc_combine", &json!({ "paths": ["a.pdf", "b.pdf"], "pages": ["9", null] })), Err(ToolError::Failed(_)))); + assert!(matches!(a.call("doc_combine", &json!({ "paths": ["a.pdf", "b.pdf"], "pages": ["1"] })), Err(ToolError::InvalidArgs(_)))); let ex = ok(&mut a, "page_extract", json!({ "doc": doc, "pages": [2, 4] })); let ex_doc = ex["document"]["doc"].as_u64().unwrap(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index f7ab326e..8897aa49 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -330,6 +330,9 @@ fn render_threads() -> usize { std::thread::available_parallelism().map(|n| n.get()).unwrap_or(4).clamp(2, 8) - 1 } +/// A file to combine: its name (the bookmark title), bytes, and page range (`None`: all). +pub type CombineSource = (String, Arc>, Option); + /// A document's working file captured for crash recovery. #[derive(Clone, Debug)] pub struct RecoverySnapshot { @@ -1819,6 +1822,28 @@ impl Session { self.write_new(&out) } + /// Combine Files with a page range per file ("1-3, 6"; `None` or empty for all pages). + pub fn combine_ranges(&self, sources: &[CombineSource]) -> Result>, EditError> { + let docs = sources.iter().map(|(n, b, _)| open_source(n, b)).collect::, _>>()?; + let mut pages = Vec::with_capacity(docs.len()); + for ((name, _, range), d) in sources.iter().zip(&docs) { + let range = range.as_deref().map(str::trim).filter(|r| !r.is_empty()); + pages.push(match range { + Some(r) => { + let n = printcraft_organize::page_count(d)?; + let p = printcraft_print::select_pages(n, Some(r), &[], printcraft_print::Subset::All, false) + .map_err(|e| EditError::Print(format!("{name}: {e}")))?; + Some(p) + } + None => None, + }); + } + let named: Vec<(&str, &printcraft_cos::Document, Option<&[usize]>)> = + sources.iter().zip(docs.iter()).zip(&pages).map(|(((n, _, _), d), p)| (n.as_str(), d, p.as_deref())).collect(); + let out = printcraft_organize::combine_selected(&named)?; + self.write_new(&out) + } + /// New PDF bytes containing copies of `pages` of the document (Extract Pages). pub fn extract(&self, id: DocId, pages: &[usize]) -> Result>, EditError> { let src = self.cos(id)?; diff --git a/crates/organize/src/import.rs b/crates/organize/src/import.rs index c84761fa..79013ed9 100644 --- a/crates/organize/src/import.rs +++ b/crates/organize/src/import.rs @@ -462,13 +462,29 @@ pub fn split(src: &Document, by: &SplitBy) -> Result, OrganizeErro /// The source's own bookmarks are nested (collapsed) under its entry, and its document-level /// attachments are carried over. pub fn combine(sources: &[(&str, &Document)]) -> Result { + let all: Vec<(&str, &Document, Option<&[usize]>)> = sources.iter().map(|(t, d)| (*t, *d, None)).collect(); + combine_selected(&all) +} + +/// Combine Files with chosen pages: each source contributes `pages` (0-based, in that order; +/// `None` for all of them). Bookmarks that point at pages left out lose their destination. +pub fn combine_selected(sources: &[(&str, &Document, Option<&[usize]>)]) -> Result { let mut out = Document::new_empty(); let mut marks = Vec::new(); let mut attachments = Vec::new(); - for (title, src) in sources { + for (title, src, chosen) in sources { let n = crate::page_count(src)?; + let pages: Vec = match chosen { + Some(p) => { + if let Some(bad) = p.iter().find(|i| **i >= n) { + return Err(OrganizeError::NoSuchPage(*bad)); + } + p.to_vec() + } + None => (0..n).collect(), + }; let at = crate::page_count(&out)?; - let (new, page_map) = import_pages_mapped(&mut out, src, &(0..n).collect::>(), at)?; + let (new, page_map) = import_pages_mapped(&mut out, src, &pages, at)?; if let Some(first) = new.first() { marks.push((title.to_string(), *first, *src, page_map)); } diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index 41aeb4ec..137f50d6 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -20,7 +20,7 @@ pub mod view; pub use boxes::{BoxSpec, PageBox, page_boxes, set_page_box}; pub use dedupe::dedupe_resources; -pub use import::{SplitBy, combine, extract_pages, import_pages, page_as_form, split, split_ranges}; +pub use import::{SplitBy, combine, combine_selected, extract_pages, import_pages, page_as_form, split, split_ranges}; pub use labels::{LabelRange, LabelStyle, number_pages, page_label_ranges, page_labels, set_page_label_ranges}; pub use outline::{ Bookmark, OutlineError, add_bookmark, bookmarks, delete_bookmark, move_bookmark, rename_bookmark, set_bookmark_open, set_bookmark_page, diff --git a/crates/ui-egui/src/combine_ui.rs b/crates/ui-egui/src/combine_ui.rs new file mode 100644 index 00000000..3f168c78 --- /dev/null +++ b/crates/ui-egui/src/combine_ui.rs @@ -0,0 +1,120 @@ +//! Combine files: the files to combine, in order, each with the pages to take (all, or a range +//! such as "1-3, 6"); reorder, remove or add files, then Combine. + +use std::sync::Arc; + +use egui::{Align, Layout}; + +use crate::theme::{self, Tokens}; +use crate::{Dialog, PrintCraftApp, icons, widgets}; + +#[derive(Clone, Debug)] +pub struct CombineFile { + pub name: String, + pub bytes: Arc>, + pub pages: usize, + /// The pages to take ("" = all). + pub range: String, +} + +enum RowAction { + Up(usize), + Down(usize), + Remove(usize), +} + +pub(crate) fn body(ui: &mut egui::Ui, app: &mut PrintCraftApp, t: &Tokens) -> (bool, bool) { + ui.label(egui::RichText::new("Combine files").font(theme::semibold(18.0))); + ui.add_space(4.0); + ui.label(egui::RichText::new("Files are combined in this order. Leave Pages empty to take every page.").small().color(t.text_faint)); + ui.add_space(8.0); + let mut action = None; + let n = app.combine_draft.len(); + egui::ScrollArea::vertical().max_height(360.0).auto_shrink([false, true]).show(ui, |ui| { + egui::Grid::new("combine-files").num_columns(4).min_col_width(40.0).spacing([10.0, 6.0]).striped(true).show(ui, |ui| { + ui.label(egui::RichText::new("File").color(t.text_muted)); + ui.label(egui::RichText::new("Pages").color(t.text_muted)); + ui.label(""); + ui.label(""); + ui.end_row(); + for (i, f) in app.combine_draft.iter_mut().enumerate() { + ui.horizontal(|ui| { + ui.add(icons::image("file-text", 16.0, t.icon)); + ui.add(egui::Label::new(&f.name).truncate()); + ui.label(egui::RichText::new(format!("{} page{}", f.pages, if f.pages == 1 { "" } else { "s" })).small().color(t.text_faint)); + }); + ui.add_sized([120.0, 22.0], egui::TextEdit::singleline(&mut f.range).hint_text("All pages")) + .on_hover_text(format!("Pages of {} to combine, e.g. 1-3, 6", f.name)); + ui.horizontal(|ui| { + if ui.add_enabled_ui(i > 0, |ui| icons::button(ui, "chevron-up", 24.0, false, "Move up")).inner.clicked() { + action = Some(RowAction::Up(i)); + } + if ui.add_enabled_ui(i + 1 < n, |ui| icons::button(ui, "chevron-down", 24.0, false, "Move down")).inner.clicked() { + action = Some(RowAction::Down(i)); + } + }); + if icons::button(ui, "trash-2", 24.0, false, &format!("Remove {}", f.name)).clicked() { + action = Some(RowAction::Remove(i)); + } + ui.end_row(); + } + }); + }); + match action { + Some(RowAction::Up(i)) => app.combine_draft.swap(i, i - 1), + Some(RowAction::Down(i)) => app.combine_draft.swap(i, i + 1), + Some(RowAction::Remove(i)) => { + app.combine_draft.remove(i); + } + None => {} + } + ui.add_space(10.0); + let (mut go, mut cancel) = (false, false); + ui.horizontal(|ui| { + if widgets::pill_button(ui, "Add files…", false).clicked() { + app.combine_dialog(); + } + ui.with_layout(Layout::right_to_left(Align::Center), |ui| { + let ok = app.combine_draft.len() >= 2; + if ui.add_enabled_ui(ok, |ui| widgets::pill_button(ui, "Combine", true)).inner.on_disabled_hover_text("Add at least two files").clicked() + { + go = true; + } + if widgets::pill_button(ui, "Cancel", false).clicked() { + cancel = true; + } + }); + }); + (go, cancel) +} + +impl PrintCraftApp { + /// Add picked files to the Combine files list (and show it). + pub(crate) fn stage_combine(&mut self, files: Vec<(String, Vec)>) { + for (name, bytes) in files { + let bytes = Arc::new(bytes); + match printcraft_render::inspect(bytes.clone(), None) { + Ok(info) => self.combine_draft.push(CombineFile { name, bytes, pages: info.pages.len(), range: String::new() }), + Err(e) => self.notify(format!("Couldn't add {name}: {e}")), + } + } + self.dialog = Some(Dialog::Combine); + } + + /// Combine the listed files into a new, unsaved document. + pub fn combine_staged(&mut self) { + let files = std::mem::take(&mut self.combine_draft); + if files.is_empty() { + return; + } + let count = files.len(); + let sources: Vec<(String, Arc>, Option)> = files + .into_iter() + .map(|f| (crate::files::strip_pdf(&f.name).to_string(), f.bytes, Some(f.range).filter(|r| !r.trim().is_empty()))) + .collect(); + match self.session.combine_ranges(&sources) { + Ok(bytes) => self.open_created("Combined.pdf", bytes, &format!("Combined {count} files")), + Err(e) => self.notify(format!("Couldn't combine files: {e}")), + } + } +} diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 12bb837a..517fdcea 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -50,6 +50,7 @@ pub fn show(app: &mut PrintCraftApp, ctx: &egui::Context) { let mut replace_now = false; let mut open_revision: Option = None; let mut a11y_now = false; + let mut combine_now = false; let t = Tokens::get(ctx); let mut close = false; let mut next = dialog; @@ -757,6 +758,16 @@ pub fn show(app: &mut PrintCraftApp, ctx: &egui::Context) { close = apply || cancel; return; } + Dialog::Combine => { + ui.set_width(620.0); + let (go, cancel) = crate::combine_ui::body(ui, app, &t); + combine_now = go; + if cancel { + app.combine_draft.clear(); + } + close = go || cancel; + return; + } Dialog::AccessibilityOptions => { let (start, cancel) = crate::a11y_ui::options_body(ui, app, &t); a11y_now = start; @@ -1112,6 +1123,9 @@ pub fn show(app: &mut PrintCraftApp, ctx: &egui::Context) { if let Some(n) = open_revision { app.open_revision(n); } + if combine_now { + app.combine_staged(); + } if a11y_now { app.a11y_skipped.clear(); app.run_accessibility_check(); diff --git a/crates/ui-egui/src/files.rs b/crates/ui-egui/src/files.rs index f8981691..36c697d6 100644 --- a/crates/ui-egui/src/files.rs +++ b/crates/ui-egui/src/files.rs @@ -158,7 +158,7 @@ impl PrintCraftApp { /// Use picked files (also the entry point for tests and automation). pub fn use_files(&mut self, purpose: FilePurpose, files: Vec<(String, Vec)>) { match purpose { - FilePurpose::Combine => self.combine_files(files), + FilePurpose::Combine => self.stage_combine(files), FilePurpose::InsertPages => { for (name, bytes) in files { self.insert_pages_from(&name, bytes); @@ -172,19 +172,6 @@ impl PrintCraftApp { } } - /// Combine files, in order, into a new unsaved document tab. - pub fn combine_files(&mut self, files: Vec<(String, Vec)>) { - if files.is_empty() { - return; - } - let sources: Vec<(String, Arc>)> = files.into_iter().map(|(n, b)| (strip_pdf(&n).to_string(), Arc::new(b))).collect(); - let count = sources.len(); - match self.session.combine(&sources) { - Ok(bytes) => self.open_created("Combined.pdf", bytes, &format!("Combined {count} files")), - Err(e) => self.notify(format!("Couldn't combine files: {e}")), - } - } - pub fn replace_pages_dialog(&mut self) { if self.active.is_none() { self.notify("Open a document first"); @@ -352,7 +339,7 @@ impl PrintCraftApp { } } - fn open_created(&mut self, name: &str, bytes: Arc>, message: &str) { + pub(crate) fn open_created(&mut self, name: &str, bytes: Arc>, message: &str) { match self.session.open_new(name, bytes) { Ok(id) => { let info = &self.session.get(id).expect("just opened").info; @@ -365,7 +352,7 @@ impl PrintCraftApp { } } -fn strip_pdf(name: &str) -> &str { +pub(crate) fn strip_pdf(name: &str) -> &str { name.strip_suffix(".pdf").or_else(|| name.strip_suffix(".PDF")).unwrap_or(name) } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 52c2a5f0..aae9459e 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -8,6 +8,7 @@ mod a11y_ui; pub mod canvas; mod chrome; +mod combine_ui; mod commands; mod comment_props; pub mod comments; @@ -174,6 +175,8 @@ pub enum Dialog { DuplicateField, /// Check for accessibility ▸ Accessibility Checker Options. AccessibilityOptions, + /// Combine files: the files, their order and pages. + Combine, } #[derive(Clone, Copy, Debug, PartialEq, Eq)] @@ -276,6 +279,8 @@ pub struct PrintCraftApp { pub a11y_options: a11y_ui::A11yOptions, pub a11y: a11y_ui::A11yState, pub a11y_skipped: std::collections::BTreeSet, + /// Combine files: the files staged so far. + pub combine_draft: Vec, /// PDF Optimizer choices. pub optimize_draft: OptimizeDraft, /// Pages copied or cut in Organize Pages, ready to paste (into any document). @@ -398,6 +403,7 @@ impl PrintCraftApp { a11y_options: a11y_ui::A11yOptions::default(), a11y: a11y_ui::A11yState::default(), a11y_skipped: Default::default(), + combine_draft: Vec::new(), optimize_draft: OptimizeDraft::default(), page_clipboard: None, last_snapshot: None, diff --git a/crates/ui-egui/tests/editing.rs b/crates/ui-egui/tests/editing.rs index 490606e3..eabe5a27 100644 --- a/crates/ui-egui/tests/editing.rs +++ b/crates/ui-egui/tests/editing.rs @@ -321,6 +321,8 @@ fn combining_files_opens_a_new_unsaved_tab() { app.use_files(printcraft_ui_egui::FilePurpose::Combine, vec![("one.pdf".into(), fixture(2)), ("two.pdf".into(), fixture(1))]); }); h.run_steps(3); + h.get_by_label("Combine").click(); + h.run_steps(3); let app = h.state(); assert_eq!(app.views.len(), 2); assert_eq!(app.active, Some(1)); @@ -332,6 +334,28 @@ fn combining_files_opens_a_new_unsaved_tab() { h.get_by_label("Combined.pdf (edited)"); } +#[test] +fn combine_files_takes_chosen_pages_in_the_order_listed() { + let mut h = harness(1, |app| { + app.use_files(printcraft_ui_egui::FilePurpose::Combine, vec![("one.pdf".into(), fixture(3)), ("two.pdf".into(), fixture(2))]); + }); + h.run_steps(3); + h.get_by_label_contains("Files are combined in this order"); + h.get_by_label("3 pages"); + // two.pdf first; one.pdf's pages 3 and 1. + h.get_all_by_label("Move up").last().unwrap().click(); + h.run_steps(2); + h.state_mut().combine_draft[1].range = "3, 1".into(); + h.run_steps(1); + h.get_by_label("Combine").click(); + h.run_steps(3); + let app = h.state(); + assert_eq!(texts_of(app, 1), ["Page 1", "Page 2", "Page 3", "Page 1"]); + let doc = app.session.get(app.views[1].id).unwrap(); + assert_eq!(doc.info.outline.iter().map(|o| o.title.as_str()).collect::>(), ["two", "one"]); + assert!(app.combine_draft.is_empty()); +} + #[test] fn extract_button_copies_selected_pages_to_a_new_tab() { let mut h = organize(3); diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 3b035c70..33892496 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -2887,11 +2887,11 @@ title = "Choose which pages of each file to combine" area = "D" tier = "P1" milestone = "M4" -status = "planned" -commands = [] -tools = [] -evidence = [] -notes = "" +status = "shipped" +commands = ["page.combine"] +tools = ["doc_combine"] +evidence = ["crates/ui-egui/tests/editing.rs::combine_files_takes_chosen_pages_in_the_order_listed", "crates/automation/tests/automation.rs::combine_extract_and_split"] +notes = "Combine files list: order (move up/down), remove, add files, a page range per file (such as 1-3, 6; in the order given); bookmarks to left-out pages lose their destination" [[feature]] id = "organize.combine-sort"