mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-10-28 20:18:06 -06:00
* Extract pages: a File name field names the extracted files (#737) The Extract pages dialog had no way to name the result: separate files were always "<document> (page N).pdf" in the chosen folder, and a new tab was "<document> (extract).pdf". The dialog now has a File name field, filled in with the document's name when it opens. Separate files are "<name> (page N).pdf"; a single renamed page is "<name>.pdf"; extracting into a new tab names the tab (and so the Save As suggestion) "<name>.pdf". The name is made safe for every platform (separators, reserved and control characters, trailing dots and spaces, any-case ".pdf"), and falls back to the document's name when nothing usable is left. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * Extract pages: Windows device names get a prefix, and the name is capped at 200 bytes "NUL.pdf" writes nowhere on Windows, and 200 CJK characters overflow the 255-byte name limit. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: DWAA1660 <DWAA1660@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com> Co-authored-by: Brandon Thomas <echelon@gmail.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
DWAA1660
Brandon Thomas
parent
49a8d04f24
commit
0bc619cd02
@@ -553,7 +553,7 @@ impl PdfCraftApp {
|
||||
self.boxes_draft.seeded = None;
|
||||
self.dialog = Some(Dialog::PageBoxes);
|
||||
}
|
||||
"page.extract" => self.dialog = Some(Dialog::Extract),
|
||||
"page.extract" => self.open_extract_dialog(),
|
||||
"page.rotate_dialog" => {
|
||||
if let Some(i) = active {
|
||||
let n = self.session.get(self.views[i].id).map_or(1, |d| d.info.pages.len());
|
||||
|
||||
@@ -561,6 +561,12 @@ pub fn show(app: &mut PdfCraftApp, ctx: &egui::Context) {
|
||||
crate::i18n::fmt(tl!("{n} pages selected."), &[("n", &count.to_string())])
|
||||
};
|
||||
ui.label(text);
|
||||
ui.add_space(4.0);
|
||||
// The name the pages are saved under (#737); separate files add " (page N)".
|
||||
ui.horizontal(|ui| {
|
||||
let l = ui.label(tl!("File name"));
|
||||
ui.add(egui::TextEdit::singleline(&mut app.extract_draft.name).desired_width(240.0)).labelled_by(l.id);
|
||||
});
|
||||
ui.checkbox(&mut app.extract_draft.delete, tl!("Delete pages after extracting"));
|
||||
ui.checkbox(&mut app.extract_draft.separate, tl!("Extract pages as separate files"));
|
||||
ui.add_space(12.0);
|
||||
|
||||
@@ -143,7 +143,7 @@ impl PdfCraftApp {
|
||||
Some(crate::canvas::ViewAction::Save) => {
|
||||
self.save_active(SaveTarget::InPlace);
|
||||
}
|
||||
Some(crate::canvas::ViewAction::Extract) => self.dialog = Some(crate::Dialog::Extract),
|
||||
Some(crate::canvas::ViewAction::Extract) => self.open_extract_dialog(),
|
||||
Some(crate::canvas::ViewAction::Split) => self.dialog = Some(crate::Dialog::Split),
|
||||
Some(crate::canvas::ViewAction::CopyPages { cut }) => self.copy_pages(cut),
|
||||
Some(crate::canvas::ViewAction::PastePages) => self.paste_pages(),
|
||||
|
||||
@@ -115,6 +115,37 @@ pub struct ExtractDraft {
|
||||
pub separate: bool,
|
||||
/// Delete the pages after extracting them.
|
||||
pub delete: bool,
|
||||
/// The name the extracted pages are saved under, without `.pdf` (#737). The dialog fills in
|
||||
/// 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,
|
||||
}
|
||||
|
||||
/// `name` made safe as a file name on every platform: no path separators, reserved or control
|
||||
/// characters, no trailing dots or spaces (Windows drops them), no `.pdf` (added back on
|
||||
/// writing), at most 200 characters. Empty when nothing usable is left.
|
||||
pub(crate) fn file_stem_from_user(name: &str) -> String {
|
||||
let name = name.trim();
|
||||
// Any case of ".pdf": it's added back on writing.
|
||||
let name =
|
||||
name.len().checked_sub(4).and_then(|i| name.get(i..).filter(|ext| ext.eq_ignore_ascii_case(".pdf")).and(name.get(..i))).unwrap_or(name);
|
||||
let cleaned: String =
|
||||
name.chars().map(|c| if c.is_control() || matches!(c, '/' | '\\' | ':' | '*' | '?' | '"' | '<' | '>' | '|') { '_' } else { c }).collect();
|
||||
// At most 200 bytes, so " (page N).pdf" still fits the usual 255-byte file name limit.
|
||||
let mut end = cleaned.len().min(200);
|
||||
while !cleaned.is_char_boundary(end) {
|
||||
end -= 1;
|
||||
}
|
||||
let cleaned = cleaned.get(..end).unwrap_or_default();
|
||||
// Trimming trailing dots also leaves "." and ".." empty.
|
||||
let stem = cleaned.trim().trim_end_matches(['.', ' ']).trim_start();
|
||||
// Windows reserves these device names whatever the extension: "NUL.pdf" writes nowhere.
|
||||
let device = stem.split('.').next().unwrap_or_default().to_ascii_uppercase();
|
||||
let reserved = matches!(device.as_str(), "CON" | "PRN" | "AUX" | "NUL")
|
||||
|| (device.len() == 4
|
||||
&& (device.starts_with("COM") || device.starts_with("LPT"))
|
||||
&& device.as_bytes().get(3).is_some_and(|d| (b'1'..=b'9').contains(d)));
|
||||
if reserved { format!("_{stem}") } else { stem.to_string() }
|
||||
}
|
||||
|
||||
/// Pages ▸ Rotate Pages.
|
||||
@@ -382,6 +413,13 @@ impl PdfCraftApp {
|
||||
}
|
||||
}
|
||||
|
||||
/// Show Extract pages, its file name filled in with the active document's (#737).
|
||||
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.dialog = Some(crate::Dialog::Extract);
|
||||
}
|
||||
|
||||
/// Copy 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).
|
||||
@@ -392,12 +430,18 @@ impl PdfCraftApp {
|
||||
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();
|
||||
// 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);
|
||||
let base = if chosen.is_empty() { stem.clone() } else { chosen };
|
||||
if opts.separate {
|
||||
// Each page as its own file, in a chosen folder.
|
||||
let mut named = Vec::new();
|
||||
for &p in &pages {
|
||||
match self.session.extract(id, &[p]) {
|
||||
Ok(bytes) => named.push((format!("{stem} (page {}).pdf", p + 1), bytes)),
|
||||
// One page the user named is saved under exactly that name.
|
||||
Ok(bytes) if renamed && pages.len() == 1 => named.push((format!("{base}.pdf"), bytes)),
|
||||
Ok(bytes) => named.push((format!("{base} (page {}).pdf", p + 1), bytes)),
|
||||
Err(e) => {
|
||||
self.notify_fmt("Couldn't extract pages: {e}", &[("e", &e.to_string())]);
|
||||
return;
|
||||
@@ -429,7 +473,9 @@ impl PdfCraftApp {
|
||||
} else {
|
||||
crate::i18n::fmt(tl!("Extracted {n} pages"), &[("n", &pages.len().to_string())])
|
||||
};
|
||||
self.open_created(&format!("{stem} (extract).pdf"), bytes, &message)
|
||||
// Save As offers the tab's name, so a name typed in the dialog is kept.
|
||||
let name = if renamed { format!("{base}.pdf") } else { format!("{stem} (extract).pdf") };
|
||||
self.open_created(&name, bytes, &message)
|
||||
}
|
||||
Err(e) => {
|
||||
self.notify_fmt("Couldn't extract pages: {e}", &[("e", &e.to_string())]);
|
||||
@@ -754,3 +800,24 @@ impl PdfCraftApp {
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod stem_tests {
|
||||
use super::file_stem_from_user;
|
||||
|
||||
#[test]
|
||||
fn user_file_names_stay_inside_the_folder_and_writable_everywhere() {
|
||||
assert_eq!(file_stem_from_user("../../etc/passwd"), ".._.._etc_passwd");
|
||||
assert_eq!(file_stem_from_user(".."), "");
|
||||
assert_eq!(file_stem_from_user("Report.PDF"), "Report");
|
||||
// Windows device names, in any case and with any extension, get a prefix.
|
||||
assert_eq!(file_stem_from_user("nul"), "_nul");
|
||||
assert_eq!(file_stem_from_user("COM1.pdf"), "_COM1");
|
||||
assert_eq!(file_stem_from_user("lpt9.tar"), "_lpt9.tar");
|
||||
assert_eq!(file_stem_from_user("COM10"), "COM10");
|
||||
assert_eq!(file_stem_from_user("Console"), "Console");
|
||||
// At most 200 bytes, cut on a character boundary.
|
||||
let long = file_stem_from_user(&"文".repeat(300));
|
||||
assert!(long.len() <= 200 && long.chars().all(|c| c == '文'), "{}", long.len());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1503,7 +1503,7 @@ fn extract_options_and_rotate_pages_dialog() {
|
||||
h.state_mut().views[0].select_pages(&[1, 2]);
|
||||
h.state_mut().run_command("page.extract");
|
||||
h.run_steps(2);
|
||||
h.state_mut().extract_draft = pdfcraft_ui_egui::ExtractDraft { separate: true, delete: true };
|
||||
h.state_mut().extract_draft = pdfcraft_ui_egui::ExtractDraft { separate: true, delete: true, ..Default::default() };
|
||||
h.get_all_by_label("Extract").last().unwrap().click();
|
||||
h.run_steps(3);
|
||||
let mut names: Vec<String> = std::fs::read_dir(&dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect();
|
||||
|
||||
@@ -213,6 +213,67 @@ fn extract_and_delete_deletes_the_pages_only_after_their_files_are_written() {
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// #737: Extract pages names the files after the name typed in the dialog, which starts out
|
||||
/// as the document's name.
|
||||
#[test]
|
||||
fn extract_pages_saves_under_the_file_name_typed_in_the_dialog() {
|
||||
let dir = scratch("extract-named");
|
||||
let mut h = harness(&[3]);
|
||||
assert!(h.state_mut().execute("page.extract"));
|
||||
h.run_steps(2);
|
||||
assert_eq!(h.state().extract_draft.name, "doc0", "the dialog starts with the document's name");
|
||||
// Several pages as separate files: each is "<name> (page N).pdf".
|
||||
h.state_mut().views[0].select_pages(&[0, 2]);
|
||||
h.state_mut().extract_draft.name = "Invoices.pdf".into();
|
||||
h.state_mut().extract_draft.separate = true;
|
||||
h.state_mut().pick_override = Some(vec![dir.to_string_lossy().into_owned()]);
|
||||
h.state_mut().extract_selection();
|
||||
h.run_steps(2);
|
||||
assert!(dir.join("Invoices (page 1).pdf").exists() && dir.join("Invoices (page 3).pdf").exists());
|
||||
// One renamed page as a separate file is saved under exactly that name; characters a file
|
||||
// name can't hold become "_".
|
||||
h.state_mut().views[0].select_pages(&[1]);
|
||||
h.state_mut().extract_draft.name = " March: 2026/receipt ".into();
|
||||
h.state_mut().extract_selection();
|
||||
h.run_steps(2);
|
||||
assert!(dir.join("March_ 2026_receipt.pdf").exists());
|
||||
assert_eq!(std::fs::read_dir(&dir).unwrap().count(), 3, "nothing written outside those names");
|
||||
// Into a new tab: the tab, and so Save As, has the typed name.
|
||||
h.state_mut().extract_draft.separate = false;
|
||||
h.state_mut().extract_draft.name = "Chapter 2".into();
|
||||
h.state_mut().extract_selection();
|
||||
h.run_steps(2);
|
||||
let s = h.state();
|
||||
let tab = s.active.and_then(|i| s.views.get(i)).and_then(|v| s.session.get(v.id)).map(|d| d.name.clone());
|
||||
assert_eq!(tab.as_deref(), Some("Chapter 2.pdf"));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
/// 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() {
|
||||
let dir = scratch("extract-default-name");
|
||||
let mut h = harness(&[3]);
|
||||
h.state_mut().open_extract_dialog();
|
||||
h.state_mut().extract_draft.separate = true;
|
||||
h.state_mut().pick_override = Some(vec![dir.to_string_lossy().into_owned()]);
|
||||
h.state_mut().extract_selection();
|
||||
h.run_steps(2);
|
||||
assert!(dir.join("doc0 (page 1).pdf").exists());
|
||||
h.state_mut().extract_draft.name = " .. ".into();
|
||||
h.state_mut().views[0].select_pages(&[1]);
|
||||
h.state_mut().extract_selection();
|
||||
h.run_steps(2);
|
||||
assert!(dir.join("doc0 (page 2).pdf").exists(), "a name with nothing usable falls back to the document's");
|
||||
h.state_mut().extract_draft.separate = false;
|
||||
h.state_mut().extract_selection();
|
||||
h.run_steps(2);
|
||||
let s = h.state();
|
||||
let tab = s.active.and_then(|i| s.views.get(i)).and_then(|v| s.session.get(v.id)).map(|d| d.name.clone());
|
||||
assert_eq!(tab.as_deref(), Some("doc0 (extract).pdf"));
|
||||
let _ = std::fs::remove_dir_all(&dir);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_cancelled_folder_pick_extracts_and_deletes_nothing() {
|
||||
let mut h = harness(&[3]);
|
||||
|
||||
@@ -2722,8 +2722,8 @@ milestone = "M4"
|
||||
status = "shipped"
|
||||
commands = ["page.extract"]
|
||||
tools = ["page_extract"]
|
||||
evidence = ["crates/ui-egui/tests/editing.rs::extract_options_and_rotate_pages_dialog", "crates/automation/tests/automation.rs::organizing_with_filters_bookmark_splits_and_extract_options"]
|
||||
notes = ""
|
||||
evidence = ["crates/ui-egui/tests/editing.rs::extract_options_and_rotate_pages_dialog", "crates/automation/tests/automation.rs::organizing_with_filters_bookmark_splits_and_extract_options", "crates/ui-egui/tests/pickers.rs::extract_pages_saves_under_the_file_name_typed_in_the_dialog"]
|
||||
notes = "The dialog has a File name field (#737), filled in with the document's name: pages are written as `<name> (page N).pdf` into the chosen folder, and one renamed page as `<name>.pdf`. Extracting into a new tab names the tab (and so Save As) after it."
|
||||
|
||||
[[feature]]
|
||||
id = "organize.extract-and-delete"
|
||||
|
||||
Reference in New Issue
Block a user