Merge PR #3750: feat(organize): Extract pages takes typed page ranges (#3593)

crates/ui-egui/src/dialogs.rs

@@ -594,15 +594,31 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) {
                 return;
             }
             Dialog::Extract => {
-                let count = app.active_ids().map_or(0, |(i, _)| app.views[i].target_pages().len());
+                let (selected, total) = app.active_ids().map_or((Vec::new(), 0), |(i, _)| (app.views[i].target_pages(), app.views[i].page_count()));
                 ui.label(egui::RichText::new(tl!("Extract pages")).font(theme::semibold(18.0)));
                 ui.add_space(8.0);
-                let text = if count == 1 {
-                    tl!("1 page selected.").to_string()
-                } else {
-                    crate::i18n::fmt(tl!("{n} pages selected."), &[("n", &count.to_string())])
+                // Page ranges to take instead of the selection (#3593); the hint is the selection.
+                let hint = match crate::combine_ui::range_of(&selected, total) {
+                    all if all.is_empty() && total > 1 => format!("1-{total}"),
+                    all if all.is_empty() => "1".to_string(),
+                    some => some,
                 };
-                ui.label(text);
+                ui.horizontal(|ui| {
+                    let l = ui.label(tl!("Pages"));
+                    ui.add(egui::TextEdit::singleline(&mut app.extract_draft.pages).hint_text(hint).desired_width(240.0)).labelled_by(l.id);
+                });
+                let chosen = app.extract_draft.pages_to_extract(selected, total);
+                match &chosen {
+                    Ok(pages) if pages.len() == 1 => {
+                        ui.label(tl!("1 page selected."));
+                    }
+                    Ok(pages) => {
+                        ui.label(crate::i18n::fmt(tl!("{n} pages selected."), &[("n", &pages.len().to_string())]));
+                    }
+                    Err(e) => {
+                        ui.colored_label(ui.visuals().error_fg_color, e);
+                    }
+                }
                 ui.add_space(4.0);
                 // The name the pages are saved under (#737); separate files add " (page N)".
                 ui.horizontal(|ui| {
@@ -613,7 +629,7 @@ fn dialogs(app: &mut PdfCraftApp, ctx: &egui::Context) {
                 ui.add(widgets::checkbox(&mut app.extract_draft.separate, tl!("Extract pages as separate files")));
                 ui.add_space(12.0);
                 ui.with_layout(Layout::right_to_left(Align::Center), |ui| {
-                    if widgets::pill_button(ui, tl!("Extract"), true).clicked() {
+                    if ui.add_enabled_ui(chosen.is_ok(), |ui| widgets::pill_button(ui, tl!("Extract"), true)).inner.clicked() {
                         extract_now = true;
                         close = true;
                     }

crates/ui-egui/src/files.rs

@@ -166,6 +166,28 @@ pub struct ExtractDraft {
     /// the document's name when it opens; empty means that name too. As separate files each
     /// page is `<name> (page N).pdf`, and a single page the user renamed is `<name>.pdf`.
     pub name: String,
+    /// The pages to extract as typed ranges, 1-based ("1-5, 8, 11-13") (#3593). Empty means the
+    /// selected pages (or the current one); the dialog opens with it empty.
+    pub pages: String,
+}
+
+impl ExtractDraft {
+    /// The pages to extract (0-based, in the order typed, each once) from a document of `count`
+    /// pages: the typed [`Self::pages`] ranges, or `selected` when none are typed. Says what is
+    /// wrong with a range that can't be read or takes no page.
+    pub fn pages_to_extract(&self, selected: Vec<usize>, count: usize) -> Result<Vec<usize>, String> {
+        if self.pages.trim().is_empty() {
+            return Ok(selected);
+        }
+        match pdfcraft_engine::print::select_pages(count, Some(&self.pages), &[], pdfcraft_engine::print::Subset::All, false) {
+            Ok(p) if p.is_empty() => Err(tl!("No pages selected").to_string()),
+            Ok(p) => {
+                let mut seen = std::collections::HashSet::new();
+                Ok(p.into_iter().filter(|page| seen.insert(*page)).collect())
+            }
+            Err(e) => Err(e.to_string()),
+        }
+    }
 }
 
 /// `name` made safe as a file name on every platform: no path separators, reserved or control
@@ -605,23 +627,35 @@ impl PdfCraftApp {
         }
     }
 
-    /// Show Extract pages, its file name filled in with the active document's (#737).
+    /// Show Extract pages, its file name filled in with the active document's (#737) and no
+    /// page ranges typed, so it takes the selected pages until some are (#3593).
     pub fn open_extract_dialog(&mut self) {
         self.extract_draft.name =
             self.active_ids().and_then(|(_, id)| self.session.get(id)).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default();
+        self.extract_draft.pages.clear();
         self.dialog = Some(crate::Dialog::Extract);
     }
 
-    /// Copy the selected pages (or the current page) into a new unsaved document tab.
+    /// Copy the pages typed in Extract pages, else the selected pages (or the current page), into
+    /// a new unsaved document tab.
     pub fn extract_selection(&mut self) {
         // What's typed in a form field is part of the document (#166).
         if !self.commit_form_typing() {
             return;
         }
         let Some((i, id)) = self.active_ids() else { return };
-        let pages = self.views[i].target_pages();
-        let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default();
         let opts = self.extract_draft.clone();
+        let pages = match opts.pages_to_extract(self.views[i].target_pages(), self.views[i].page_count()) {
+            Ok(pages) => pages,
+            Err(e) => {
+                self.notify_fmt("Couldn't extract pages: {e}", &[("e", &e)]);
+                return;
+            }
+        };
+        // Deleting takes the pages in document order (typed ranges may run backwards).
+        let mut doomed = pages.clone();
+        doomed.sort_unstable();
+        let stem = self.session.get(id).map(|d| strip_pdf(&d.name).to_string()).unwrap_or_default();
         // The name typed in the dialog (#737), else the document's.
         let chosen = file_stem_from_user(&opts.name);
         let renamed = !chosen.is_empty() && chosen != file_stem_from_user(&stem);
@@ -651,7 +685,7 @@ impl PdfCraftApp {
                 }
                 let unchanged = app.session.get(id).map(|d| d.edit_generation()) == generation;
                 if unchanged && app.active_ids().map(|(_, active)| active) == Some(id) {
-                    app.apply_edit(pdfcraft_engine::Edit::DeletePages { pages });
+                    app.apply_edit(pdfcraft_engine::Edit::DeletePages { pages: doomed });
                 } else {
                     app.notify_tr("The pages were extracted but not deleted, because the document changed while you were choosing a folder.");
                 }
@@ -677,7 +711,7 @@ impl PdfCraftApp {
         if opts.delete {
             // Back on the original document.
             self.active = Some(i);
-            self.apply_edit(pdfcraft_engine::Edit::DeletePages { pages });
+            self.apply_edit(pdfcraft_engine::Edit::DeletePages { pages: doomed });
         }
     }
 

crates/ui-egui/tests/it/pickers.rs

@@ -7,8 +7,8 @@
 use crate::support;
 
 use egui_kittest::Harness;
-use egui_kittest::kittest::Queryable;
-use pdfcraft_render::PageRenderer;
+use egui_kittest::kittest::{NodeT, Queryable};
+use pdfcraft_render::{PageRenderer, RenderRequest, RequestKind};
 use pdfcraft_ui_egui::PdfCraftApp;
 
 /// An `n`-page document with a proper xref table.
@@ -255,6 +255,58 @@ fn extract_pages_saves_under_the_file_name_typed_in_the_dialog() {
     let _ = std::fs::remove_dir_all(&dir);
 }
 
+/// #3593: Extract pages takes page ranges typed in its Pages field, in the order typed, each page
+/// once; left empty, it takes the selected pages as before. A range it can't use is refused, with
+/// the reason in the dialog and the Extract button off.
+#[test]
+fn extract_pages_takes_the_page_ranges_typed_in_the_dialog() {
+    let texts = |app: &PdfCraftApp, view: usize| {
+        let doc = app.session.get(app.views[view].id).unwrap();
+        let mut r = PageRenderer::new(doc.bytes.clone(), Default::default());
+        (0..r.page_count())
+            .map(|p| {
+                let out = r.render(RenderRequest { page: p, kind: RequestKind::Text, scale: 1.0, ..Default::default() });
+                out.text.map(|t| t.plain_text().trim().to_string()).unwrap_or_default()
+            })
+            .collect::<Vec<_>>()
+    };
+    let mut h = harness(&[6]);
+    h.state_mut().views[0].select_pages(&[1]);
+    assert!(h.state_mut().execute("page.extract"));
+    h.run_steps(2);
+    assert_eq!(h.state().extract_draft.pages, "", "opens on the selection");
+    h.get_by_label("Pages");
+    h.get_by_label_contains("1 page selected.");
+    // Ranges, a backward one and a repeat: in that order, each page once.
+    h.state_mut().extract_draft.pages = "5-6, 3-2, 6".into();
+    h.run_steps(2);
+    h.get_by_label_contains("4 pages selected.");
+    h.state_mut().extract_selection();
+    h.run_steps(2);
+    let s = h.state();
+    assert_eq!(s.views.len(), 2, "the pages open in a new tab");
+    assert_eq!(texts(s, 1), ["Page 5", "Page 6", "Page 3", "Page 2"]);
+    // A page that isn't there: the dialog says so, Extract is off and extracts nothing.
+    h.state_mut().active = Some(0);
+    h.state_mut().open_extract_dialog();
+    h.state_mut().extract_draft.pages = "2, 9".into();
+    h.run_steps(2);
+    assert!(h.get_by_label("Extract").accesskit_node().is_disabled(), "Extract is off");
+    h.state_mut().extract_selection();
+    h.run_steps(2);
+    assert_eq!(h.state().views.len(), 2, "nothing extracted");
+    // Extract and delete takes the typed pages out of the document.
+    h.state_mut().active = Some(0);
+    h.state_mut().open_extract_dialog();
+    h.state_mut().extract_draft.pages = "6, 1-2".into();
+    h.state_mut().extract_draft.delete = true;
+    h.state_mut().extract_selection();
+    h.run_steps(2);
+    let s = h.state();
+    assert_eq!(texts(s, 2), ["Page 6", "Page 1", "Page 2"]);
+    assert_eq!(texts(s, 0), ["Page 3", "Page 4", "Page 5"]);
+}
+
 /// Left as the document's name (or emptied), the names are the ones Extract always used.
 #[test]
 fn extract_pages_keeps_the_usual_names_when_the_file_name_is_unchanged() {

parity/acrobat-features.toml

@@ -2788,8 +2788,8 @@ milestone = "M4"
 status = "shipped"
 commands = ["page.extract"]
 tools = ["page_extract"]
-evidence = ["crates/ui-egui/tests/it/editing.rs::extract_button_copies_selected_pages_to_a_new_tab", "crates/automation/tests/it/automation.rs::combine_extract_and_split", "crates/organize/src/tests.rs::extract_and_split_keep_the_pdfx_output_intent_and_identification"]
-notes = "Carries the PDF/X output intent and identification (#263) in a new XMP packet; the source XMP (dates, other schemas) is not copied."
+evidence = ["crates/ui-egui/tests/it/editing.rs::extract_button_copies_selected_pages_to_a_new_tab", "crates/automation/tests/it/automation.rs::combine_extract_and_split", "crates/organize/src/tests.rs::extract_and_split_keep_the_pdfx_output_intent_and_identification", "crates/ui-egui/tests/it/pickers.rs::extract_pages_takes_the_page_ranges_typed_in_the_dialog"]
+notes = "Carries the PDF/X output intent and identification (#263) in a new XMP packet; the source XMP (dates, other schemas) is not copied. The dialog's Pages field takes page ranges (\"1-5, 8, 11-13\", in the order typed, each page once) instead of the selection (#3593); page labels are not accepted there yet."
 
 [[feature]]
 id = "organize.extract-separate-files"