mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-31 21:48:09 -06:00
feat(organize): Extract pages takes typed page ranges (#3593)
Extract pages only took the selected pages. Its dialog now has a Pages field for ranges such as "1-5, 8, 11-13", read like Combine files' and Print's ranges, in the order typed and each page once. Left empty (as the dialog opens), it takes the selection as before, shown as the hint. A range it can't use says why and turns Extract off; Extract and delete removes the typed pages. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
parent
9526363152
commit
01744fd7eb
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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
|
||||
@@ -604,23 +626,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);
|
||||
@@ -650,7 +684,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 });
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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"
|
||||
|
||||
Reference in New Issue
Block a user