coderabbit review

ARCHITECTURE.md

@@ -1314,10 +1314,10 @@ The global allocator is `mimalloc` (v3), chosen for its low-latency multi-thread
 | `check_and_run_popup` | `src/bin/main.rs:131` | Check popup conditions, dispatch to popup::run_with |
 | `sk_main` | `src/bin/main.rs:144` | CLI orchestration + output printing |
 | `SkimEvent` | `src/binds.rs:25` | `change`/`start`/`load`/`result`/`focus`/`zero`/`one` synthetic events → reserved `KeyEvent` |
-| `parse_key` | `src/binds.rs:213` | `"ctrl-a"` → `KeyEvent` |
-| `parse_action_binds` | `src/binds.rs:299` | `"reload:first"`, `"act-up:suppress+down"` → action follow-up map |
-| `parse_action_chain` | `src/binds.rs:333` | `"down+select"` → `Vec<Action>` |
-| `Action::name` | `src/tui/event.rs:316` | `Action` → canonical bind name (reverse of `parse_action`) |
+| `parse_key` | `src/binds.rs:218` | `"ctrl-a"` → `KeyEvent` |
+| `parse_action_binds` | `src/binds.rs:324` | `"reload:first"`, `"act-up:suppress+down"` → action follow-up map |
+| `parse_action_chain` | `src/binds.rs:358` | `"down+select"` → `Vec<Action>` |
+| `Action::name` | `src/tui/event.rs:320` | `Action` → canonical bind name (reverse of `parse_action`) |
 | `Matcher::create_engine_factory_with_builder` | `src/matcher.rs:189` | Build engine factory chain from options |
 | `ExactOrFuzzyEngineFactory::create_engine_with_case` | `src/engine/factory.rs:93` | Parse query prefixes, build engine |
 | `AndOrEngineFactory::parse_andor` | `src/engine/factory.rs:176` | Split query into AND/OR tree |

src/binds.rs

@@ -17,10 +17,11 @@ use crate::tui::event::{self, Action};
 /// The keymap is keyed by crossterm's [`KeyEvent`], which cannot express
 /// "the query changed" or "reading finished" directly. Each variant is
 /// therefore represented *transparently* as a reserved function-key code in the
-/// high-`F` range (`F(253)`–`F(255)`) that no real terminal ever emits. Giving
-/// these reserved codes named variants keeps them in one place instead of
-/// scattering magic `F(255)` literals across the codebase, and lets
-/// [`parse_key`] accept the friendly names `start`, `load` and `change`.
+/// high-`F` range (`F(249)`–`F(255)`) that no real terminal ever emits. The
+/// seven variants are `change`, `start`, `load`, `result`, `focus`, `zero`, and
+/// `one`. Giving these reserved codes named variants keeps them in one place
+/// instead of scattering magic function-key literals across the codebase, and
+/// lets [`parse_key`] accept every friendly event name.
 #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
 pub enum SkimEvent {
     /// Fired once, when skim has started up and entered its event loop.
@@ -210,7 +211,11 @@ pub fn get_default_key_map() -> KeyMap {
     KeyMap(ret)
 }
 
-/// Parses a key str into a crossterm `KeyEvent`
+/// Parses a key str into a crossterm `KeyEvent`.
+///
+/// In addition to keyboard names, accepts all seven names recognized by
+/// [`SkimEvent::from_name`]: `change`, `start`, `load`, `result`, `focus`,
+/// `zero`, and `one`.
 ///
 /// # Errors
 /// Returns an error if the key string is empty, contains an unknown modifier,

src/manpage.rs

@@ -121,24 +121,30 @@ const KEYS_SS: &str = "
 * alt-shift-right
 * any single character
 ";
-const BINDABLE_EVENTS_SS: &str = "
-* change: the query changes
-* start: skim enters its event loop; fired once
-* load: the reader and matcher finish consuming the current input; fired once per read, including reloads
-* result: filtering for the current query completes
-* focus: the focused item changes because of cursor movement or a result update
-* zero: a completed search has no matches
-* one: a completed search has exactly one match
-";
+const BINDABLE_EVENTS_SS: &str = concat!(
+    "\n",
+    "* change: the query changes\n",
+    "* start: skim enters its event loop; fired once\n",
+    "* load: the reader and matcher finish consuming the current input; ",
+    "fired once per read, including reloads\n",
+    "* result: filtering for the current query completes\n",
+    "* focus: the focused item changes because of cursor movement or a result update\n",
+    "* zero: a completed search has no matches\n",
+    "* one: a completed search has exactly one match\n",
+);
 
-const ACTION_BINDINGS_SS: &str = "
-Actions can also be used as binding triggers. A follow-up chain bound to an action name runs immediately after that action. Use the `act-` prefix for action triggers; it is recommended to avoid ambiguity and required when the action name is also a key, for example `act-up:last`.
+const ACTION_BINDINGS_SS: &str = concat!(
+    "\n",
+    "Actions can also be used as binding triggers. A follow-up chain bound to an action name runs immediately ",
+    "after that action. Use the `act-` prefix for action triggers; it is recommended to avoid ambiguity and ",
+    "required when the action name is also a key, for example `act-up:last`.\n\n",
+    "Follow-up chains use non-recursive (`noremap`) semantics: their actions do not trigger further action ",
+    "bindings. Add `suppress` to skip the triggering action's default behavior, for example ",
+    "`act-up:suppress+down`.\n",
+);
 
-Follow-up chains use non-recursive (`noremap`) semantics: their actions do not trigger further action bindings. Add `suppress` to skip the triggering action's default behavior, for example `act-up:suppress+down`.
-";
-
-const ACTIONS_SS: &str = "
-* abort: ctrl-c  ctrl-q  esc
+const ACTIONS_SS: &str = concat!(
+    "\n* abort: ctrl-c  ctrl-q  esc
 * accept(...): enter *the argument will be printed when the binding is triggered*
 * append-and-select
 * backward-char: ctrl-b  left
@@ -184,8 +190,10 @@ const ACTIONS_SS: &str = "
 * select-row
 * set-preview-cmd(...): *arg will be a expanded expression, see COMMAND EXPANSION for details
 * set-query(...): *arg will be a expanded expression, see COMMAND EXPANSION for details
-* suppress: *if bound to an action (e.g. `act-up:suppress`), suppresses that action's default behavior so the rest of the non-recursive chain runs once in its place; if bound to a key, equivalent to `ignore`
-* toggle
+",
+    "* suppress: *if bound to an action (e.g. `act-up:suppress`), suppresses that action's default behavior ",
+    "so the rest of the non-recursive chain runs once in its place; if bound to a key, equivalent to `ignore`\n",
+    "* toggle
 * toggle-all
 * toggle+down: ctrl-i  tab
 * toggle-in: (--layout=reverse ? toggle+up:  toggle+down)
@@ -201,7 +209,8 @@ const ACTIONS_SS: &str = "
 * unix-word-rubout: ctrl-w
 * up: ctrl-k  ctrl-p  up
 * yank: ctrl-y
-";
+",
+);
 
 #[cfg(feature = "listen")]
 const REMOTE_SECTION: &str = "
@@ -301,10 +310,13 @@ Exact search can be enabled by default by the `--exact` command-line flag. In ex
     section(
         &mut custom,
         "KEYBINDS",
-        "
-Bindings can be set by the `--bind` option, which takes a comma-separated list of `<trigger>:<action>[+action2]` expressions. A trigger can be a key, a finder event, or an action name.
-Actions can take arguments, specified either between parentheses `reload(ls)` or after a colon `reload:ls`.
-",
+        concat!(
+            "\nBindings can be set by the `--bind` option, which takes a comma-separated list of ",
+            "`<trigger>:<action>[+action2]` expressions. A trigger can be a key, a finder event, or an action ",
+            "name.\n",
+            "Actions can take arguments, specified either between parentheses `reload(ls)` or after a colon ",
+            "`reload:ls`.\n",
+        ),
     );
     subsection(&mut custom, "Available keys (aliases in parentheses)", KEYS_SS);
     subsection(&mut custom, "Bindable finder events", BINDABLE_EVENTS_SS);

src/tui/app_tests.rs

@@ -80,6 +80,11 @@ fn add_char_updates_query_and_emits_events() {
     let events = act(&mut app, Action::AddChar('x'));
     assert_eq!(app.input.value, "x");
     // on_query_changed emits a `change` event key (SkimEvent::Change) and a RunPreview
+    assert!(
+        events
+            .iter()
+            .any(|event| matches!(event, Event::Key(key) if *key == crate::binds::SkimEvent::Change.key_event()))
+    );
     assert!(events.iter().any(|e| matches!(e, Event::RunPreview)));
 }