test(spreadsheet-ui): fix a wrong assertion in the new exchange test

origin/main was red: navigation_and_lifecycle_are_headless asserted that
after `exchange_sheet_data(0, &mut external)` the caller's buffer holds
"active". Nothing in the test ever writes that string.

`exchange_sheet_data` is a two-way `mem::swap`, so `external` comes back
with sheet 0's contents. Sheet 0 is empty at that point: "adapter" was
written while sheet 1 was active, and `remove_sheet(1)` then deleted the
sheet holding it.

Corrected to assert the empty string, and added the assertion the test
was missing -- that the sheet now holds "grid". Checking only one side of
a swap would pass for a function that merely cleared the buffer.

Verified pre-existing: fails identically on origin/main without my
commits.

.forgejo/workflows/nigig-build.yml

@@ -176,6 +176,33 @@ 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,6 +252,23 @@ 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
-                KeyEnter if self.drawing.is_drawing && !self.drawing.dde_buffer.is_empty() => {
+                ReturnKey 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
                 }
-                KeyEnter if self.drawing.is_drawing && self.drawing.tool == CadTool::Polyline => {
+                ReturnKey if self.drawing.is_drawing && self.drawing.tool == CadTool::Polyline => {
                     self.finish_polyline(cx);
                     true
                 }
-                KeyEnter if self.drawing.is_drawing && self.drawing.tool == CadTool::Polygon => {
+                ReturnKey if self.drawing.is_drawing && self.drawing.tool == CadTool::Polygon => {
                     self.finish_drawing(cx);
                     true
                 }
-                KeyEnter if self.drawing.is_drawing && self.drawing.tool == CadTool::Circle => {
+                ReturnKey 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
-                KeyBackspace if self.drawing.is_drawing && !self.drawing.dde_buffer.is_empty() => {
+                Backspace 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
-                KeyBackspace
+                Backspace
                     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
-                BracketLeft => {
+                LBracket => {
                     self.snap.snap_tolerance = (self.snap.snap_tolerance * 0.5).max(0.01);
                     self.area.redraw(cx);
                     cx.redraw_all();
                     true
                 }
-                BracketRight => {
+                RBracket => {
                     self.snap.snap_tolerance = (self.snap.snap_tolerance * 2.0).min(5.0);
                     self.area.redraw(cx);
                     cx.redraw_all();
@@ -7284,6 +7284,41 @@ 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',
-        Digit0 => '0',
-        Digit1 => '1',
-        Digit2 => '2',
-        Digit3 => '3',
-        Digit4 => '4',
-        Digit5 => '5',
-        Digit6 => '6',
-        Digit7 => '7',
-        Digit8 => '8',
-        Digit9 => '9',
+        Key0 => '0',
+        Key1 => '1',
+        Key2 => '2',
+        Key3 => '3',
+        Key4 => '4',
+        Key5 => '5',
+        Key6 => '6',
+        Key7 => '7',
+        Key8 => '8',
+        Key9 => '9',
         Space => ' ',
         Minus => '-',
-        Equal => '=',
-        LeftBracket => '[',
-        RightBracket => ']',
+        Equals => '=',
+        LBracket => '[',
+        RBracket => ']',
         Backslash => '\\',
         Semicolon => ';',
-        Apostrophe => '\'',
+        Quote => '\'',
         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',
-        Digit0 => '0',
-        Digit1 => '1',
-        Digit2 => '2',
-        Digit3 => '3',
-        Digit4 => '4',
-        Digit5 => '5',
-        Digit6 => '6',
-        Digit7 => '7',
-        Digit8 => '8',
-        Digit9 => '9',
+        Key0 => '0',
+        Key1 => '1',
+        Key2 => '2',
+        Key3 => '3',
+        Key4 => '4',
+        Key5 => '5',
+        Key6 => '6',
+        Key7 => '7',
+        Key8 => '8',
+        Key9 => '9',
         Space => ' ',
         Minus => '-',
-        Equal => '=',
-        LeftBracket => '[',
-        RightBracket => ']',
+        Equals => '=',
+        LBracket => '[',
+        RBracket => ']',
         Backslash => '\\',
         Semicolon => ';',
-        Apostrophe => '\'',
+        Quote => '\'',
         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',
-        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 => '.',
+        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 => '.',
         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',
-        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 => '.',
+        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 => '.',
         Slash => '/', _ => return None,
     };
     if is_shift {

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

@@ -160,26 +160,28 @@ 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 {
-        f.write_str(&self.to_string())
+        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)
     }
 }
 
@@ -208,8 +210,11 @@ impl Range {
         (min_r..=max_r).flat_map(move |r| (min_c..=max_c).map(move |c| (r, c)))
     }
 
-    pub fn to_string(&self) -> String {
-        format!("{}:{}", self.start, self.end)
+}
+
+impl fmt::Display for Range {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "{}:{}", self.start, self.end)
     }
 }
 
@@ -1216,6 +1221,43 @@ 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,10 +147,27 @@ 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), "active");
+        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!(!model.exchange_sheet_data(99, &mut external));
     }
 }

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

@@ -447,9 +447,11 @@ impl UserProjectPillRef {
     }
 
     pub fn selected(&self, actions: &Actions) -> Option<UserProjectPillAction> {
-        for action in actions.filter_widget_actions_cast::<UserProjectPillAction>(self.widget_uid()) {
-            return Some(action);
-        }
-        None
+        // `.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()
     }
 }