mirror of
https://github.com/storytold/designcraft.git
synced 2026-10-28 20:26:57 -06:00
Reduce cognitive complexity in find, lists, PDF font/layer scans; fix wildcard import (#1846)
* test(pdf): replace wildcard super import in registration_tests Resolves SonarCloud rust:S2208 (new code). * test(engine): extract child-scope rejection assertions in find tests Resolves SonarCloud rust:S3776 on find_scope_children_error_atomically_and_parent_requests_stay_top_level. * refactor(engine): extract per-paragraph list update from apply_list Resolves SonarCloud rust:S3776 on apply_list. * refactor(pdf): extract form scanning from FontScan::content Resolves SonarCloud rust:S3776 on FontScan::content (pdfx/fonts.rs). * refactor(pdf): extract layer helpers from optional_content::prepare Resolves SonarCloud rust:S3776 on prepare (optional_content.rs).
This commit is contained in:
Dylan M. Taylor
@@ -1082,6 +1082,27 @@ mod tests {
|
||||
(s, sid, selected)
|
||||
}
|
||||
|
||||
/// Child-story scopes are rejected by every find command, leaving the document untouched.
|
||||
fn assert_child_scope_requests_rejected(s: &mut Session) {
|
||||
let before = s.doc().unwrap().doc.clone();
|
||||
let selection = s.doc().unwrap().selection.clone();
|
||||
let history = s.doc().unwrap().history.undo.len();
|
||||
let revision = s.doc().unwrap().revision;
|
||||
let journal = s.journal.clone();
|
||||
for scope in ["story", "selection"] {
|
||||
for command in ["find.find", "find.next", "find.change"] {
|
||||
let err = s.execute(command, &json!({"find": "red", "change": "blue", "scope": scope})).unwrap_err();
|
||||
assert!(matches!(err, crate::EngineError::BadParams { .. }), "{err}");
|
||||
assert!(err.to_string().contains("not supported"));
|
||||
assert!(std::sync::Arc::ptr_eq(&before, &s.doc().unwrap().doc));
|
||||
assert_eq!(s.doc().unwrap().selection, selection);
|
||||
assert_eq!(s.doc().unwrap().history.undo.len(), history);
|
||||
assert_eq!(s.doc().unwrap().revision, revision);
|
||||
assert_eq!(s.journal, journal);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_scope_children_error_atomically_and_parent_requests_stay_top_level() {
|
||||
for note in [false, true] {
|
||||
@@ -1090,23 +1111,8 @@ mod tests {
|
||||
selected.anchor = 0;
|
||||
selected.focus = child.len();
|
||||
s.doc_mut().unwrap().selection = designcraft_doc::Selection::text(selected);
|
||||
let before = s.doc().unwrap().doc.clone();
|
||||
assert_child_scope_requests_rejected(&mut s);
|
||||
let selection = s.doc().unwrap().selection.clone();
|
||||
let history = s.doc().unwrap().history.undo.len();
|
||||
let revision = s.doc().unwrap().revision;
|
||||
let journal = s.journal.clone();
|
||||
for scope in ["story", "selection"] {
|
||||
for command in ["find.find", "find.next", "find.change"] {
|
||||
let err = s.execute(command, &json!({"find": "red", "change": "blue", "scope": scope})).unwrap_err();
|
||||
assert!(matches!(err, crate::EngineError::BadParams { .. }), "{err}");
|
||||
assert!(err.to_string().contains("not supported"));
|
||||
assert!(std::sync::Arc::ptr_eq(&before, &s.doc().unwrap().doc));
|
||||
assert_eq!(s.doc().unwrap().selection, selection);
|
||||
assert_eq!(s.doc().unwrap().history.undo.len(), history);
|
||||
assert_eq!(s.doc().unwrap().revision, revision);
|
||||
assert_eq!(s.journal, journal);
|
||||
}
|
||||
}
|
||||
for explicit in [false, true] {
|
||||
let mut p = json!({"find": "red", "change": "blue"});
|
||||
if explicit {
|
||||
|
||||
@@ -92,40 +92,43 @@ fn apply_list(s: &mut Session, p: &Value, kind: ListType) -> Result<Value> {
|
||||
for t in &targets {
|
||||
let Some(st) = d.text_story_mut(t.story, t.cell) else { continue };
|
||||
let span = para_span(&st.text, &t.range);
|
||||
st.format_paras(span, |pf| {
|
||||
let (now, _) = styles.resolve_para(pf);
|
||||
// Off takes away this list only: a paragraph with the other list keeps it.
|
||||
if !on && now.list_type != kind {
|
||||
return;
|
||||
}
|
||||
let (style, _) = styles.resolve_para_style(&pf.style);
|
||||
// The hanging indent this button set (and nothing has changed since).
|
||||
let own_hang = pf.para.list_hang == Some(true) && now.left_indent == LIST_HANG && now.first_line_indent == -LIST_HANG;
|
||||
// A value: left to the style when it gives the same, else set here.
|
||||
let mut set = |key: &str, v: Value, same: bool| {
|
||||
let _ = pf.para.set_json(key, if same { &Value::Null } else { &v });
|
||||
};
|
||||
let list = if on { kind } else { ListType::None };
|
||||
set("listType", serde_json::to_value(list).unwrap_or(Value::Null), style.list_type == list);
|
||||
if on && now.left_indent == 0.0 && now.first_line_indent == 0.0 {
|
||||
set("leftIndent", json!(LIST_HANG), style.left_indent == LIST_HANG);
|
||||
set("firstLineIndent", json!(-LIST_HANG), style.first_line_indent == -LIST_HANG);
|
||||
set("listHang", json!(true), false);
|
||||
} else if !on && now.list_type == kind {
|
||||
// Only the hanging indent set here: one the paragraph had before, or its style
|
||||
// gives, stays.
|
||||
if own_hang {
|
||||
set("leftIndent", json!(0.0), style.left_indent == 0.0);
|
||||
set("firstLineIndent", json!(0.0), style.first_line_indent == 0.0);
|
||||
}
|
||||
set("listHang", Value::Null, true);
|
||||
}
|
||||
});
|
||||
st.format_paras(span, |pf| set_para_list(pf, &styles, on, kind));
|
||||
}
|
||||
Ok(json!({"on": on}))
|
||||
})
|
||||
}
|
||||
|
||||
/// One paragraph's share of [`apply_list`]: turns the list on or off, setting only the values the
|
||||
/// style does not already give.
|
||||
fn set_para_list(pf: &mut designcraft_doc::ParaFormat, styles: &designcraft_doc::Styles, on: bool, kind: ListType) {
|
||||
let (now, _) = styles.resolve_para(pf);
|
||||
// Off takes away this list only: a paragraph with the other list keeps it.
|
||||
if !on && now.list_type != kind {
|
||||
return;
|
||||
}
|
||||
let (style, _) = styles.resolve_para_style(&pf.style);
|
||||
// The hanging indent this button set (and nothing has changed since).
|
||||
let own_hang = pf.para.list_hang == Some(true) && now.left_indent == LIST_HANG && now.first_line_indent == -LIST_HANG;
|
||||
// A value: left to the style when it gives the same, else set here.
|
||||
let mut set = |key: &str, v: Value, same: bool| {
|
||||
let _ = pf.para.set_json(key, if same { &Value::Null } else { &v });
|
||||
};
|
||||
let list = if on { kind } else { ListType::None };
|
||||
set("listType", serde_json::to_value(list).unwrap_or(Value::Null), style.list_type == list);
|
||||
if on && now.left_indent == 0.0 && now.first_line_indent == 0.0 {
|
||||
set("leftIndent", json!(LIST_HANG), style.left_indent == LIST_HANG);
|
||||
set("firstLineIndent", json!(-LIST_HANG), style.first_line_indent == -LIST_HANG);
|
||||
set("listHang", json!(true), false);
|
||||
} else if !on && now.list_type == kind {
|
||||
// Only the hanging indent set here: one the paragraph had before, or its style gives, stays.
|
||||
if own_hang {
|
||||
set("leftIndent", json!(0.0), style.left_indent == 0.0);
|
||||
set("firstLineIndent", json!(0.0), style.first_line_indent == 0.0);
|
||||
}
|
||||
set("listHang", Value::Null, true);
|
||||
}
|
||||
}
|
||||
|
||||
/// The range whose paragraphs a list applies to: a selection ending where a paragraph starts (after
|
||||
/// a return) doesn't reach into that paragraph. The story's last paragraph, empty after a final
|
||||
/// return, stays in when the range runs to the end (a whole frame).
|
||||
|
||||
@@ -239,16 +239,7 @@ pub(crate) fn prepare(bytes: &Arc<Vec<u8>>, choice: Option<&[String]>) -> Result
|
||||
let is_hidden = match (choice, name.as_ref()) {
|
||||
(None, _) => !fallback && (off.contains(&id) || (base_off && !on.contains(&id))),
|
||||
(Some([]), None) => false,
|
||||
(Some(_), Some(name)) => {
|
||||
let name = String::from_utf8_lossy(name.as_bytes());
|
||||
let mut is_hidden = false;
|
||||
for (n, found) in hidden.iter().zip(&mut found) {
|
||||
if n == name.as_ref() {
|
||||
(*found, is_hidden) = (true, true);
|
||||
}
|
||||
}
|
||||
is_hidden
|
||||
}
|
||||
(Some(_), Some(name)) => matches_hidden(hidden, &mut found, name.as_bytes()),
|
||||
(Some(_), None) => return Err(invalid()),
|
||||
};
|
||||
if is_hidden {
|
||||
@@ -260,18 +251,8 @@ pub(crate) fn prepare(bytes: &Arc<Vec<u8>>, choice: Option<&[String]>) -> Result
|
||||
if !is_hidden && name.is_some() && choice.is_none() {
|
||||
continue;
|
||||
}
|
||||
let marker = is_hidden.to_string();
|
||||
let body = match name {
|
||||
Some(_) => edited(&group, &[(MARKER, marker.as_bytes())])?,
|
||||
None => {
|
||||
if group.contains_key(b"Name") && group.get::<Object<'_>>(b"Name").is_none() {
|
||||
return Err(invalid());
|
||||
}
|
||||
renamed = true;
|
||||
let label = format!("(Placed PDF layer {} {})", id.obj_number, id.gen_number);
|
||||
edited(&group, &[(MARKER, marker.as_bytes()), (b"Name", label.as_bytes())])?
|
||||
}
|
||||
};
|
||||
let (body, generated) = layer_edit(&group, id, name.is_some(), is_hidden)?;
|
||||
renamed |= generated;
|
||||
changes.push((id, body));
|
||||
}
|
||||
if found.contains(&false) {
|
||||
@@ -280,6 +261,39 @@ pub(crate) fn prepare(bytes: &Arc<Vec<u8>>, choice: Option<&[String]>) -> Result
|
||||
effective.sort();
|
||||
effective.dedup();
|
||||
let data = if changes.is_empty() { bytes.clone() } else { Arc::new(append(bytes, &changes, xref.root_id())?) };
|
||||
let warning = layer_warning(fallback, incomplete, renamed);
|
||||
Ok(Variant { data, hidden: effective, warning, has_optional_content: true })
|
||||
}
|
||||
|
||||
/// Whether the layer called `name` is one of the `hidden` choices. Each choice it matches is
|
||||
/// recorded in `found`, so a choice that names no layer can be reported.
|
||||
fn matches_hidden(hidden: &[String], found: &mut [bool], name: &[u8]) -> bool {
|
||||
let name = String::from_utf8_lossy(name);
|
||||
let mut is_hidden = false;
|
||||
for (n, found) in hidden.iter().zip(found) {
|
||||
if n == name.as_ref() {
|
||||
(*found, is_hidden) = (true, true);
|
||||
}
|
||||
}
|
||||
is_hidden
|
||||
}
|
||||
|
||||
/// The rewritten group for a layer in the output: its marker records `is_hidden`, and a layer
|
||||
/// without a readable name gets a generated one. The flag reports whether a name was generated.
|
||||
fn layer_edit(group: &Dict<'_>, id: ObjectIdentifier, named: bool, is_hidden: bool) -> Result<(Vec<u8>, bool)> {
|
||||
let marker = is_hidden.to_string();
|
||||
if named {
|
||||
return Ok((edited(group, &[(MARKER, marker.as_bytes())])?, false));
|
||||
}
|
||||
if group.contains_key(b"Name") && group.get::<Object<'_>>(b"Name").is_none() {
|
||||
return Err(invalid());
|
||||
}
|
||||
let label = format!("(Placed PDF layer {} {})", id.obj_number, id.gen_number);
|
||||
Ok((edited(group, &[(MARKER, marker.as_bytes()), (b"Name", label.as_bytes())])?, true))
|
||||
}
|
||||
|
||||
/// The warning a prepared variant carries, or `None` when the layers were read as written.
|
||||
fn layer_warning(fallback: bool, incomplete: bool, renamed: bool) -> Option<String> {
|
||||
let mut warnings = Vec::new();
|
||||
if fallback {
|
||||
warnings.push("layer state not readable, all layers shown unless hidden by Object Layer Options");
|
||||
@@ -290,8 +304,7 @@ pub(crate) fn prepare(bytes: &Arc<Vec<u8>>, choice: Option<&[String]>) -> Result
|
||||
if renamed {
|
||||
warnings.push("layer display name not readable; generated a name, visibility preserved");
|
||||
}
|
||||
let warning = if warnings.is_empty() { None } else { Some(warnings.join("; ")) };
|
||||
Ok(Variant { data, hidden: effective, warning, has_optional_content: true })
|
||||
if warnings.is_empty() { None } else { Some(warnings.join("; ")) }
|
||||
}
|
||||
|
||||
pub(crate) fn register(bytes: &[u8]) -> Result<Vec<u8>> {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
|
||||
|
||||
use hayro_syntax::content::UntypedIter;
|
||||
use hayro_syntax::object::{Array, Dict, Name, Object, ObjectIdentifier};
|
||||
use hayro_syntax::object::{Array, Dict, Name, Object, ObjectIdentifier, Stream};
|
||||
use hayro_syntax::page::Resources;
|
||||
|
||||
use super::MAX_DEPTH;
|
||||
@@ -127,46 +127,46 @@ impl<'a> FontScan<'a> {
|
||||
false
|
||||
})
|
||||
&& self.active.insert(form.obj_id())
|
||||
&& self.scan_form(form, res, depth)
|
||||
{
|
||||
let mut resources = Vec::new();
|
||||
let mut parent = Some(res);
|
||||
while let Some(r) = parent {
|
||||
resources.push((r.fonts.data(), r.x_objects.data()));
|
||||
parent = r.parent();
|
||||
}
|
||||
let key = FormContext {
|
||||
form: form.obj_id(),
|
||||
font: self.font.as_ref().map(Dict::data),
|
||||
mode: self.mode,
|
||||
resources,
|
||||
ancestors: self.path.clone(),
|
||||
};
|
||||
if let Some(found) = self.completed.get(&key) {
|
||||
self.found.extend(found.iter().cloned());
|
||||
self.active.remove(&form.obj_id());
|
||||
continue;
|
||||
}
|
||||
let inner = Resources::from_parent(form.dict().get::<Dict<'_>>(b"Resources").unwrap_or_default(), res.clone());
|
||||
let state = (self.font.clone(), self.mode);
|
||||
let outer_found = std::mem::take(&mut self.found);
|
||||
self.path.push(form.obj_id());
|
||||
if let Ok(data) = form.decoded() {
|
||||
self.content(&data, &inner, depth + 1);
|
||||
}
|
||||
self.path.pop();
|
||||
if !self.limited {
|
||||
self.completed.insert(key, self.found.clone());
|
||||
}
|
||||
self.found.extend(outer_found);
|
||||
(self.font, self.mode) = state;
|
||||
self.active.remove(&form.obj_id());
|
||||
if self.limited {
|
||||
return;
|
||||
}
|
||||
return;
|
||||
}
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Scans a form XObject the caller has marked active. Returns `true` when the operation
|
||||
/// limit stopped the scan, so the caller must stop too.
|
||||
fn scan_form(&mut self, form: Stream<'a>, res: &Resources<'a>, depth: usize) -> bool {
|
||||
let mut resources = Vec::new();
|
||||
let mut parent = Some(res);
|
||||
while let Some(r) = parent {
|
||||
resources.push((r.fonts.data(), r.x_objects.data()));
|
||||
parent = r.parent();
|
||||
}
|
||||
let key =
|
||||
FormContext { form: form.obj_id(), font: self.font.as_ref().map(Dict::data), mode: self.mode, resources, ancestors: self.path.clone() };
|
||||
if let Some(found) = self.completed.get(&key) {
|
||||
self.found.extend(found.iter().cloned());
|
||||
self.active.remove(&form.obj_id());
|
||||
return false;
|
||||
}
|
||||
let inner = Resources::from_parent(form.dict().get::<Dict<'_>>(b"Resources").unwrap_or_default(), res.clone());
|
||||
let state = (self.font.clone(), self.mode);
|
||||
let outer_found = std::mem::take(&mut self.found);
|
||||
self.path.push(form.obj_id());
|
||||
if let Ok(data) = form.decoded() {
|
||||
self.content(&data, &inner, depth + 1);
|
||||
}
|
||||
self.path.pop();
|
||||
if !self.limited {
|
||||
self.completed.insert(key, self.found.clone());
|
||||
}
|
||||
self.found.extend(outer_found);
|
||||
(self.font, self.mode) = state;
|
||||
self.active.remove(&form.obj_id());
|
||||
self.limited
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,4 +1,7 @@
|
||||
use super::*;
|
||||
use super::{
|
||||
Affine, Arc, AssetId, BookletOptions, Cache, Content, Document, Item, PdfOptions, Rect, SpreadRef, Standard, export_booklet, export_pdf,
|
||||
merge_pdfs,
|
||||
};
|
||||
|
||||
fn placed_document(source: Vec<u8>) -> Document {
|
||||
use designcraft_doc::{Asset, Graphic, ItemId, Shape};
|
||||
|
||||
Reference in New Issue
Block a user