Hole: a held hole that cannot follow says so on a drag too, until the inputs change (#6818)

The message for a pick the held line cannot reach is now set through the dialog's refusal, so a changed distance or edge clears it; the marker drag gives it as well; it is shown only when the line leaves the face, not when the distance does not evaluate (which has its own error); and it tells where to pick: where the line crosses the face.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

assets/locales/pending/hole-held-off-its-line.json

@@ -1,20 +1,20 @@
 {
   "en": {
-    "dialog.hole_held_off_its_line": "The reference edge holds this hole on a line that leaves the face before the picked point. Pick nearer the line, or take the edge out to move the hole freely."
+    "dialog.hole_held_off_its_line": "The reference edge holds this hole on a line that leaves the face before the picked point. Pick where the line crosses the face, or take the edge out to move the hole freely."
   },
   "ar": {
-    "dialog.hole_held_off_its_line": "الحافة المرجعية تُبقي هذا الثقب على خط يغادر الوجه قبل النقطة المختارة. اختر نقطة أقرب إلى الخط، أو أزل الحافة لتحريك الثقب بحرية."
+    "dialog.hole_held_off_its_line": "الحافة المرجعية تُبقي هذا الثقب على خط يغادر الوجه قبل النقطة المختارة. اختر حيث يعبر الخط الوجه، أو أزل الحافة لتحريك الثقب بحرية."
   },
   "he": {
-    "dialog.hole_held_off_its_line": "קצה הייחוס מחזיק את החור על קו שיוצא מהפאה לפני הנקודה שנבחרה. יש לבחור קרוב יותר לקו, או להסיר את הקצה כדי להזיז את החור בחופשיות."
+    "dialog.hole_held_off_its_line": "קצה הייחוס מחזיק את החור על קו שיוצא מהפאה לפני הנקודה שנבחרה. יש לבחור במקום שבו הקו עובר על הפאה, או להסיר את הקצה כדי להזיז את החור בחופשיות."
   },
   "ja": {
-    "dialog.hole_held_off_its_line": "参照エッジがこの穴を直線上に固定していますが、その直線は選択した点より手前で面の外に出ます。直線の近くを選択するか、エッジを外すと穴を自由に移動できます。"
+    "dialog.hole_held_off_its_line": "参照エッジがこの穴を直線上に固定していますが、その直線は選択した点より手前で面の外に出ます。直線が面の上を通る位置を選択するか、エッジを外すと穴を自由に移動できます。"
   },
   "vi": {
-    "dialog.hole_held_off_its_line": "Cạnh tham chiếu giữ lỗ này trên một đường thẳng ra khỏi mặt trước điểm đã chọn. Hãy chọn gần đường hơn, hoặc bỏ cạnh để di chuyển lỗ tự do."
+    "dialog.hole_held_off_its_line": "Cạnh tham chiếu giữ lỗ này trên một đường thẳng ra khỏi mặt trước điểm đã chọn. Hãy chọn chỗ đường thẳng đi qua mặt, hoặc bỏ cạnh để di chuyển lỗ tự do."
   },
   "zh": {
-    "dialog.hole_held_off_its_line": "参考边将此孔固定在一条直线上，该直线在所选点之前就离开了该面。请在靠近直线处选取，或移除该边以自由移动孔。"
+    "dialog.hole_held_off_its_line": "参考边将此孔固定在一条直线上，该直线在所选点之前就离开了该面。请在直线经过该面的位置选取，或移除该边以自由移动孔。"
   }
 }

crates/ui-egui/src/dialogs.rs

@@ -771,7 +771,7 @@ impl Dialog {
     }
 
     /// Show why OK (or starting the sketch) was refused, for as long as the inputs stay as they are.
-    fn refuse(&mut self, app: &SolveApp, error: String) {
+    pub(crate) fn refuse(&mut self, app: &SolveApp, error: String) {
         self.refused = Some(Refused::now(app, self));
         self.error = Some(error);
     }

crates/ui-egui/src/hole_place.rs

@@ -240,36 +240,48 @@ pub(crate) fn move_place(d: &mut Dialog, s: &Session, i: usize, to: Sel) -> bool
     true
 }
 
+/// What a pick on a face with a hole did to that hole (see [`pick_moves`]).
+pub(crate) enum PickMove {
+    /// Not a move: the pick goes to the dialog as any pick does.
+    No,
+    Moved,
+    /// Spent, the hole left where it is (the dialog already says why, if anything does).
+    Spent,
+    /// Spent, the hole left where it is, and why.
+    Refused(&'static str),
+}
+
 /// Why a pick did not move a hole one reference edge holds: its line leaves the face first.
-pub(crate) const HELD_OFF_ITS_LINE: &str = "The reference edge holds this hole on a line that leaves the face before the picked point. Pick nearer the line, or take the edge out to move the hole freely.";
+pub(crate) const HELD_OFF_ITS_LINE: &str = "The reference edge holds this hole on a line that leaves the face before the picked point. Pick where the line crosses the face, or take the edge out to move the hole freely.";
 
 /// A Single hole's face place picked on a face that already has a hole moves that hole there
 /// (true), when holes move by clicking. A pick on the hole itself (within its radius) is left to
 /// take it out, as before. A hole held along one reference edge goes along it only, and the pick
 /// is spent either way (it never takes the hole out instead): when the line leaves the face before
-/// the pick, the dialog says so.
-pub(crate) fn pick_moves(d: &mut Dialog, s: &Session, sel: &Sel) -> bool {
-    let Kind::Hole { opts, diameter, .. } = &d.kind else { return false };
-    let Sel::Face { point, .. } = sel else { return false };
+/// the pick, it is refused with why.
+pub(crate) fn pick_moves(d: &mut Dialog, s: &Session, sel: &Sel) -> PickMove {
+    let Kind::Hole { opts, diameter, .. } = &d.kind else { return PickMove::No };
+    let Sel::Face { point, .. } = sel else { return PickMove::No };
     if opts.multiple || d.active != 0 {
-        return false;
+        return PickMove::No;
     }
     let radius = s.doc.eval(diameter, ValueKind::Length).ok().filter(|v| v.is_finite() && *v > 0.0).map_or(0.0, |v| v / 2.0);
     let group = crate::viewport::grouped_selection(s, sel);
-    let Some(inp) = d.inputs.first() else { return false };
-    let Some(i) = inp.items.iter().position(|x| group.iter().any(|g| same_item(x, g))) else { return false };
-    let Some(Sel::Face { point: was, .. }) = inp.items.get(i) else { return false };
+    let Some(inp) = d.inputs.first() else { return PickMove::No };
+    let Some(i) = inp.items.iter().position(|x| group.iter().any(|g| same_item(x, g))) else { return PickMove::No };
+    let Some(Sel::Face { point: was, .. }) = inp.items.get(i) else { return PickMove::No };
     if was.dist(*point) <= radius {
-        return false;
+        return PickMove::No;
     }
     match crate::hole_refs::freedom(s, d, i) {
-        crate::place_gizmo::Freedom::Plane => move_place(d, s, i, sel.clone()),
-        crate::place_gizmo::Freedom::Line(_) => {
-            if !crate::hole_refs::drag_to(s, d, i, *point) {
-                d.error = Some(HELD_OFF_ITS_LINE.to_string());
-            }
-            true
-        }
+        crate::place_gizmo::Freedom::Plane if move_place(d, s, i, sel.clone()) => PickMove::Moved,
+        crate::place_gizmo::Freedom::Plane => PickMove::No,
+        crate::place_gizmo::Freedom::Line(_) => match crate::hole_refs::drag_to(s, d, i, *point) {
+            Ok(()) => PickMove::Moved,
+            Err(crate::hole_refs::Stuck::OffFace) => PickMove::Refused(HELD_OFF_ITS_LINE),
+            // A distance that does not evaluate shows its own error.
+            Err(crate::hole_refs::Stuck::Not) => PickMove::Spent,
+        },
     }
 }
 

crates/ui-egui/src/hole_refs.rs

@@ -253,24 +253,34 @@ pub(crate) fn freedom(s: &Session, d: &Dialog, i: usize) -> Freedom {
     }
 }
 
+/// Why [`drag_to`] left the hole where it was.
+#[derive(Clone, Copy, Debug, PartialEq, Eq)]
+pub(crate) enum Stuck {
+    /// The line the edge holds the hole on leaves the face before the point asked for.
+    OffFace,
+    /// Anything else: not the hole the edges place, no frame, not exactly one edge, or a distance
+    /// that does not evaluate (which shows its own error).
+    Not,
+}
+
 /// Move the hole at item `i` of the face input toward `near` as far as its one reference edge
 /// lets it: onto the line at the typed distance from it, nearest `near`. The typed distance
-/// stays as it is, an expression included, so a drag never rewrites it. False when the hole
-/// cannot go there (off its face, no frame, not exactly one edge, or not the hole the edges place).
-pub(crate) fn drag_to(s: &Session, d: &mut Dialog, i: usize, near: Vec3) -> bool {
+/// stays as it is, an expression included, so a drag never rewrites it.
+pub(crate) fn drag_to(s: &Session, d: &mut Dialog, i: usize, near: Vec3) -> Result<(), Stuck> {
     if place_item(d) != Some(i) {
-        return false;
+        return Err(Stuck::Not);
     }
-    let Some((f, picked)) = frame(s, d) else { return false };
+    let Some((f, picked)) = frame(s, d) else { return Err(Stuck::Not) };
     if picked.len() != 1 {
-        return false;
+        return Err(Stuck::Not);
     }
-    let Kind::Hole { opts, .. } = &d.kind else { return false };
+    let Kind::Hole { opts, .. } = &d.kind else { return Err(Stuck::Not) };
     let values: Option<Vec<f64>> = picked.iter().map(|k| model_text(&opts.refs, *k).and_then(|e| s.doc.eval(e, ValueKind::Length).ok())).collect();
-    let Some(to) = values.and_then(|v| f.place(near, &v).ok()).filter(|c| on_reference_face(&crate::open_meshing::session_world(s), &f, *c)) else {
-        return false;
-    };
-    let Some(Sel::Face { point, .. }) = d.inputs.first_mut().and_then(|inp| inp.items.get_mut(i)) else { return false };
+    let Some(to) = values.and_then(|v| f.place(near, &v).ok()) else { return Err(Stuck::Not) };
+    if !on_reference_face(&crate::open_meshing::session_world(s), &f, to) {
+        return Err(Stuck::OffFace);
+    }
+    let Some(Sel::Face { point, .. }) = d.inputs.first_mut().and_then(|inp| inp.items.get_mut(i)) else { return Err(Stuck::Not) };
     *point = to;
     // In step already: the place moved by its distances, so there is nothing to measure again.
     let edges = edges(d);
@@ -281,7 +291,7 @@ pub(crate) fn drag_to(s: &Session, d: &mut Dialog, i: usize, near: Vec3) -> bool
         sn.edges = edges;
     }
     d.error = None;
-    true
+    Ok(())
 }
 
 /// Keep the distances and the hole's place in step: a new place or a new edge is measured, a

crates/ui-egui/src/place_gizmo/hole.rs

@@ -51,14 +51,22 @@ fn places(app: &SolveApp, d: &Dialog) -> Vec<Place> {
 
 /// The hole at `key` moved to the face place a click with `hits` would take (snapped as the
 /// pick is), unless that face already has another hole; held by a reference edge, as far along
-/// the edge as the pick is.
+/// the edge as the pick is, and where the edge's line leaves the face first, refused with why.
 fn move_to(app: &SolveApp, d: &mut Dialog, hits: &[Hit], key: usize) -> bool {
     let s = &app.session;
     let Some(to) = face_pick(d, s, hits) else { return false };
     match crate::hole_refs::freedom(s, d, key) {
         Freedom::Plane => crate::hole_place::move_place(d, s, key, to),
         Freedom::Line(_) => match to {
-            Sel::Face { point, .. } => crate::hole_refs::drag_to(s, d, key, point),
+            Sel::Face { point, .. } => match crate::hole_refs::drag_to(s, d, key, point) {
+                Ok(()) => true,
+                // As a pick there says (see `hole_place::pick_moves`), until the inputs change.
+                Err(crate::hole_refs::Stuck::OffFace) => {
+                    d.refuse(app, crate::hole_place::HELD_OFF_ITS_LINE.to_string());
+                    false
+                }
+                Err(crate::hole_refs::Stuck::Not) => false,
+            },
             _ => false,
         },
     }

crates/ui-egui/src/place_gizmo/hole_tests.rs

@@ -251,12 +251,11 @@ fn a_pick_elsewhere_on_the_face_keeps_a_held_hole_on_its_line() {
     assert!(matches!(p.as_slice(), [p] if (p.x - 10.0).abs() < 1e-6 && (p.y - 25.0).abs() < 0.3), "moved along the edge only: {p:?}");
 }
 
-/// On an L-shaped face the held hole's line can leave the face before the pick: the hole stays,
-/// and the dialog says why instead of taking the pick without a word.
-#[test]
-fn a_pick_the_held_line_cannot_reach_says_why_and_leaves_the_hole() {
+/// A hole at (10, 5) on the first block's top, held by its left edge, the top an L: a notch out of
+/// the back left corner, so along the edge from (30, 25) is the notch (the line x = 10 has no face
+/// at y = 25). The Face input is active again.
+fn held_on_an_l() -> Harness {
     let mut h = blocks();
-    // A notch out of the first block's back left corner: its top is an L.
     h.app.run("solid.box", json!({"length": 21, "width": 16, "height": 22, "corner": [-1, 15, -1], "operation": "cut"})).unwrap();
     h.settle();
     h.step(&json!({"start": "solid.hole"})).unwrap();
@@ -266,16 +265,59 @@ fn a_pick_the_held_line_cannot_reach_says_why_and_leaves_the_hole() {
     let left = screen(&mut h, Vec3::new(0.0, 7.0, 20.0));
     h.step(&json!({"click": left})).unwrap();
     h.settle();
-    let before = places(&h);
-    assert!(matches!(before.as_slice(), [p] if p.dist(Vec3::new(10.0, 5.0, 20.0)) < 0.3), "{before:?}");
+    assert!(matches!(places(&h).as_slice(), [p] if p.dist(Vec3::new(10.0, 5.0, 20.0)) < 0.3), "{:?}", places(&h));
     h.step(&json!({"click": {"handle": "dialog:Face"}})).unwrap();
-    // Along the edge from here is the notch: the line at x = 10 has no face at y = 25.
+    h
+}
+
+fn error(h: &Harness) -> Option<String> {
+    h.app.dialog.as_ref().and_then(|d| d.error.clone())
+}
+
+/// On an L-shaped face the held hole's line can leave the face before the pick: the hole stays,
+/// and the dialog says why (until the inputs change) instead of taking the pick without a word.
+#[test]
+fn a_pick_the_held_line_cannot_reach_says_why_and_leaves_the_hole() {
+    let mut h = held_on_an_l();
+    let before = places(&h);
     let at = screen(&mut h, Vec3::new(30.0, 25.0, 20.0));
     h.step(&json!({"click": at})).unwrap();
     h.settle();
     assert_eq!(places(&h), before, "the hole stays on its line");
-    let error = h.app.dialog.as_ref().and_then(|d| d.error.clone());
-    assert_eq!(error.as_deref(), Some(crate::hole_place::HELD_OFF_ITS_LINE), "the dialog says why");
+    assert_eq!(error(&h).as_deref(), Some(crate::hole_place::HELD_OFF_ITS_LINE), "the dialog says why");
+    // A typed distance changes what the dialog makes: the message gives way.
+    h.step(&json!({"click": {"handle": "dialog:Distance 1"}})).unwrap();
+    h.step(&json!({"key": "A", "cmd": true})).unwrap();
+    h.step(&json!({"text": "12"})).unwrap();
+    h.frames(3);
+    assert_eq!(error(&h), None, "a changed distance clears it");
+}
+
+/// The marker dragged where the held line cannot follow says the same.
+#[test]
+fn a_marker_drag_the_held_line_cannot_follow_says_why() {
+    let mut h = held_on_an_l();
+    let before = places(&h);
+    let to = screen(&mut h, Vec3::new(30.0, 25.0, 20.0));
+    h.step(&json!({"drag": [{"handle": "hole:0"}, to], "steps": 1})).unwrap();
+    h.settle();
+    assert_eq!(places(&h), before, "the hole stays on its line");
+    assert_eq!(error(&h).as_deref(), Some(crate::hole_place::HELD_OFF_ITS_LINE), "the dialog says why");
+}
+
+/// A distance that does not evaluate is its own error: a pick does not blame the face for it.
+#[test]
+fn a_pick_with_an_unreadable_distance_does_not_blame_the_face() {
+    let mut h = held_on_an_l();
+    h.step(&json!({"click": {"handle": "dialog:Distance 1"}})).unwrap();
+    h.step(&json!({"key": "A", "cmd": true})).unwrap();
+    h.step(&json!({"text": "nonsense"})).unwrap();
+    h.frames(3);
+    h.step(&json!({"click": {"handle": "dialog:Face"}})).unwrap();
+    let at = screen(&mut h, Vec3::new(30.0, 25.0, 20.0));
+    h.step(&json!({"click": at})).unwrap();
+    h.settle();
+    assert_ne!(error(&h).as_deref(), Some(crate::hole_place::HELD_OFF_ITS_LINE), "{:?}", error(&h));
 }
 
 #[test]

crates/ui-egui/src/viewport.rs

@@ -3399,10 +3399,14 @@ pub(crate) fn dialog_pick(app: &mut SolveApp, hit: &Hit, sel: Sel) {
     let sel = crate::dialogs_move::from_shown(app, sel);
     let Some(mut d) = app.dialog.take() else { return };
     // With holes moving by clicking (Preferences), a Single hole picked again on its face moves there.
-    if app.preferences.hole_moves && crate::hole_place::pick_moves(&mut d, &app.session, &sel) {
-        d.focus = true;
-    } else {
-        d.pick(&app.session, sel);
+    match if app.preferences.hole_moves { crate::hole_place::pick_moves(&mut d, &app.session, &sel) } else { crate::hole_place::PickMove::No } {
+        crate::hole_place::PickMove::No => d.pick(&app.session, sel),
+        crate::hole_place::PickMove::Moved | crate::hole_place::PickMove::Spent => d.focus = true,
+        // Shown until the inputs change, as a refused OK is.
+        crate::hole_place::PickMove::Refused(why) => {
+            d.focus = true;
+            d.refuse(app, why.to_string());
+        }
     }
     if let crate::dialogs::Kind::PathPlane { sketch, .. } = &mut d.kind {
         *sketch = match hit {