mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-01 22:19:10 -06:00
fix(automation): a page whose paragraph listing was dropped at the cap is refused, not read by position (#2516)
The listings kept for paragraph numbers are capped at 256 pages, and the 257th cleared them all. A page that lost its listing fell back to reading a number as a position in the page's current paragraphs, so after an edit regrouped it a number could rewrite another paragraph. Documents that lose a listing to the cap are remembered, and a page of one with no listing is refused with a message to list it again. doc_close drops a document's listings and that mark. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Sonnet 5.5
parent
acacf9485f
commit
0ca7ed07fa
@@ -32,7 +32,7 @@ mod signing;
|
||||
mod text_cache;
|
||||
mod tools;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::collections::{HashMap, HashSet};
|
||||
use std::path::{Path, PathBuf};
|
||||
use std::sync::Arc;
|
||||
|
||||
@@ -121,6 +121,9 @@ pub struct Automation {
|
||||
/// The paragraphs of the last `text_paragraphs` listing of each page, so a paragraph number
|
||||
/// keeps naming that paragraph after edits regroup the page (#2516).
|
||||
listed: HashMap<(DocId, usize), Vec<BlockKey>>,
|
||||
/// Documents that lost a listing to the cap. A page of one with no listing is refused rather
|
||||
/// than read as "a number is a position", which the page may no longer match.
|
||||
evicted: HashSet<DocId>,
|
||||
}
|
||||
|
||||
impl Default for Automation {
|
||||
@@ -131,7 +134,15 @@ impl Default for Automation {
|
||||
|
||||
impl Automation {
|
||||
pub fn new() -> Self {
|
||||
Self { session: Session::new(), root: None, roots: None, renderers: HashMap::new(), texts: TextCache::default(), listed: HashMap::new() }
|
||||
Self {
|
||||
session: Session::new(),
|
||||
root: None,
|
||||
roots: None,
|
||||
renderers: HashMap::new(),
|
||||
texts: TextCache::default(),
|
||||
listed: HashMap::new(),
|
||||
evicted: HashSet::new(),
|
||||
}
|
||||
}
|
||||
|
||||
/// Confine every path the tools read or write to `root` (relative paths resolve inside it).
|
||||
@@ -908,6 +919,7 @@ impl Automation {
|
||||
/// resolved against that listing.
|
||||
fn record_listing(&mut self, doc: DocId, page: usize, blocks: &[TextBlock]) {
|
||||
if self.listed.len() >= Self::MAX_LISTINGS && !self.listed.contains_key(&(doc, page)) {
|
||||
self.evicted.extend(self.listed.keys().map(|(d, _)| *d));
|
||||
self.listed.clear();
|
||||
}
|
||||
self.listed.insert((doc, page), blocks.iter().map(BlockKey::of).collect());
|
||||
@@ -918,9 +930,16 @@ impl Automation {
|
||||
/// After `text_paragraphs` a number names the paragraph that listing showed, wherever it is
|
||||
/// now. Without a listing it is a position in `blocks`. A paragraph an edit merged into a
|
||||
/// neighbour (or changed) is refused: the number would otherwise rewrite another paragraph
|
||||
/// and lose its text (#2516).
|
||||
/// and lose its text (#2516). So is any number on a page of a document whose listings were
|
||||
/// dropped at the cap, until the page is listed again.
|
||||
fn paragraph_index(&self, doc: DocId, page: usize, k: i64, blocks: &[TextBlock]) -> Result<usize> {
|
||||
let keys = self.listed.get(&(doc, page));
|
||||
if keys.is_none() && self.evicted.contains(&doc) {
|
||||
return Err(ToolError::InvalidArgs(format!(
|
||||
"page {} has no paragraph listing any more: too many pages were listed and the oldest listings were dropped. List the page again with text_paragraphs",
|
||||
page + 1
|
||||
)));
|
||||
}
|
||||
let n = keys.map_or(blocks.len(), Vec::len);
|
||||
let i = usize::try_from(k)
|
||||
.ok()
|
||||
@@ -972,6 +991,8 @@ impl Automation {
|
||||
self.session.close(id);
|
||||
self.renderers.remove(&id);
|
||||
self.texts.remove(&id);
|
||||
self.listed.retain(|(d, _), _| *d != id);
|
||||
self.evicted.remove(&id);
|
||||
Ok(json!({ "closed": id.0 }))
|
||||
}
|
||||
|
||||
|
||||
@@ -6991,6 +6991,37 @@ fn page_render_hide_text_block_refuses_a_paragraph_number_an_earlier_edit_regrou
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
/// The listings kept for paragraph numbers are capped. A page whose listing was dropped must not
|
||||
/// fall back to reading its numbers as positions, which an edit may have regrouped: it is refused
|
||||
/// until the page is listed again (#2516).
|
||||
#[test]
|
||||
fn text_edit_refuses_a_paragraph_number_whose_listing_was_dropped_at_the_cap() {
|
||||
let dir = workdir("text-edit-evicted-listing");
|
||||
std::fs::write(dir.join("p.pdf"), three_paragraph_pdf()).unwrap();
|
||||
let mut a = auto(&dir);
|
||||
let open = |a: &mut Automation| ok(a, "doc_open", json!({"path":"p.pdf"}))["doc"].as_u64().unwrap();
|
||||
let doc = open(&mut a);
|
||||
ok(&mut a, "text_paragraphs", json!({"doc":doc,"page":1}));
|
||||
// 256 more pages fill the cap, so the next new page drops every earlier listing.
|
||||
let others: Vec<u64> = (0..256).map(|_| open(&mut a)).collect();
|
||||
for &other in &others {
|
||||
ok(&mut a, "text_paragraphs", json!({"doc":other,"page":1}));
|
||||
}
|
||||
|
||||
let dropped = a.call("text_edit", &json!({"doc":doc,"page":1,"paragraph":2,"text":"Bravo paragraph, edited."}));
|
||||
assert!(matches!(&dropped, Err(ToolError::InvalidArgs(m)) if m.contains("text_paragraphs")), "{dropped:?}");
|
||||
assert!(!page_text(&mut a, doc).join(" ").contains("Bravo paragraph, edited."), "nothing was rewritten");
|
||||
let hidden = a.call("page_render", &json!({"doc":doc,"page":1,"dpi":72,"hide_text_block":2}));
|
||||
assert!(matches!(&hidden, Err(ToolError::InvalidArgs(m)) if m.contains("text_paragraphs")), "{:?}", hidden.map(|_| ()));
|
||||
|
||||
// A page listed after the drop keeps its numbers, and so does the page once it is listed again.
|
||||
ok(&mut a, "text_edit", json!({"doc":others[255],"page":1,"paragraph":2,"text":"Bravo paragraph, edited."}));
|
||||
ok(&mut a, "text_paragraphs", json!({"doc":doc,"page":1}));
|
||||
ok(&mut a, "text_edit", json!({"doc":doc,"page":1,"paragraph":2,"text":"Bravo paragraph, edited."}));
|
||||
assert!(page_text(&mut a, doc).join(" ").contains("Bravo paragraph, edited."));
|
||||
std::fs::remove_dir_all(dir).unwrap();
|
||||
}
|
||||
|
||||
/// A number from the listing keeps naming its paragraph while other paragraphs are edited, and
|
||||
/// a paragraph can be edited twice in a row by the same number.
|
||||
#[test]
|
||||
|
||||
@@ -105,6 +105,8 @@ two the listing showed. A paragraph an edit merged into a neighbour (or changed)
|
||||
itself when the edit merged it into one, with
|
||||
a message to list the page again, instead of rewriting whichever paragraph took its number
|
||||
(#2516). The result of an edit names the numbers that are gone in `stale_paragraphs`. Listing
|
||||
a page again renumbers it. Without a listing, a number is a position in the page's current
|
||||
a page again renumbers it. The listings kept are capped (256 pages); once the cap drops
|
||||
them, a page of an affected document is refused until it is listed again, instead of reading its
|
||||
numbers as positions. Without a listing, a number is a position in the page's current
|
||||
paragraphs, as before. The inline editor holds the paragraph a switching click asked for the
|
||||
same way. What is still missing is an identity that survives save and reopen (design: #2508).
|
||||
|
||||
Reference in New Issue
Block a user