mirror of
https://github.com/storytold/pdfcraft.git
synced 2026-11-09 12:32:52 +00:00
test(ui): the page guard sees router aliases and scoped names; true docs
Found in review: - Rule (f) missed `pub(in …)` functions, the visibility the guard's commit brought in (`declares` strips any `pub(…)`), and flagged names that merely end in its words (`dropdowns`): it now matches whole words of the name. - Inside `canvas` the narrowed visibility doesn't hold, and a router taken whole (`let r = &view.router;`) got past the checks by method name; such a use now counts as the router's (`takes_the_router`). - Docs: the router's module doc and `PagePress::Free` say a declined drag is nobody's once it starts; Prepare's note says a highlight's drag is Prepare's; Add text's paragraph note names `ParagraphCx::adding`; a test's doc claimed crop-mode keys it doesn't press. - One source each for the objects' menu test (`object_ui::menu_open`) and Edit text's gesture cancel (`cancel_pointer_gestures` calls `cancel_edit_text_gestures`); the page-menu test asserts an entry only the page's menu has. Each guard addition seen failing against a violation planted in `#[cfg(any())]` code in `canvas/area.rs`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
8 files changed
+48
-24
No files matched your search
@@ -878,18 +878,13 @@ impl DocView {
|
||||
if let Some(q) = &mut self.camera {
|
||||
q.grab = None;
|
||||
}
|
||||
self.block_drag = None;
|
||||
self.org_drag = None;
|
||||
self.panel_drag = None;
|
||||
self.selection_drag = false;
|
||||
self.router.cancel();
|
||||
self.auto_scroll.cancel();
|
||||
self.prepare.cancel_pointer_gesture();
|
||||
self.content.cancel_pointer_gesture();
|
||||
self.objects.cancel_pointer_gesture();
|
||||
if let Some(image) = &mut self.image_selection {
|
||||
image.cancel_pointer_gesture();
|
||||
}
|
||||
self.cancel_edit_text_gestures();
|
||||
}
|
||||
|
||||
/// Abandon Edit text's pointer gestures (a paragraph moved or rewrapped, a picture moved,
|
||||
|
||||
@@ -104,8 +104,7 @@ pub(super) fn page_tool_input(ui: &mut egui::Ui, view: &mut DocView, frame: &mut
|
||||
&& can_modify
|
||||
&& xf.rect.contains(o)
|
||||
&& crate::object_ui::adds(ptr.modifiers)
|
||||
&& !resp.context_menu_opened()
|
||||
&& !egui::Popup::is_any_open(ui.ctx());
|
||||
&& !crate::object_ui::menu_open(ui, resp);
|
||||
let objects = adds.then(|| view.objects.inventory(doc, i));
|
||||
let at = hits.at(view, o, objects.as_ref());
|
||||
// A press is the page's it lies on, or past its edge the one whose selected picture has its
|
||||
@@ -549,9 +548,9 @@ impl PageHits<'_> {
|
||||
.filter(|e| !e.has_changes())
|
||||
.and_then(|e| lines.get(e.block))
|
||||
.map(|l| xf.user_rect(info, page, l.rect.map(|v| v as f32)).expand(2.0));
|
||||
// Add text acts on the page's paragraphs only in their boxes, edges included (it
|
||||
// finishes the box being typed there, `over_source`): not on pictures, nor beside a
|
||||
// paragraph, where a click starts a new box.
|
||||
// Add text acts on the page's paragraphs only in their boxes, edges included (its click
|
||||
// there finishes the box being typed and opens the paragraph, `ParagraphCx::adding`):
|
||||
// not on pictures, nor beside a paragraph, where a click starts a new box.
|
||||
let edit_text = tool == QuickTool::EditText;
|
||||
let on_paragraphs = edit_text || crate::edit_text_ui::over_paragraph(xf, info, page, lines, p);
|
||||
(
|
||||
|
||||
@@ -260,7 +260,11 @@ fn declares(line: &str) -> Option<&str> {
|
||||
let mut rest = line;
|
||||
loop {
|
||||
let before = rest;
|
||||
for prefix in ["pub(crate) ", "pub(super) ", "pub ", "const ", "async ", "unsafe "] {
|
||||
// `pub(crate)`, `pub(super)`, `pub(in crate::canvas)` and the like.
|
||||
if let Some(scoped) = rest.strip_prefix("pub(") {
|
||||
rest = scoped.split_once(") ").map_or(rest, |(_, after)| after);
|
||||
}
|
||||
for prefix in ["pub ", "const ", "async ", "unsafe "] {
|
||||
rest = rest.strip_prefix(prefix).unwrap_or(rest);
|
||||
}
|
||||
if rest == before {
|
||||
@@ -605,9 +609,21 @@ const ROUTER_READERS: [(&str, &str, &str); 1] = [(
|
||||
/// for itself whether the press is its own, which the page view tells it (`PagePress`).
|
||||
const PRESS_CLAIMING_WORDS: [&str; 2] = ["grabs", "owns"];
|
||||
|
||||
/// The function `line` declares, when its name has one of [`PRESS_CLAIMING_WORDS`] in it.
|
||||
/// The function `line` declares, when one of the words of its name is one of
|
||||
/// [`PRESS_CLAIMING_WORDS`].
|
||||
fn press_claiming_fn(line: &str) -> Vec<String> {
|
||||
declares(line.trim_start()).filter(|name| PRESS_CLAIMING_WORDS.iter().any(|w| name.contains(w))).map(str::to_owned).into_iter().collect()
|
||||
declares(line.trim_start())
|
||||
.filter(|name| name.split('_').any(|word| PRESS_CLAIMING_WORDS.contains(&word)))
|
||||
.map(str::to_owned)
|
||||
.into_iter()
|
||||
.collect()
|
||||
}
|
||||
|
||||
/// Whether `line` takes the router whole (`let r = &view.router;`), which the checks by method
|
||||
/// name would not see through: anything but a method call on it.
|
||||
fn takes_the_router(line: &str) -> bool {
|
||||
line.match_indices(".router")
|
||||
.any(|(at, name)| line[at + name.len()..].chars().next().is_none_or(|c| c != '.' && !(c.is_alphanumeric() || c == '_')))
|
||||
}
|
||||
|
||||
/// What ignores the press the pages hold until the button is up.
|
||||
@@ -665,7 +681,7 @@ fn page_modules_act_on_a_press_only_through_the_router() {
|
||||
// functions use it.
|
||||
let known: Vec<&str> = ROUTER_WRITES.iter().chain(ROUTER_READS.iter()).copied().collect();
|
||||
let unlisted = page_module_matches(|line| {
|
||||
let router = uses(line, ".router.") || uses(line, "PageRouter::");
|
||||
let router = uses(line, ".router.") || uses(line, "PageRouter::") || takes_the_router(line);
|
||||
if router && !known.iter().any(|name| uses(line, name)) { vec!["router".to_owned()] } else { Vec::new() }
|
||||
});
|
||||
problems.extend(
|
||||
@@ -766,6 +782,11 @@ fn the_page_guards_read_hit_tests_split_calls_and_aliases() {
|
||||
assert_eq!(declares("let hit = 1;"), None);
|
||||
assert_eq!(press_claiming_fn(" pub(crate) fn fill_grabs(view: &DocView) -> bool {"), ["fill_grabs"]);
|
||||
assert_eq!(press_claiming_fn("fn owns_press() {"), ["owns_press"]);
|
||||
assert_eq!(press_claiming_fn(" pub(in crate::canvas) fn link_grabs(view: &DocView) {"), ["link_grabs"]);
|
||||
assert!(press_claiming_fn("fn dropdowns() {").is_empty(), "a word that ends in one");
|
||||
assert_eq!(declares("pub(in crate::canvas) fn drag_began(&mut self) {"), Some("drag_began"));
|
||||
assert!(takes_the_router("let r = &view.router;") && takes_the_router("keep(view.router, 1)"));
|
||||
assert!(!takes_the_router("view.router.press_for(i)") && !takes_the_router("view.routers.len()"));
|
||||
assert!(press_claiming_fn("fn holds(&self, page: usize) -> bool {").is_empty());
|
||||
assert!(press_claiming_fn("let grabs = 1;").is_empty());
|
||||
let source = "impl PageHits<'_> {\n fn at(&self) {\n self\n .hits(view, p, false)\n }\n}\nfn after() {\n // view.router.owner(0)\n let _ = view\n .router\n .owner(0);\n}\n";
|
||||
|
||||
@@ -6,8 +6,10 @@
|
||||
//! press point decide who captures it ([`PageRouter::press`]), by the same table: the highest layer
|
||||
//! the active tool reaches. Until the button is up only the owner acts on the press, its drag and
|
||||
//! its release, so what the cursor promised is what the press does, and nothing takes a gesture
|
||||
//! back halfway. Each page is told what it has to do with the press ([`PagePress`]): its owner's,
|
||||
//! the active tool's, or none of its business.
|
||||
//! back halfway; an owner that declines the drag (text markup, a link with the Link tool) lets it
|
||||
//! go as it starts, and the drag is nobody's. Each page is told what it has to do with the press
|
||||
//! ([`PagePress`]): its owner's, the active tool's, or none of its business. Only the page view
|
||||
//! reads the router: its methods are `canvas`'s alone.
|
||||
|
||||
use egui::CursorIcon;
|
||||
use pdfcraft_engine::ObjectTarget;
|
||||
@@ -143,8 +145,9 @@ pub(crate) enum PagePress {
|
||||
/// The press began on this page, and the owner's hit won it: its module acts on it. Others
|
||||
/// only let go of their own state (a click on anything but the selected comment unselects it).
|
||||
Mine(Owner),
|
||||
/// No button is down, or the press began on this page and nobody's hit won it: the active
|
||||
/// tool's. Also every page's while a press the page view does not hold is down (one that began
|
||||
/// No button is down, or the press began on this page and nobody's hit won it, or its owner let
|
||||
/// its drag go ([`PageRouter::drag_began`]): the active tool's. Also every page's while a
|
||||
/// press the page view does not hold is down (one that began
|
||||
/// over a panel), and after the press was dropped ([`PageRouter::cancel`]) until the button is
|
||||
/// up (`Elsewhere` instead when Escape came in the press's own frame; page input skips a
|
||||
/// cancelled press either way).
|
||||
|
||||
@@ -160,6 +160,12 @@ fn screen(xf: &PageXform, info: &DocInfo, page: usize, object: &EditableObject)
|
||||
xf.user_rect(info, page, object.rect.map(|v| v as f32))
|
||||
}
|
||||
|
||||
/// A menu is open (the page's, or another popup): the objects selected together leave the pointer
|
||||
/// alone, and a press with Shift or ⌘ is not theirs.
|
||||
pub(crate) fn menu_open(ui: &egui::Ui, resp: &egui::Response) -> bool {
|
||||
resp.context_menu_opened() || egui::Popup::is_any_open(ui.ctx())
|
||||
}
|
||||
|
||||
/// Whether `modifiers` make a press add to the objects selected together, or take from them.
|
||||
pub(crate) fn adds(modifiers: egui::Modifiers) -> bool {
|
||||
modifiers.shift || modifiers.ctrl || modifiers.command
|
||||
@@ -247,7 +253,7 @@ pub(crate) fn page_input(cx: PageToolContext<'_>, press: PagePress, view: &mut D
|
||||
if view.objects.input_blocked {
|
||||
return true;
|
||||
}
|
||||
if resp.context_menu_opened() || egui::Popup::is_any_open(ui.ctx()) {
|
||||
if menu_open(ui, resp) {
|
||||
return false;
|
||||
}
|
||||
match press {
|
||||
|
||||
@@ -380,7 +380,8 @@ pub(crate) fn page_input(
|
||||
|
||||
// The Select tool: what the press is on ([`hit`]), a corner handle or a field; a press nobody
|
||||
// holds draws a selection rectangle, or a field with a field tool. A press held by something
|
||||
// drawn over the fields (a comment) is not Prepare's.
|
||||
// drawn over the fields (a comment) is not Prepare's, though its drag is once that something
|
||||
// lets it go (a highlight's, `PageRouter::drag_began`).
|
||||
let free = press == PagePress::Free;
|
||||
let held = matches!(press, PagePress::Mine(Owner::PrepareField { .. } | Owner::PrepareCorner { .. }));
|
||||
let hit = match press {
|
||||
|
||||
@@ -7503,8 +7503,7 @@ fn delete_on_a_selected_added_picture_deletes_it_alone() {
|
||||
assert_eq!(texts, ["Caption"], "and the text placed after it stays");
|
||||
}
|
||||
|
||||
/// With a dialog open, Delete is the dialog's: the selected picture stays, and so does crop mode
|
||||
/// under Escape and Enter.
|
||||
/// With a dialog open, Delete and Backspace are the dialog's: the selected picture stays.
|
||||
#[test]
|
||||
fn a_dialog_keeps_its_delete_while_a_picture_is_selected() {
|
||||
let mut h = picture_pages(false);
|
||||
|
||||
@@ -1856,7 +1856,7 @@ fn with_prepare_a_right_click_beside_the_selected_field_opens_the_pages_menu() {
|
||||
let blank = at(&h, 60.0, 300.0);
|
||||
right_click(&mut h, blank);
|
||||
assert!(h.query_by_label("Duplicate…").is_none(), "no field menu on the empty page");
|
||||
assert!(h.query_all_by_label_contains("Copy").next().is_some(), "the page's menu");
|
||||
assert!(h.query_by_label("Marquee Zoom").is_some(), "the page's menu");
|
||||
h.key_press(egui::Key::Escape);
|
||||
h.run_steps(3);
|
||||
right_click(&mut h, on_box);
|
||||
|
||||
Reference in new issue
Block a user