A keys row is drawn only if the builders asked for it (ssoe)
KeyRow::looked_up is private to key_row, whose keys() is the lookup pages() hands the builders; the reviewer's direct call is E0624. A new test asserts the drawn keys rows equal help_bindings(), which catches the remaining self::keys bypass. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
@@ -1088,3 +1088,24 @@ failed: ex_usage_is_placeholders("q or press Q")`. The rule cost one row:
|
||||
`:add-items <cat> a b c ...` had three bare words and is now
|
||||
`:add-items <cat> <item> ...`. This test pins the rule the macro asks, in both
|
||||
directions, including that shipped usage and the rival.
|
||||
|
||||
### help::tests::every_keys_row_drawn_is_one_the_builders_asked_for
|
||||
|
||||
improvise-ssoe, found in re-review of #180 at 544744c. `KeyRow` closed
|
||||
hand-typed keys, but `KeyRow::looked_up` was `pub(super)`, so a page builder
|
||||
could call it directly instead of through its `keys` parameter — and
|
||||
`help_bindings()` records only what goes through the parameter. Injected into
|
||||
`editing()`: `KeyRow::looked_up(n, "clear-cell", None, "Edit the current
|
||||
cell")`, which captions `x` as editing the cell — the exact lie `jhdf`'s test
|
||||
exists for — and the whole workspace stayed green, because both guards that
|
||||
would object read `help_bindings()`.
|
||||
|
||||
Two changes. `looked_up` is now private to the `key_row` module, whose one
|
||||
caller is the module's own `keys` — the lookup `pages()` hands every builder —
|
||||
so the reviewer's injection is `E0624: associated function looked_up is
|
||||
private`. That narrows the door without closing it: a builder can still reach
|
||||
the module-level `keys` by path (`self::keys(..)`) rather than through its
|
||||
shadowing parameter. So this test states the premise directly — the number of
|
||||
keys rows drawn equals the number of rows the builders asked for (every asked
|
||||
row finds its keys, by `every_keys_row_the_pages_ask_for_finds_its_keys`) —
|
||||
and the `self::keys` bypass fails it `51 != 50`.
|
||||
|
||||
@@ -324,11 +324,12 @@ pub struct HelpPage {
|
||||
}
|
||||
|
||||
pub use key_row::KeyRow;
|
||||
use key_row::keys;
|
||||
|
||||
/// The one door to a keys row: its fields are private to this module, and its
|
||||
/// only constructor looks the keys up.
|
||||
mod key_row {
|
||||
use super::{ModeKey, key_label, keys_for, prefixed_labels};
|
||||
use super::{HelpLine, ModeKey, key_label, keys_for, prefixed_labels};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct KeyRow {
|
||||
@@ -347,7 +348,7 @@ mod key_row {
|
||||
|
||||
/// An unbound command yields no keys and no row, so the page never
|
||||
/// advertises a key that does not exist.
|
||||
pub(super) fn looked_up(
|
||||
fn looked_up(
|
||||
mode: &ModeKey,
|
||||
command: &str,
|
||||
args: Option<&[String]>,
|
||||
@@ -361,17 +362,18 @@ mod key_row {
|
||||
(!keys.is_empty()).then_some(Self { keys, description })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// A keys row, looked up in `mode`.
|
||||
fn keys(
|
||||
mode: &ModeKey,
|
||||
command: &'static str,
|
||||
args: Option<&[&str]>,
|
||||
description: &'static str,
|
||||
) -> Option<HelpLine> {
|
||||
let args: Option<Vec<String>> = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
|
||||
KeyRow::looked_up(mode, command, args.as_deref(), description).map(HelpLine::Keys)
|
||||
/// A keys row, looked up in `mode`: the one caller of [`KeyRow::looked_up`],
|
||||
/// and the lookup `pages` hands every builder (improvise-ssoe).
|
||||
pub(super) fn keys(
|
||||
mode: &ModeKey,
|
||||
command: &'static str,
|
||||
args: Option<&[&str]>,
|
||||
description: &'static str,
|
||||
) -> Option<HelpLine> {
|
||||
let args: Option<Vec<String>> = args.map(|a| a.iter().map(|s| (*s).to_string()).collect());
|
||||
KeyRow::looked_up(mode, command, args.as_deref(), description).map(HelpLine::Keys)
|
||||
}
|
||||
}
|
||||
|
||||
/// `gg`, `yy`, `ZZ`: a prefix key and the key after it, both off the keymap.
|
||||
@@ -1021,6 +1023,18 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-ssoe: a keys row drawn but never asked for through the builders'
|
||||
/// parameter is invisible to every guard. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
fn every_keys_row_drawn_is_one_the_builders_asked_for() {
|
||||
let drawn = pages()
|
||||
.iter()
|
||||
.flat_map(|page| page.lines.iter())
|
||||
.filter(|line| matches!(line, HelpLine::Keys(_)))
|
||||
.count();
|
||||
assert_eq!(drawn, help_bindings().len());
|
||||
}
|
||||
|
||||
/// A row whose lookup finds no key is dropped silently, so a broken lookup
|
||||
/// looks like a shorter page. See `context/test-evidence/protocol.md`.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user