fix(keymap): register the fixed bindings before the config, not after

gpui resolves a keystroke by sorting the matches on context depth and then on
registration index, later winning:

    matched_bindings.sort_by(|(depth_a, ix_a, _), (depth_b, ix_b, _)| {
        depth_b.cmp(depth_a).then(ix_b.cmp(ix_a))
    });

`rebuild_keymap` added the config's bindings and then the fixed ones, so a
fixed binding won any tie. Six of the seven are scoped — `Terminal`,
`Switcher`, `Palette` — and win on depth whatever the order. The seventh is
global: `secondary-+`, the font-size step. A config that asked for that chord
got `IncreaseFontSize` instead, with no error, nothing in the keybindings UI,
and nothing in the log — which is the silence this tree has already gone out
of its way to remove from an unparseable chord, a clamped setting and an
unread config key.

Swapping the two lines fixes it and changes nothing else: depth is compared
before index, so the scoped six still win. And it matches what "fixed" was
ever for, which the comment on `fixed_bindings` states — they are not in
`effective_bindings`, so a rebuild replaying only the config would drop them.
That is an argument about existing, not about outranking.

Found by checking the reverse direction of the shortcuts page: every chord it
prints against every chord tty7 binds. Nothing was undocumented — `⌘ C`,
`⌘ V`, `⌃ R` and the arrow shorthands are keys tty7 answers without a keymap
action, which is why a strict guard there would be wrong — but `⌘ +` turned
out to be bound twice, and following that up is what surfaced the ordering.

CHANGELOG.md

@@ -217,6 +217,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   which is the wrong conclusion to reach while reading a log to work out when
   something happened.
 
+- **A keybinding you set for `⌘+` is no longer silently overridden.** gpui
+  resolves a keystroke by context depth and then by registration order, later
+  winning, and tty7's fixed bindings were registered after the config — so the
+  one *global* entry among them, the font-size step on `⌘+`, beat anything a
+  config asked for on that chord, with no error and nothing in the keybindings
+  UI to explain it. The fixed bindings are registered first now. The others are
+  unaffected: they are scoped to Terminal, Switcher and Palette, which are
+  deeper contexts than a config binding can name, and depth is compared before
+  order. What "fixed" was for is that they exist at all — they are not in the
+  config table, so a rebuild replaying only the config would drop them — not
+  that they outrank what somebody typed.
+
 - **A commit author's name draws on one row.** git refuses a control
   character in a *branch* name and accepts one in an author name without
   comment — `git -c user.name=$'Bad\rName' commit` succeeds and

src/ui/keymap.rs

@@ -80,8 +80,21 @@ fn rebuild_keymap(cx: &mut App) {
         .try_global::<BaseBindings>()
         .map(|base| base.0.clone())
         .unwrap_or_default();
-    bindings.extend(action_bindings(&effective));
+    // Fixed first, the config after it. gpui sorts a keystroke's matches by
+    // context depth and then by *index*, later winning — so adding these last
+    // let the one global entry among them, `secondary-+`, beat a config that
+    // asked for that chord. The user got `IncreaseFontSize` and no word about
+    // why, which is the same silence this tree already went to some trouble to
+    // remove from an unparseable chord and a clamped setting.
+    //
+    // The other fixed bindings are unaffected: `Terminal`, `Switcher` and
+    // `Palette` are deeper contexts than a config binding's, and depth is
+    // compared before index. What "fixed" was ever meant to buy them is that
+    // they *exist* — they are not in `effective_bindings`, so a rebuild that
+    // replayed only the config would drop them — not that they outrank what
+    // somebody typed.
     bindings.extend(fixed_bindings());
+    bindings.extend(action_bindings(&effective));
     cx.clear_key_bindings();
     cx.bind_keys(bindings);
 }
@@ -1511,6 +1524,42 @@ mod tests {
     #[cfg(not(target_os = "macos"))]
     const CTRL: &str = "Ctrl";
 
+    /// A fixed binding does not outrank the config.
+    ///
+    /// gpui resolves a keystroke by context depth first and then by index,
+    /// with the later-added binding winning. `fixed_bindings` were added after
+    /// the config, so the one *global* entry among them — `secondary-+` for
+    /// the font step — could not be rebound: a config asking for that chord
+    /// got `IncreaseFontSize` instead, silently. This tree has already gone to
+    /// some trouble to stop a keybinding being ignored without a word.
+    ///
+    /// The context-scoped ones are unaffected by the order, because depth is
+    /// compared first, and they need to win: a `tab` bound in `Terminal` or
+    /// `Switcher` is deeper than anything the config can express.
+    ///
+    /// Read from the source, because the property is the order of two lines
+    /// and exercising it would mean standing up a gpui `App` to press a key
+    /// into.
+    #[test]
+    fn the_fixed_bindings_are_registered_before_the_config_not_after() {
+        const SRC: &str = include_str!("keymap.rs");
+        let start = SRC
+            .find("fn rebuild_keymap(")
+            .expect("the keymap is rebuilt here");
+        let body = &SRC[start..];
+        let body = &body[..body.find("\n}\n").expect("the function ends")];
+
+        let fixed = body.find("bindings.extend(fixed_bindings())").expect("fixed");
+        let config = body
+            .find("bindings.extend(action_bindings(")
+            .expect("the config's own bindings");
+        assert!(
+            fixed < config,
+            "the fixed bindings are added after the config, so the global one \
+             among them shadows a chord the user asked for"
+        );
+    }
+
     #[test]
     fn key_tokens_maps_modifiers_to_glyphs() {
         assert_eq!(key_tokens("secondary-t"), vec![SECONDARY, "T"]);
@@ -2206,3 +2255,4 @@ mod gpui_tests {
         });
     }
 }
+