Mark the help rows a browser cannot run, and guard it from one list
improvise-uln5. The browser painted every help-overlay row plainly,
including six whose keys go upstream to a host that answers nothing:
/, n, H, z, Enter, and search mode's Enter/Esc. Every ex-command row was
painted plainly too, although `:` opens a command line the browser
cannot run. A dead key is worse than a gap: the keys typed after it land
on live bindings (improvise-koh6).
- resolve::NEEDS_THE_HOST is now the one list of names this client has
no arm for. KeyRow keeps the command it was built for.
- The browser's RenderHelpRow draws a listed row, and every ex row while
the command line is stranded, as `dim` with "(not in the browser yet)".
It marks rather than omits: both front ends keep drawing the same rows,
the feature stays findable, and web/ needs no change.
- the_command_names_that_still_need_the_host_are_these now drives that
list rather than a table of its own.
- every_help_row_the_browser_cannot_run_is_one_it_marks holds it from
the other side: silent help rows == listed rows, and no hint line
names one.
- every_advertised_command_answers_in_its_own_mode no longer restates
the black holes.
Red: a_help_row_the_browser_cannot_run_is_drawn_marked at 202e7eac.
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:
+35
-3
@@ -578,11 +578,43 @@ immutable `CmdContext`; the commands are then folded in order.
|
||||
|
||||
### What still needs the host
|
||||
|
||||
`the_command_names_that_still_need_the_host_are_these` names them, and
|
||||
`Host::handle` answers a `Key` frame with an empty vec, so each one is a binding
|
||||
that **does nothing at all** in a browser
|
||||
`resolve::NEEDS_THE_HOST` names them, and
|
||||
`the_command_names_that_still_need_the_host_are_these` drives each through the
|
||||
key that reaches it. `Host::handle` answers a `Key` frame with an empty vec, so
|
||||
each one is a binding that **does nothing at all** in a browser
|
||||
(`doctrine(an_upstream_key_is_a_black_hole)`).
|
||||
|
||||
**The help overlay marks them, and it marks rather than omits** (improvise-uln5).
|
||||
The overlay is shared content, and before this the browser painted every row
|
||||
plainly, including six that do nothing there. That is worse than a gap:
|
||||
improvise-koh6 showed that the keys typed after a dead one land on live Normal
|
||||
bindings, and `i` opens an editor. The browser's `RenderHelpRow` now draws a row
|
||||
whose command is on the list as `dim`, and appends "(not in the browser yet)"
|
||||
to its description. It does the same for every ex-command row while `:` opens a
|
||||
command line this client cannot run (`resolve::runs_the_command_line`, which is
|
||||
`stranding`'s answer, not a second one).
|
||||
|
||||
Marking was chosen over omitting for three reasons:
|
||||
- **Both front ends draw the same rows.** A page with rows missing is a second
|
||||
layout of the same content.
|
||||
- **The feature stays findable.** The row still says it exists in the terminal.
|
||||
- **It needs no change in `web/`.** `dim` is a kind the page already paints.
|
||||
|
||||
The TUI is unaffected, because the marking lives in the browser's renderer.
|
||||
|
||||
**One list, held from both sides.** `NEEDS_THE_HOST` is production data because
|
||||
the renderer needs it. Two tests hold it to what `resolve` actually does:
|
||||
- `the_command_names_that_still_need_the_host_are_these` fails if a name on it
|
||||
resolves locally.
|
||||
- `every_help_row_the_browser_cannot_run_is_one_it_marks` fails if a help row
|
||||
is silent in the browser without being on the list, or on the list without
|
||||
being silent. It also fails if a *hint line* ever names a listed command,
|
||||
because the status bar is never marked.
|
||||
|
||||
`every_advertised_command_answers_in_its_own_mode` therefore no longer restates
|
||||
the black holes. It skips a silent row whose command is on the list, because
|
||||
the other test owns it.
|
||||
|
||||
**An upstream key that the app also ADVERTISES is a different severity, and it
|
||||
had its own class of defect** (improvise-rmvx). A key nobody offers doing
|
||||
nothing is a gap; a key named in the mode's own hint line doing nothing reads as
|
||||
|
||||
@@ -213,6 +213,45 @@ when Ctrl-S became a `Request`; putting the entry back fails with *"save
|
||||
stops being a gap"*. So the mechanism gotcha 47 asks for was watched failing
|
||||
rather than assumed.
|
||||
|
||||
**Since improvise-uln5 it iterates `resolve::NEEDS_THE_HOST`** rather than a
|
||||
table of its own. It derives each name's keys from the Normal and search-mode
|
||||
keymaps, and arms a prefix before pressing its second key. So the list the
|
||||
browser's help marks and the list this test drives are the same list.
|
||||
`exit-search-mode` joined it, because search mode's `Enter`/`Esc` are silent
|
||||
too. Rival: add `move-selection`, which the browser runs, to the list. This test
|
||||
fails, and so does `every_help_row_the_browser_cannot_run_is_one_it_marks`.
|
||||
|
||||
### every_help_row_the_browser_cannot_run_is_one_it_marks
|
||||
|
||||
improvise-uln5. Every help row is pressed, in its own mode, on the fixture
|
||||
`furnished` builds. The commands whose keys are silent must be exactly the
|
||||
commands on `NEEDS_THE_HOST`, which are the rows the browser draws marked. The
|
||||
check runs in both directions, because each direction catches its own mistake:
|
||||
- a silent row that is not on the list is an unmarked black hole;
|
||||
- a listed row that acts is a false warning.
|
||||
|
||||
It also asserts that no status-bar hint names a listed command, since
|
||||
`hint_line` is never marked.
|
||||
|
||||
Rivals, each run against the fixed tree:
|
||||
- Drop `search` from the list: red. This test fails, and so does
|
||||
`every_advertised_command_answers_in_its_own_mode`, which no longer skips `/`.
|
||||
- Add `move-selection` to the list: red. This test fails, and so does the
|
||||
list's own test.
|
||||
|
||||
### a_help_row_the_browser_cannot_run_is_drawn_marked
|
||||
|
||||
improvise-uln5, the display half, and the red test. On the Navigation page the
|
||||
row "Start search — …" (key `/`) must render as `dim`, with "not in the browser"
|
||||
in its description. On the Commands page every `:` row must say the same. A
|
||||
row the browser runs ("Jump to last row") must stay a plain `keys` row with its
|
||||
description unchanged, so a renderer that marks everything cannot pass.
|
||||
|
||||
Red at `202e7eac`: the search row rendered as `keys`. Rival: the renderer's
|
||||
`needs_the_host` check replaced by `true`. It is red here, and only here, which
|
||||
is why the display has a test of its own and does not lean on the resolve-side
|
||||
guard.
|
||||
|
||||
### a_prompt_this_client_cannot_commit_is_never_entered
|
||||
|
||||
The refusal list above is only half the property. `Upstream` says the key
|
||||
@@ -2105,7 +2144,10 @@ two cannot drift — including prefix sequences (`gg`, `yy`, `ZZ`), which it
|
||||
presses by arming the prefix first. That is what added the five host-only
|
||||
black holes (`/`, `n`, `H`, `z`, Normal's `Enter`) to the list: each was already
|
||||
in `the_command_names_that_still_need_the_host_are_these`, and the help page
|
||||
advertised it anyway.
|
||||
advertised it anyway. Since the rest of improvise-uln5 they are no longer listed
|
||||
here: the browser marks those rows, and this test skips a silent row whose
|
||||
command is on `resolve::NEEDS_THE_HOST`, which
|
||||
`every_help_row_the_browser_cannot_run_is_one_it_marks` owns.
|
||||
|
||||
*Which cells.* `furnished` stood on a record cell only, so `i` on an aggregate
|
||||
(s8h7's whole subject) was outside its reach. Normal mode is now pressed on a
|
||||
|
||||
@@ -335,6 +335,7 @@ mod key_row {
|
||||
#[derive(Debug, Clone, PartialEq, Eq)]
|
||||
pub struct KeyRow {
|
||||
keys: Vec<String>,
|
||||
command: &'static str,
|
||||
description: &'static str,
|
||||
}
|
||||
|
||||
@@ -343,6 +344,12 @@ mod key_row {
|
||||
&self.keys
|
||||
}
|
||||
|
||||
/// The command the keys run, kept so a front end can ask whether it
|
||||
/// can run it (improvise-uln5).
|
||||
pub fn command(&self) -> &'static str {
|
||||
self.command
|
||||
}
|
||||
|
||||
pub fn description(&self) -> &'static str {
|
||||
self.description
|
||||
}
|
||||
@@ -351,7 +358,7 @@ mod key_row {
|
||||
/// advertises a key that does not exist.
|
||||
fn looked_up(
|
||||
mode: &ModeKey,
|
||||
command: &str,
|
||||
command: &'static str,
|
||||
args: Option<&[String]>,
|
||||
description: &'static str,
|
||||
) -> Option<Self> {
|
||||
@@ -360,7 +367,11 @@ mod key_row {
|
||||
.map(|(c, m)| key_label(c, *m))
|
||||
.collect();
|
||||
keys.extend(prefixed_labels(mode, command, args));
|
||||
(!keys.is_empty()).then_some(Self { keys, description })
|
||||
(!keys.is_empty()).then_some(Self {
|
||||
keys,
|
||||
command,
|
||||
description,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -359,6 +359,34 @@ fn not_yet(what: &str) -> String {
|
||||
format!("{what} is not available in the browser yet")
|
||||
}
|
||||
|
||||
/// The command names this client has no arm for, so their keys go upstream to
|
||||
/// a host that answers nothing (gotcha 47). The one list: see clients.md.
|
||||
pub const NEEDS_THE_HOST: &[&str] = &[
|
||||
"enter-advance",
|
||||
"exit-search-mode",
|
||||
"force-quit",
|
||||
"hide-selected-row-item",
|
||||
"search",
|
||||
"search-navigate",
|
||||
"toggle-group-under-cursor",
|
||||
];
|
||||
|
||||
/// Whether a key running `name` does nothing in this client.
|
||||
pub fn needs_the_host(name: &str) -> bool {
|
||||
NEEDS_THE_HOST.contains(&name)
|
||||
}
|
||||
|
||||
/// Whether `:` opens a command line this client can run; it cannot, so an
|
||||
/// ex-command it advertises is unreachable (improvise-uln5).
|
||||
pub fn runs_the_command_line() -> bool {
|
||||
AppMode::from_name("command").is_some_and(|mode| stranding(&mode).is_none())
|
||||
}
|
||||
|
||||
/// A help description, marked as naming something this client cannot do.
|
||||
pub fn not_here(description: &str) -> String {
|
||||
format!("{description} (not in the browser yet)")
|
||||
}
|
||||
|
||||
/// The subset of command names this client can turn into `Command`s; `None`
|
||||
/// means "the host's". Each arm mirrors its `Cmd` line for line — clients.md.
|
||||
fn composed(
|
||||
@@ -2402,7 +2430,12 @@ mod tests {
|
||||
}
|
||||
}
|
||||
}
|
||||
for (where_, got) in report.into_iter().filter(|(_, got)| *got != Heard::Acts) {
|
||||
// A black hole the browser marks is the next test's, by NEEDS_THE_HOST.
|
||||
let marked = |got: &Heard| *got == Heard::Silent && needs_the_host(command);
|
||||
for (where_, got) in report
|
||||
.into_iter()
|
||||
.filter(|(_, got)| *got != Heard::Acts && !marked(got))
|
||||
{
|
||||
short
|
||||
.entry((where_, command, got))
|
||||
.or_default()
|
||||
@@ -2426,14 +2459,6 @@ mod tests {
|
||||
"COMMAND: Enter execute-command Silent",
|
||||
"EXPORT: Enter commit-export Silent",
|
||||
"SAVE AS: Enter commit-save Silent",
|
||||
"NORMAL SEARCH: Enter/Esc exit-search-mode Silent",
|
||||
// Commands the browser has no arm for: upstream, so a black hole
|
||||
// (improvise-uln5). Each is also in the host-only list below.
|
||||
"NORMAL: Enter enter-advance Silent",
|
||||
"NORMAL: H hide-selected-row-item Silent",
|
||||
"NORMAL: / search Silent",
|
||||
"NORMAL: n search-navigate Silent",
|
||||
"NORMAL: z toggle-group-under-cursor Silent",
|
||||
// Honestly unavailable, and saying so in `declined`'s words.
|
||||
"FORMULAS: Del/d delete-formula-at-cursor Declines",
|
||||
"NORMAL: > drill-into-cell Declines",
|
||||
@@ -2667,41 +2692,93 @@ mod tests {
|
||||
/// closing one of these gaps has to come here and say so. Evidence file.
|
||||
#[test]
|
||||
fn the_command_names_that_still_need_the_host_are_these() {
|
||||
let mut state = in_mode(AppMode::Normal);
|
||||
state.grid.extent = GridExtent { rows: 4, cols: 4 };
|
||||
state.window = (4, 4);
|
||||
for (key, name) in [
|
||||
// The three prefixes left with improvise-hs0u: arming is a
|
||||
// `Command`, and the second key resolves in the sub-keymap.
|
||||
(ev(KeyCode::Char('H')), "hide-selected-row-item"),
|
||||
(ev(KeyCode::Char('z')), "toggle-group-under-cursor"),
|
||||
// `>`, `R`, `P`, Ctrl-W and Ctrl-E are NOT here any more: each
|
||||
// answers with a status of its own (improvise-gd28, improvise-rmvx).
|
||||
|
||||
// `<` left too, and further: since improvise-cfly it composes a
|
||||
// real `Command::ViewBack` rather than a refusal.
|
||||
(ev(KeyCode::Char('/')), "search"),
|
||||
(ev(KeyCode::Char('n')), "search-navigate"),
|
||||
// `N` left this list with improvise-koh6: both its arms are
|
||||
// answered here, one of them out loud. See clients.md.
|
||||
(ev(KeyCode::Enter), "enter-advance"),
|
||||
// Ctrl-S left this list with improvise-jz6w: it is a `Request`,
|
||||
// not a `Command`, and the host has answered it since 368h.
|
||||
(
|
||||
KeyEvent::new(KeyCode::Char('q'), KeyModifiers::CONTROL),
|
||||
"force-quit",
|
||||
),
|
||||
] {
|
||||
assert_eq!(
|
||||
resolve(&state, key),
|
||||
KeyOutcome::Upstream(key),
|
||||
"{name} ({:?}) was resolved locally; if that is deliberate, \
|
||||
this list is where it stops being a gap",
|
||||
key.code
|
||||
// `N` left with improvise-koh6, Ctrl-S with jz6w, and `>`, `R`, `P`,
|
||||
// Ctrl-W, Ctrl-E and `<` with gd28, rmvx and cfly: see clients.md.
|
||||
for name in NEEDS_THE_HOST {
|
||||
let mut sequences = Vec::new();
|
||||
for (mode, search) in [(AppMode::Normal, false), (AppMode::Normal, true)] {
|
||||
let key = ModeKey::from_app_mode(&mode, search).expect("a keyed mode");
|
||||
let keymap = Keymaps::shared().get(&key).expect("a keymap");
|
||||
for sequence in sequences_running(keymap, name, None) {
|
||||
sequences.push((mode.clone(), search, sequence));
|
||||
}
|
||||
}
|
||||
assert!(
|
||||
!sequences.is_empty(),
|
||||
"{name} is bound nowhere this test looks"
|
||||
);
|
||||
for (mode, search, sequence) in sequences {
|
||||
let mut state = in_mode(mode);
|
||||
state.view.search_mode = search;
|
||||
state.grid.extent = GridExtent { rows: 4, cols: 4 };
|
||||
state.window = (4, 4);
|
||||
let (key, prefix) = sequence.split_last().expect("a key");
|
||||
for armed in prefix {
|
||||
if let KeyOutcome::Local(commands) = resolve(&state, *armed) {
|
||||
commands.iter().for_each(|c| state.fold(c));
|
||||
}
|
||||
}
|
||||
let key = *key;
|
||||
assert_eq!(
|
||||
resolve(&state, key),
|
||||
KeyOutcome::Upstream(key),
|
||||
"{name} ({:?}) was resolved locally; if that is deliberate, it \
|
||||
leaves NEEDS_THE_HOST, and the browser stops marking it",
|
||||
key.code
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// improvise-uln5: the help rows a browser cannot run are exactly the ones
|
||||
/// it marks, and no hint names one. See `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn every_help_row_the_browser_cannot_run_is_one_it_marks() {
|
||||
for mode in AppMode::all() {
|
||||
for (command, _, _) in crate::help::advertised(&mode) {
|
||||
assert!(
|
||||
!needs_the_host(command),
|
||||
"the {mode:?} hint line advertises {command}, which a browser \
|
||||
cannot run and which the hint line never marks"
|
||||
);
|
||||
}
|
||||
}
|
||||
let mut silent = Vec::new();
|
||||
let mut marked = Vec::new();
|
||||
for (key, command, args, _) in crate::help::help_bindings() {
|
||||
let (mode, search) = match &key {
|
||||
ModeKey::SearchMode => (AppMode::Normal, true),
|
||||
key => (
|
||||
AppMode::all()
|
||||
.into_iter()
|
||||
.find(|m| ModeKey::from_app_mode(m, false).as_ref() == Some(key))
|
||||
.expect("a help row's mode is reachable"),
|
||||
false,
|
||||
),
|
||||
};
|
||||
let keymap = Keymaps::shared().get(&key).expect("a keymap");
|
||||
for sequence in sequences_running(keymap, command, args.as_deref()) {
|
||||
let mut state = furnished(mode.clone());
|
||||
state.view.search_mode = search;
|
||||
if press(state, &sequence) == Heard::Silent {
|
||||
silent.push(command);
|
||||
}
|
||||
}
|
||||
if needs_the_host(command) {
|
||||
marked.push(command);
|
||||
}
|
||||
}
|
||||
for list in [&mut silent, &mut marked] {
|
||||
list.sort();
|
||||
list.dedup();
|
||||
}
|
||||
assert_eq!(
|
||||
silent, marked,
|
||||
"left: help rows whose keys do nothing in a browser; right: the rows \
|
||||
it marks as needing the terminal. They must be the same rows."
|
||||
);
|
||||
}
|
||||
|
||||
/// improvise-jz6w: the outcome a resolver had no way to name. See
|
||||
/// `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
//! already made. The four silent ones are in `context/code/clients.md`.
|
||||
|
||||
use improvise_protocol::viewmodel::{CatTreeEntry, flatten_cat_tree};
|
||||
use improvise_protocol::{AxisFrame, CellDisplay, CellId, FrameEntry};
|
||||
use improvise_protocol::{AxisFrame, CellDisplay, CellId, FrameEntry, resolve};
|
||||
use serde::Serialize;
|
||||
|
||||
use crate::{Client, ClientState};
|
||||
@@ -423,22 +423,37 @@ impl RenderHelpRow {
|
||||
L::Heading(t) => prose("heading", t),
|
||||
L::Text(t) => prose("text", t),
|
||||
L::Accent(t) => prose("accent", t),
|
||||
L::Keys(row) => Self {
|
||||
keys: row.keys().join(" "),
|
||||
description: row.description().to_string(),
|
||||
kind: "keys",
|
||||
},
|
||||
L::Keys(row) => Self::row(
|
||||
row.keys().join(" "),
|
||||
row.description(),
|
||||
!resolve::needs_the_host(row.command()),
|
||||
false,
|
||||
),
|
||||
L::Literal {
|
||||
keys,
|
||||
description,
|
||||
dim,
|
||||
} => Self {
|
||||
keys: (*keys).to_string(),
|
||||
description: description.to_string(),
|
||||
kind: match dim {
|
||||
true => "dim",
|
||||
false => "keys",
|
||||
},
|
||||
} => Self::row(
|
||||
(*keys).to_string(),
|
||||
description,
|
||||
resolve::runs_the_command_line(),
|
||||
*dim,
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// A keys row. One this client cannot run is drawn dim and says so, since a
|
||||
/// dead key followed by live ones edits the grid (improvise-uln5, koh6).
|
||||
fn row(keys: String, description: &str, runs: bool, dim: bool) -> Self {
|
||||
Self {
|
||||
keys,
|
||||
description: match runs {
|
||||
true => description.to_string(),
|
||||
false => resolve::not_here(description),
|
||||
},
|
||||
kind: match dim || !runs {
|
||||
true => "dim",
|
||||
false => "keys",
|
||||
},
|
||||
}
|
||||
}
|
||||
@@ -579,6 +594,50 @@ mod tests {
|
||||
FrameEntry::Item(vec![label.into()])
|
||||
}
|
||||
|
||||
fn help_page(page: usize) -> Vec<RenderHelpRow> {
|
||||
let view = ViewState {
|
||||
mode: improvise_protocol::AppMode::Help,
|
||||
help_page: page,
|
||||
..ViewState::default()
|
||||
};
|
||||
help_rows(&view)
|
||||
}
|
||||
|
||||
/// improvise-uln5: a row whose key this client sends to a host that answers
|
||||
/// nothing is drawn marked. Evidence: `context/test-evidence/clients.md`.
|
||||
#[test]
|
||||
fn a_help_row_the_browser_cannot_run_is_drawn_marked() {
|
||||
let rows = help_page(1);
|
||||
let row = |prefix: &str| {
|
||||
let found = rows.iter().find(|r| r.description.starts_with(prefix));
|
||||
found.unwrap_or_else(|| panic!("no Navigation row reads {prefix:?}"))
|
||||
};
|
||||
let search = row("Start search");
|
||||
assert_eq!(search.kind, "dim", "{search:?}");
|
||||
assert!(
|
||||
search.description.contains("not in the browser"),
|
||||
"{search:?}"
|
||||
);
|
||||
let control = row("Jump to last row");
|
||||
assert_eq!(
|
||||
control.kind, "keys",
|
||||
"a row the browser runs is drawn plainly"
|
||||
);
|
||||
assert_eq!(control.description, "Jump to last row");
|
||||
let commands = help_page(4);
|
||||
let ex: Vec<_> = commands
|
||||
.iter()
|
||||
.filter(|r| r.keys.trim_start().starts_with(':'))
|
||||
.collect();
|
||||
assert!(
|
||||
!ex.is_empty(),
|
||||
"precondition: the Commands page lists ex-commands"
|
||||
);
|
||||
for row in ex {
|
||||
assert!(row.description.contains("not in the browser"), "{row:?}");
|
||||
}
|
||||
}
|
||||
|
||||
/// A run is opened by a HEADER, not by a name — improvise-h0iv, and both
|
||||
/// ends now ask the one rule. See the evidence file.
|
||||
#[test]
|
||||
|
||||
Reference in New Issue
Block a user