Name the failing option and page count when a chained edit page is out of range (#1550) (#1575)

This commit is contained in:
MarekSpodBiedry
2026-10-13 00:10:05 -04:00
committed by GitHub
parent 91c0b93c7f
commit 9215b6f3b4
8 changed files with 36 additions and 3 deletions
+12
View File
@@ -51,6 +51,7 @@ pdfcraft-cli render <file.pdf> --page N [--dpi 96] --out x.png (.png, .jpg, .t
pdfcraft-cli text <file.pdf> [--page N] extracted text (pages separated by form feeds)
pdfcraft-cli edit <in.pdf> --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 <a.pdf> <b.pdf> … --out combined.pdf
pdfcraft-cli extract <in.pdf> --pages 1,3,5 --out out.pdf
pdfcraft-cli split <in.pdf> (--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())?;
+15
View File
@@ -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());
}
+2 -1
View File
@@ -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 },
+1
View File
@@ -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");
}
+2 -1
View File
@@ -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),
+1
View File
@@ -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());
}
+2 -1
View File
@@ -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,
+1
View File
@@ -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");
}