From 9215b6f3b4db1e61366f88f0c17570700fbf3673 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 06:10:05 +0200 Subject: [PATCH] Name the failing option and page count when a chained edit page is out of range (#1550) (#1575) --- apps/pdfcraft-cli/src/main.rs | 12 ++++++++++++ apps/pdfcraft-cli/tests/edit.rs | 15 +++++++++++++++ crates/annot/src/lib.rs | 3 ++- crates/annot/src/tests.rs | 1 + crates/edit/src/lib.rs | 3 ++- crates/edit/src/tests.rs | 1 + crates/organize/src/lib.rs | 3 ++- crates/organize/src/tests.rs | 1 + 8 files changed, 36 insertions(+), 3 deletions(-) diff --git a/apps/pdfcraft-cli/src/main.rs b/apps/pdfcraft-cli/src/main.rs index fcb1f8feb3..818928583f 100644 --- a/apps/pdfcraft-cli/src/main.rs +++ b/apps/pdfcraft-cli/src/main.rs @@ -51,6 +51,7 @@ pdfcraft-cli render --page N [--dpi 96] --out x.png (.png, .jpg, .t pdfcraft-cli text [--page N] extracted text (pages separated by form feeds) pdfcraft-cli edit --out out.pdf [--rotate 1,3:90] [--delete 2,4] [--move 5:1] [--insert-blank 1] [--title T] [--author A] [--full] + (options run in order; page numbers refer to the document after the earlier options) pdfcraft-cli combine … --out combined.pdf pdfcraft-cli extract --pages 1,3,5 --out out.pdf pdfcraft-cli split (--every N | --before 3,7) [--out-dir DIR] @@ -439,6 +440,17 @@ fn edit(args: &[String]) -> Result<(), CliError> { if let Some((option, at)) = destination.filter(|(_, at)| *at > n) { return Err(format!("{option} {} is past the end: it must be between 1 and {} (the document has {n} pages)", at + 1, n + 1).into()); } + // Pages count from the document as the earlier options left it, so a page past the end is + // reported with its own option and the current page count (#1550). + let listed = match &e { + Edit::RotatePages { pages, .. } => Some(("--rotate", pages)), + Edit::DeletePages { pages } => Some(("--delete", pages)), + Edit::MovePages { pages, .. } => Some(("--move", pages)), + _ => None, + }; + if let Some((option, bad)) = listed.and_then(|(option, pages)| pages.iter().find(|p| **p >= n).map(|p| (option, *p))) { + return Err(format!("{option}: page {} does not exist; after the earlier options the document has {n} pages", bad + 1).into()); + } session.apply(id, e).map_err(|e| e.to_string())?; } let bytes = if full { session.save_full_bytes(id) } else { session.save_bytes(id) }.map_err(|e| e.to_string())?; diff --git a/apps/pdfcraft-cli/tests/edit.rs b/apps/pdfcraft-cli/tests/edit.rs index b7c3439d09..c9a5e7e2b3 100644 --- a/apps/pdfcraft-cli/tests/edit.rs +++ b/apps/pdfcraft-cli/tests/edit.rs @@ -224,3 +224,18 @@ fn the_append_destination_and_counts_grown_by_earlier_edits_are_accepted() { assert_eq!(sizes, expected, "{name}"); } } + +/// A page past the end names its own option and the page count left by the earlier options (#1550). +#[test] +fn a_page_past_the_end_after_earlier_edits_names_that_option() { + let input = tmp("chained", "in.pdf"); + std::fs::write(&input, fixture()).unwrap(); + let output = tmp("chained", "out.pdf"); + // 1 page, grown to 3, then --delete leaves 2: the 3 in --move 3:1 is the page that is gone. + let args = ["--insert-blank", "2", "--insert-blank", "3", "--delete", "3", "--move", "3:1"]; + let out = Command::new(BIN).args(["edit", input.to_str().unwrap(), "--out", output.to_str().unwrap()]).args(args).output().unwrap(); + assert!(!out.status.success()); + let err = String::from_utf8_lossy(&out.stderr); + assert!(err.contains("--move: page 3 does not exist") && err.contains("has 2 pages"), "{err}"); + assert!(!output.exists()); +} diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index 642969c5d2..282498f969 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -26,7 +26,8 @@ mod tests; pub enum AnnotError { #[error("the document has no page tree")] NoPageTree, - #[error("page {0} does not exist")] + /// Holds the 0-based index; the message names the 1-based page a user counts (#1550). + #[error("page {} does not exist", .0.saturating_add(1))] NoSuchPage(usize), #[error("there is no comment {index} on page {page}")] NoSuchAnnotation { page: usize, index: usize }, diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index c00ed2be79..59b69dc603 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -187,6 +187,7 @@ fn invalid_geometry_is_rejected() { assert!(matches!(add_annotation(&mut doc, &new(0, s.clone()), &meta("x")), Err(AnnotError::Invalid(_))), "{s:?}"); } assert_eq!(add_annotation(&mut doc, &new(5, Shape::Rectangle { rect: [0.0, 0.0, 10.0, 10.0] }), &meta("x")), Err(AnnotError::NoSuchPage(5))); + assert_eq!(AnnotError::NoSuchPage(5).to_string(), "page 6 does not exist", "the message counts pages from 1 (#1550)"); assert!(!doc.is_modified(), "failed adds change nothing"); } diff --git a/crates/edit/src/lib.rs b/crates/edit/src/lib.rs index 701ed11182..d92f284052 100644 --- a/crates/edit/src/lib.rs +++ b/crates/edit/src/lib.rs @@ -19,7 +19,8 @@ use pdfcraft_fonts::{helvetica_width, literal, win_ansi}; #[derive(Debug, thiserror::Error, PartialEq)] pub enum EditError { - #[error("page {0} does not exist")] + /// Holds the 0-based index; the message names the 1-based page a user counts (#1550). + #[error("page {} does not exist", .0.saturating_add(1))] NoSuchPage(usize), #[error("{0}")] Invalid(String), diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 2ec5f2f495..1ed11f0367 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -179,6 +179,7 @@ fn invalid_requests_change_nothing() { add_background(&mut doc, &[9], &Background { color: [1.0; 3], opacity: 1.0, ..Background::default() }, false), Err(EditError::NoSuchPage(9)) ); + assert_eq!(EditError::NoSuchPage(9).to_string(), "page 10 does not exist", "the message counts pages from 1 (#1550)"); assert!(!doc.is_modified()); } diff --git a/crates/organize/src/lib.rs b/crates/organize/src/lib.rs index bb18939b52..ffbf7cb912 100644 --- a/crates/organize/src/lib.rs +++ b/crates/organize/src/lib.rs @@ -40,7 +40,8 @@ pub use view::{InitialView, displays_doc_title, initial_view, set_initial_view}; pub enum OrganizeError { #[error("the document has no page tree")] NoPageTree, - #[error("page {0} does not exist")] + /// Holds the 0-based index; the message names the 1-based page a user counts (#1550). + #[error("page {} does not exist", .0.saturating_add(1))] NoSuchPage(usize), #[error("a document must keep at least one page")] WouldRemoveAllPages, diff --git a/crates/organize/src/tests.rs b/crates/organize/src/tests.rs index 2ae38fb863..274aed3e47 100644 --- a/crates/organize/src/tests.rs +++ b/crates/organize/src/tests.rs @@ -229,6 +229,7 @@ fn cannot_delete_every_page_or_missing_pages() { assert_eq!(delete_pages(&mut doc, &[0, 1, 2]), Err(OrganizeError::WouldRemoveAllPages)); assert_eq!(delete_pages(&mut doc, &[7]), Err(OrganizeError::NoSuchPage(7))); assert_eq!(rotate_pages(&mut doc, &[3], 90), Err(OrganizeError::NoSuchPage(3))); + assert_eq!(OrganizeError::NoSuchPage(3).to_string(), "page 4 does not exist", "the message counts pages from 1 (#1550)"); assert!(!doc.is_modified(), "failed operations leave the document untouched"); }