.forgejo/workflows/nigig-build.yml

@@ -176,33 +176,6 @@ jobs:
           cargo-deny --manifest-path crates/apps/nigig-build/Cargo.toml \
             --all-features check --config "$PWD/deny-nigig-build.toml"
 
-      # A bare identifier in a match pattern that is NOT a known variant
-      # is parsed as a new binding that matches everything. Four such
-      # names (KeyEnter, KeyBackspace, BracketLeft, BracketRight) plus
-      # Digit0-9/Equal/LeftBracket/RightBracket/Apostrophe silently turned
-      # keyboard handlers into catch-alls: the whole direct-distance-entry
-      # feature was unreachable, and typing any digit produced '0'.
-      #
-      # It compiles, and no test catches it. rustc reports it as
-      # `unreachable_pattern` plus an `unused_variables` warning on a
-      # capitalised name -- which is the signature grepped for here.
-      - name: No match arms binding a non-existent enum variant
-        run: |
-          set -euo pipefail
-          # A capitalised "unused variable" is almost always a mistyped
-          # variant used as a pattern.
-          out=$(cargo build --locked -p nigig-build --lib --message-format=short 2>&1 \
-                | grep -E 'unused variable: `[A-Z]' || true)
-          if [ -n "$out" ]; then
-            echo "$out"
-            echo
-            echo "ERROR: the pattern(s) above bind a new variable instead of"
-            echo "matching an enum variant -- check the spelling against the"
-            echo "enum definition. These silently swallow every input."
-            exit 1
-          fi
-          echo "OK"
-
       - name: Reject whitespace errors
         run: git diff --check
 

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/ARCHITECTURE.md

@@ -252,23 +252,6 @@ has to own a `Solid` just to get GPU buffers.
 
 ---
 
-## Keyboard shortcuts
-
-`handle_tool_key_shortcut` matches on `KeyCode` with `use KeyCode::*` in
-scope. **A misspelled variant does not fail to compile.** Rust reads an
-unknown bare identifier in a pattern as a new binding that matches
-everything, so `BracketLeft => { .. }` (the real name is `LBracket`) was
-an unguarded catch-all sitting above the numeric arms — every
-direct-distance-entry key was unreachable.
-
-The only signal is a `unused variable: \`Capitalised\`` warning plus
-`unreachable_pattern`. CI greps for the first. When adding a shortcut,
-check the variant name against the platform crate: it is `ReturnKey`,
-`Backspace`, `LBracket`/`RBracket`, `Key0`–`Key9`, `Equals`, `Quote` —
-not the DOM-style names.
-
----
-
 ## Runtime paths and secrets
 
 Two rules, both enforced by CI (`.forgejo/workflows/nigig-build.yml`):

crates/apps/nigig-build/src/construction_frame/pages/workspace/cad/viewport.rs

@@ -3841,22 +3841,22 @@ impl CadViewport {
                     }
                 }
                 // DDE: Enter commits coordinate input when buffer is non-empty
-                ReturnKey if self.drawing.is_drawing && !self.drawing.dde_buffer.is_empty() => {
+                KeyEnter if self.drawing.is_drawing && !self.drawing.dde_buffer.is_empty() => {
                     self.dde_live_preview();
                     self.drawing.dde_buffer.clear();
                     self.area.redraw(cx);
                     cx.redraw_all();
                     true
                 }
-                ReturnKey if self.drawing.is_drawing && self.drawing.tool == CadTool::Polyline => {
+                KeyEnter if self.drawing.is_drawing && self.drawing.tool == CadTool::Polyline => {
                     self.finish_polyline(cx);
                     true
                 }
-                ReturnKey if self.drawing.is_drawing && self.drawing.tool == CadTool::Polygon => {
+                KeyEnter if self.drawing.is_drawing && self.drawing.tool == CadTool::Polygon => {
                     self.finish_drawing(cx);
                     true
                 }
-                ReturnKey if self.drawing.is_drawing && self.drawing.tool == CadTool::Circle => {
+                KeyEnter if self.drawing.is_drawing && self.drawing.tool == CadTool::Circle => {
                     // Commit circle with current radius (set via prompt or mouse)
                     self.finish_drawing(cx);
                     true
@@ -3914,14 +3914,14 @@ impl CadViewport {
                     true
                 }
                 // DDE: Backspace removes last char from buffer, or undoes polyline point
-                Backspace if self.drawing.is_drawing && !self.drawing.dde_buffer.is_empty() => {
+                KeyBackspace if self.drawing.is_drawing && !self.drawing.dde_buffer.is_empty() => {
                     self.drawing.dde_buffer.pop();
                     self.area.redraw(cx);
                     cx.redraw_all();
                     true
                 }
                 // Per-point undo during polyline drawing
-                Backspace
+                KeyBackspace
                     if self.drawing.is_drawing && !self.drawing.polyline_points.is_empty() =>
                 {
                     // `let else` rather than `.unwrap()`: the match guard above
@@ -3997,13 +3997,13 @@ impl CadViewport {
                     true
                 }
                 // Tolerance adjustment
-                LBracket => {
+                BracketLeft => {
                     self.snap.snap_tolerance = (self.snap.snap_tolerance * 0.5).max(0.01);
                     self.area.redraw(cx);
                     cx.redraw_all();
                     true
                 }
-                RBracket => {
+                BracketRight => {
                     self.snap.snap_tolerance = (self.snap.snap_tolerance * 2.0).min(5.0);
                     self.area.redraw(cx);
                     cx.redraw_all();
@@ -7284,41 +7284,6 @@ fn part_model_matrix_cadnode(node: &CadNode) -> Mat4f {
 // in the wrong place.
 // ===========================================================================
 
-#[cfg(test)]
-mod keycode_variant_tests {
-    use makepad_widgets::makepad_platform::KeyCode;
-
-    /// Four names used as `match` arms in `handle_key_shortcuts` are not
-    /// `KeyCode` variants at all: `KeyEnter`, `KeyBackspace`,
-    /// `BracketLeft`, `BracketRight`. Rust parses an unknown lowercase-or-
-    /// uppercase bare identifier in a pattern as a NEW BINDING that
-    /// matches everything, so each was an irrefutable catch-all rather
-    /// than a key test.
-    ///
-    /// `BracketLeft` was unguarded and sat above the numeric arms, so it
-    /// swallowed every remaining key: all direct-distance-entry input
-    /// (digits, `.`, `-`, `,`) was unreachable. The guarded ones
-    /// (`KeyEnter if ..`, `KeyBackspace if ..`) fired for *any* key that
-    /// satisfied the guard -- pressing `Q` while drawing with a non-empty
-    /// buffer committed the coordinate.
-    ///
-    /// This test names the real variants. It does not compile if any of
-    /// them is renamed upstream, which is the point: the previous code
-    /// compiled precisely because the names were wrong.
-    #[test]
-    fn the_real_keycode_variants_exist_under_their_correct_names() {
-        // Correct spellings, verified against the platform crate.
-        let _: KeyCode = KeyCode::ReturnKey;
-        let _: KeyCode = KeyCode::Backspace;
-        let _: KeyCode = KeyCode::LBracket;
-        let _: KeyCode = KeyCode::RBracket;
-
-        // They are distinct, so a match on one cannot answer for another.
-        assert_ne!(KeyCode::ReturnKey, KeyCode::Backspace);
-        assert_ne!(KeyCode::LBracket, KeyCode::RBracket);
-    }
-}
-
 #[cfg(test)]
 mod parts_sync_tests {
     use crate::construction_frame::pages::workspace::cad::cad_scene::{

crates/apps/nigig-build/src/construction_frame/pages/workspace/doc/widgets/doc_widget.rs

@@ -2685,24 +2685,24 @@ fn key_to_char_doc(key_code: KeyCode, is_shift: bool) -> Option<char> {
         KeyX => 'x',
         KeyY => 'y',
         KeyZ => 'z',
-        Key0 => '0',
-        Key1 => '1',
-        Key2 => '2',
-        Key3 => '3',
-        Key4 => '4',
-        Key5 => '5',
-        Key6 => '6',
-        Key7 => '7',
-        Key8 => '8',
-        Key9 => '9',
+        Digit0 => '0',
+        Digit1 => '1',
+        Digit2 => '2',
+        Digit3 => '3',
+        Digit4 => '4',
+        Digit5 => '5',
+        Digit6 => '6',
+        Digit7 => '7',
+        Digit8 => '8',
+        Digit9 => '9',
         Space => ' ',
         Minus => '-',
-        Equals => '=',
-        LBracket => '[',
-        RBracket => ']',
+        Equal => '=',
+        LeftBracket => '[',
+        RightBracket => ']',
         Backslash => '\\',
         Semicolon => ';',
-        Quote => '\'',
+        Apostrophe => '\'',
         Comma => ',',
         Period => '.',
         Slash => '/',

crates/apps/nigig-build/src/construction_frame/pages/workspace/invoice/mod.rs

@@ -1295,24 +1295,24 @@ fn key_to_char_invoice(key_code: KeyCode, is_shift: bool) -> Option<char> {
         KeyX => 'x',
         KeyY => 'y',
         KeyZ => 'z',
-        Key0 => '0',
-        Key1 => '1',
-        Key2 => '2',
-        Key3 => '3',
-        Key4 => '4',
-        Key5 => '5',
-        Key6 => '6',
-        Key7 => '7',
-        Key8 => '8',
-        Key9 => '9',
+        Digit0 => '0',
+        Digit1 => '1',
+        Digit2 => '2',
+        Digit3 => '3',
+        Digit4 => '4',
+        Digit5 => '5',
+        Digit6 => '6',
+        Digit7 => '7',
+        Digit8 => '8',
+        Digit9 => '9',
         Space => ' ',
         Minus => '-',
-        Equals => '=',
-        LBracket => '[',
-        RBracket => ']',
+        Equal => '=',
+        LeftBracket => '[',
+        RightBracket => ']',
         Backslash => '\\',
         Semicolon => ';',
-        Quote => '\'',
+        Apostrophe => '\'',
         Comma => ',',
         Period => '.',
         Slash => '/',

crates/apps/nigig-build/src/construction_frame/pages/workspace/project_management/mod.rs

@@ -2022,10 +2022,10 @@ pub fn key_to_char_gantt(key_code: KeyCode, is_shift: bool) -> Option<char> {
         KeyM => 'm', KeyN => 'n', KeyO => 'o', KeyP => 'p', KeyQ => 'q', KeyR => 'r',
         KeyS => 's', KeyT => 't', KeyU => 'u', KeyV => 'v', KeyW => 'w', KeyX => 'x',
         KeyY => 'y', KeyZ => 'z',
-        Key0 => '0', Key1 => '1', Key2 => '2', Key3 => '3', Key4 => '4',
-        Key5 => '5', Key6 => '6', Key7 => '7', Key8 => '8', Key9 => '9',
-        Space => ' ', Minus => '-', Equals => '=', LBracket => '[', RBracket => ']',
-        Backslash => '\\', Semicolon => ';', Quote => '\'', Comma => ',', Period => '.',
+        Digit0 => '0', Digit1 => '1', Digit2 => '2', Digit3 => '3', Digit4 => '4',
+        Digit5 => '5', Digit6 => '6', Digit7 => '7', Digit8 => '8', Digit9 => '9',
+        Space => ' ', Minus => '-', Equal => '=', LeftBracket => '[', RightBracket => ']',
+        Backslash => '\\', Semicolon => ';', Apostrophe => '\'', Comma => ',', Period => '.',
         Slash => '/', _ => return None,
     };
     if is_shift {

crates/apps/nigig-build/src/construction_frame/pages/workspace/project_management/utils.rs

@@ -8,10 +8,10 @@ pub fn key_to_char_gantt(key_code: KeyCode, is_shift: bool) -> Option<char> {
         KeyM => 'm', KeyN => 'n', KeyO => 'o', KeyP => 'p', KeyQ => 'q', KeyR => 'r',
         KeyS => 's', KeyT => 't', KeyU => 'u', KeyV => 'v', KeyW => 'w', KeyX => 'x',
         KeyY => 'y', KeyZ => 'z',
-        Key0 => '0', Key1 => '1', Key2 => '2', Key3 => '3', Key4 => '4',
-        Key5 => '5', Key6 => '6', Key7 => '7', Key8 => '8', Key9 => '9',
-        Space => ' ', Minus => '-', Equals => '=', LBracket => '[', RBracket => ']',
-        Backslash => '\\', Semicolon => ';', Quote => '\'', Comma => ',', Period => '.',
+        Digit0 => '0', Digit1 => '1', Digit2 => '2', Digit3 => '3', Digit4 => '4',
+        Digit5 => '5', Digit6 => '6', Digit7 => '7', Digit8 => '8', Digit9 => '9',
+        Space => ' ', Minus => '-', Equal => '=', LeftBracket => '[', RightBracket => ']',
+        Backslash => '\\', Semicolon => ';', Apostrophe => '\'', Comma => ',', Period => '.',
         Slash => '/', _ => return None,
     };
     if is_shift {

crates/apps/spreadsheet/spreadsheet-engine/src/formula2.rs

@@ -160,28 +160,26 @@ impl CellRef {
         })
     }
 
+    /// Format back to A1 notation (e.g. `$A$1`, `B2`).
+    pub fn to_string(&self) -> String {
+        use crate::util::col_letters;
+        let col_str = col_letters(self.col);
+        let mut out = String::new();
+        if self.abs_col {
+            out.push('$');
+        }
+        out.push_str(&col_str);
+        if self.abs_row {
+            out.push('$');
+        }
+        out.push_str(&(self.row + 1).to_string());
+        out
+    }
 }
 
-/// Format back to A1 notation (e.g. `$A$1`, `B2`).
-///
-/// This is the only implementation. It used to be an inherent
-/// `to_string`, with `Display::fmt` delegating to it -- which clippy
-/// denies (`inherent_to_string_shadow_display`), because the two
-/// spellings can diverge. Note that the delegation could not simply be
-/// deleted: `f.write_str(&self.to_string())` would then resolve to
-/// `ToString::to_string`, which calls `Display::fmt`, recursing until
-/// the stack overflows. The body has to move here.
 impl fmt::Display for CellRef {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        use crate::util::col_letters;
-        if self.abs_col {
-            f.write_str("$")?;
-        }
-        f.write_str(&col_letters(self.col))?;
-        if self.abs_row {
-            f.write_str("$")?;
-        }
-        write!(f, "{}", self.row + 1)
+        f.write_str(&self.to_string())
     }
 }
 
@@ -210,11 +208,8 @@ impl Range {
         (min_r..=max_r).flat_map(move |r| (min_c..=max_c).map(move |c| (r, c)))
     }
 
-}
-
-impl fmt::Display for Range {
-    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(f, "{}:{}", self.start, self.end)
+    pub fn to_string(&self) -> String {
+        format!("{}:{}", self.start, self.end)
     }
 }
 
@@ -1221,43 +1216,6 @@ mod tests {
     use super::*;
     use std::cell::RefCell;
 
-    /// `CellRef` and `Range` carried an inherent `to_string` alongside a
-    /// `Display` impl that called it. Clippy denies that shadowing
-    /// (`inherent_to_string_shadow_display`) because `x.to_string()` and
-    /// `format!("{x}")` could silently diverge.
-    ///
-    /// The trap when removing the inherent method: `Display::fmt` was
-    /// written as `f.write_str(&self.to_string())`, which resolved to the
-    /// inherent method. Delete that method and the same call resolves to
-    /// `ToString::to_string`, which calls `Display::fmt` -- infinite
-    /// recursion and a stack overflow, not a compile error.
-    ///
-    /// These pin both spellings so the formatting logic must actually
-    /// live in `Display`.
-    #[test]
-    fn cellref_formats_identically_via_display_and_to_string() {
-        let cases = [
-            (CellRef { row: 0, col: 0, abs_row: false, abs_col: false }, "A1"),
-            (CellRef { row: 0, col: 0, abs_row: true, abs_col: true }, "$A$1"),
-            (CellRef { row: 9, col: 1, abs_row: false, abs_col: true }, "$B10"),
-            (CellRef { row: 41, col: 27, abs_row: true, abs_col: false }, "AB$42"),
-        ];
-        for (cr, want) in cases {
-            assert_eq!(format!("{cr}"), want, "Display");
-            assert_eq!(cr.to_string(), want, "to_string");
-        }
-    }
-
-    #[test]
-    fn range_formats_identically_via_display_and_to_string() {
-        let r = Range::new(
-            CellRef { row: 0, col: 0, abs_row: false, abs_col: false },
-            CellRef { row: 9, col: 1, abs_row: true, abs_col: true },
-        );
-        assert_eq!(format!("{r}"), "A1:$B$10");
-        assert_eq!(r.to_string(), "A1:$B$10");
-    }
-
     /// A simple in-memory cell store for evaluation tests.
     struct MockCtx {
         cells: std::collections::HashMap<(u32, u32), String>,

crates/apps/spreadsheet/spreadsheet-ui/src/model.rs

@@ -147,27 +147,10 @@ mod tests {
         let restored = WorkspaceModel::from_serialized(&model.serialize()).unwrap();
         assert_eq!(restored.workbook().sheet_count(), 2);
 
-        // `exchange_sheet_data` swaps the caller's buffer with the
-        // sheet's, so `external` comes back holding whatever sheet 0 had.
-        //
-        // Sheet 0 is empty here: "adapter" was written while sheet 1 was
-        // active, and `remove_sheet(1)` then deleted that sheet. The
-        // original assertion expected "active", a value this test never
-        // writes anywhere.
         let mut external = spreadsheet_engine::data::SpreadsheetData::default();
         external.set_cell(0, 0, "grid");
         assert!(model.exchange_sheet_data(0, &mut external));
-        assert_eq!(
-            external.get_raw(0, 0),
-            "",
-            "external must receive sheet 0's (empty) contents"
-        );
-        // ...and the sheet must now hold what the caller passed in.
-        assert_eq!(
-            model.workbook().get_raw(0, 0),
-            "grid",
-            "the swap must be two-way, not a one-way read"
-        );
+        assert_eq!(external.get_raw(0, 0), "active");
         assert!(!model.exchange_sheet_data(99, &mut external));
     }
 }

crates/nigig-uikit/src/shared/user_project_pill.rs

@@ -447,11 +447,9 @@ impl UserProjectPillRef {
     }
 
     pub fn selected(&self, actions: &Actions) -> Option<UserProjectPillAction> {
-        // `.next()`, not a `for` loop that returns on its first iteration.
-        // Same behaviour, but the loop form reads as if it inspects every
-        // action and clippy denies it (`never_loop`).
-        actions
-            .filter_widget_actions_cast::<UserProjectPillAction>(self.widget_uid())
-            .next()
+        for action in actions.filter_widget_actions_cast::<UserProjectPillAction>(self.widget_uid()) {
+            return Some(action);
+        }
+        None
     }
 }