diff --git a/context/code/cli.md b/context/code/cli.md index 5b5ab034..cf1f6d1c 100644 --- a/context/code/cli.md +++ b/context/code/cli.md @@ -369,3 +369,95 @@ non-zero and writes nothing. It is checked before the build rather than read off the build's error, so the listing is structured data and not a parsed sentence. `separate` uses `ImportPipeline::suggested_duplicate_category`, and there is deliberately no flag to name the category yet. + +## 12 — the command language reads `|pipe quoting|`, and a surplus argument is refused (improvise-31oe.3) + +`create-view |By area|` used to make a view named `|By` and drop `area|` without +a word. Two separate defects, fixed separately. + +**The quoting.** `parse.rs` now has one lexer, `lex`, and everything else is +built on it: `parse_line_with`, `segments` and `parse_segment_interactive`. The +rules are: + +- A `|…|` piece is read by `improvise_formula::quote::unescape_pipe_body`, the + one home of pipe quoting (gotcha 34). So `\|`, `\\` and `\n` mean exactly what + they mean in `.improv` and in formulas. The lexer only finds the closing pipe: + a backslash takes the next character with it, so `\|` does not close. +- A piece may sit **anywhere in a word**, and the pieces and bare text are + joined. So a coordinate quotes each half, `|Sales Region|/|East Coast|`, the + way a `.improv` data line quotes each name. The alternative, a pipe only at + the start of a word, would have left no way to spell a coordinate with a + space in it. +- `"…"` is **kept**, verbatim, with no escapes, and a `|` inside it stays a + `|`. It is not a workaround to retire. Formula text carries its own pipes: + `add-formula "Net = |Gross Rev| - Cost"` must reach `parse_formula` with the + pipes on. If the line had pipes only, every pipe in a formula would need + escaping. Its old leniency is kept too: a `"` that is never closed runs to + the end of the line. A `|` that is never closed is refused, naming the text + from the open pipe on, because a name is never meant to run to the end. +- The separator is a **bare** `.` word. A quoted `|.|` or `"."` is an argument, + and a `.` inside `|a . b|` does not split the line. The old `split_on_dot` + was a second scanner that knew double quotes and not pipes. With one lexer + there is no second scanner to disagree. +- A comment line (`#`, `//`) is not lexed, so an unmatched pipe in a comment is + not an error. + +`segments` now returns `Result`, because an unterminated pipe is a fact about +the whole line. `main.rs` refuses the line before running any of it, as §7 does +for a parse error, and `ExecuteCommand` refuses it on the status line. + +What is still not spellable: an item whose name contains `/`. The coordinate is +split on the first `/` **after** unquoting, so `|a/b|/x` is still read as +category `a` and item `b/x`. Closing that needs the lexer to hand structured +coordinates to the commands, which is a larger change than this ticket. + +**The arity.** Every registration now declares `MaxArgs::AtMost(n)` or +`MaxArgs::Any` beside its prototype. `CmdRegistry::parse` and +`CmdRegistry::interactive` apply it after construction succeeds. So the rule is +written once and every command gets it, and a new registration cannot compile +without a declaration. The refusal names the command, the limit and every +surplus argument (`create-view: takes at most 1 argument(s), got 2; unexpected +"area"`). A headless run exits non-zero through the §7 parse gate, and the `:` +line refuses on the status line. + +It runs **after** the command's own parse, deliberately. A command whose own +refusal states its shape keeps it: `add-formula`'s "quote the formula" +(improvise-z05h) is more use than an arity, and it is what `formula Product +Total = Widget + Gadget` still says. + +`MaxArgs::Any` is a claim, and the list of commands making it is short: +`add-items`, `set-cell`, `set-format` (it joins its words), `clear-cell`, +`yank`, `paste`, `drill-into-cell` and `commit-cell-edit`. Each takes a list of +items or coordinates. `no_command_silently_drops_an_argument_it_was_given` names +that list, so widening another command to `Any` has to add it there. + +## 13 — a command that names something the document lacks is refused (improvise-31oe.2) + +`set-axis Regoin row`, `set-page Region Nowhere` and `set-axis Region sideways` +all exited 0 and said nothing. §10's rule applies: nothing changed, so the +request was not met. + +- **An unreadable axis is an argument error.** `set-axis` now checks its axis + with `parse_axis` when it is parsed, so a script is refused before any of it + runs. `execute` still refuses an axis it cannot read rather than returning + nothing. That arm cannot be reached through the registry. +- **An unknown category, item, group or view is a refusal**, through + `effect::refuse` in `execute`, because only the context knows what the + document holds. `unknown_category` asks the active view (`try_axis_of`). That + is exactly the condition under which `View::set_axis` was a silent no-op. + `unknown_item` asks `effective_item_names`, so a formula target is a known + `_Measure` item. `unknown_group` asks both an item's own group and the + category's `groups`. `add_item_in_group` registers no `Group`, and a parent + group has no items, so either one alone misses a real group. + +The siblings fixed with it: `hide-item` and `show-item` (hiding an unknown item +wrote it into `hidden_items`), `toggle-group`, and `switch-view` (the effect +ignores a view it lacks, and says nothing). `set-page` with an unknown item was +worse than a no-op: it wrote the name into `page_selections`. + +The check is in the **commands**, not the effects, on purpose. The effects are +also the wire's mutation path (`model_effects`, gotcha 38), and a refusing +effect there becomes a frame rejection (`MutationRefused`). That is arguably +right, but it changes the protocol, and a browser composes these only from +names it was sent. So it is left for a ticket of its own rather than slipped +in here. diff --git a/context/code/command.md b/context/code/command.md index a89a6621..bd88459d 100644 --- a/context/code/command.md +++ b/context/code/command.md @@ -2996,8 +2996,10 @@ ride in `ViewState::transient_prefix`, so they must live where the wire types do and the re-export keeps `crate::command::key::…` paths working. `parse.rs` is the quasi-lisp prefix parser: `word arg1 arg2 …`, several commands per -line separated by `.`, coordinate pairs written `Category/Item`, and quoted strings -(`"Profit = Revenue - Cost"`). +line separated by `.`, coordinate pairs written `Category/Item`, verbatim quoted +strings (`"Profit = Revenue - Cost"`) and `|pipe-quoted names|` read by +`improvise_formula::quote`. One lexer serves every entry point, so the separator +and the quoting cannot disagree (improvise-31oe.3, `context/code/cli.md` §12). ### `cmd/core.rs` @@ -3014,6 +3016,12 @@ writes any other field, so an accepted argument list is never overridden by context (improvise-2bac, improvise-yf7p; the argument is `context/code/cli.md` §8). `register_pure` is the case with no fallback: both arms are `parse`. +Every registration also declares its `MaxArgs`, and the registry refuses surplus +arguments on both arms after the command is built. `register_nullary` declares +`AtMost(0)` for itself, and an `effect_cmd!` declares its limit in the macro +call as `MAX_ARGS`. So the arity rule is written once and is total over the +registry (improvise-31oe.3, `context/code/cli.md` §12). + `write_buffer` is the effect that writes a named buffer — the mirror of `read_buffer`, and the only place that knows "search" is not an entry in the buffers map but its own `ViewState` field (improvise-gdl, gotcha 11). That knowledge used to @@ -3372,6 +3380,14 @@ The parseable model-mutation commands are thin `Cmd` wrappers around effects, constructible from string args, sharing the same execution path as keymap-dispatched commands (gotcha 16). +A wrapper that names a category, item or group checks it in `execute` through +`unknown_category`, `unknown_item` or `unknown_group`, `switch-view` checks +`workbook.views`, and each emits +`effect::refuse` in place of its effect when the document lacks it +(improvise-31oe.2). The effects stay silent about unknown names, because they +are also the wire's mutation path. Why the check sits here and not there: +`context/code/cli.md` §13. + ### `cmd/wizard.rs` (improvise-2wm, improvise-907) Import-wizard key handling. The wizard is a small step machine, and which key means diff --git a/context/test-evidence/cli.md b/context/test-evidence/cli.md index be0c036f..bc7d0033 100644 --- a/context/test-evidence/cli.md +++ b/context/test-evidence/cli.md @@ -407,3 +407,82 @@ named, and no output file. It has two siblings. `..._sums_duplicates_when_told_t asserts `= 30` and no `Occurrence`. `..._keeps_duplicates_separate_when_told_to` asserts the category, `= 10` and `= 20`, and no `= 30`. So the two flags cannot both be summing. + +### a_pipe_quoted_argument_is_one_argument_without_its_pipes + +improvise-31oe.3. Red before the fix: the old tokenizer split on whitespace +only, so `create-view |By area|` built `CreateViewCmd(["|By", "area|"])`. It +asserts the whole built command, so a lexer that joined the words but kept the +pipes fails as well as one that split them. The second case has two quoted +arguments side by side, so a lexer that reads one pipe piece per line fails. +Rival: `bare_word` pushing `|` as a plain character. Fails here and in +`a_coordinate_may_pipe_quote_either_half`. + +### a_pipe_quoted_argument_reads_the_escapes_improv_does + +The three escapes `pipe_quote` writes, `\|`, `\\` and `\n`, each in its own +case. Red before the fix. Rival: `pipe_body` returning the raw body without +`unescape_pipe_body`. Fails here, at the first case, and nowhere else in the +crate, which is why the case exists. A lexer that treated `\|` as a closing +pipe would also fail the first case, because the pipe after `b` then opens a +piece that never closes. + +### a_command_given_more_arguments_than_it_takes_is_refused + +improvise-31oe.3's second half. Red before the fix: `create-view By area` +parsed and dropped `area`. It asserts the refusal opens with the command's name +and quotes the surplus word, and that the pipe-quoted spelling of the same line +is the control that parses. Rival: `MaxArgs::admit` answering `Ok` always. +Fails here, in `no_command_silently_drops_an_argument_it_was_given`, and in +`a_colon_line_naming_something_missing_refuses_on_the_status_line`. + +### no_command_silently_drops_an_argument_it_was_given + +The rule is one rule only if every registration obeys it, so this enumerates +every registered name against every candidate argument list, on both arms. For +each list a command accepts, it appends **four** surplus words. One would be +absorbed by an optional argument (`add-formula ["1"]` plus one more is a valid +two-argument call), and four is more than any optional span in the registry. +The last word must be named in the refusal. The exceptions are stated, not +inferred: the `Any` list, and three commands whose own shape refusal runs +first. Rivals, each run: declaring `add-item-in-group` as `Any` fails here, +naming the dropped list. Removing the check from the interactive arm alone +fails here and in the `:` line test. A test of the pure arm alone would have +missed that. + +### a_command_naming_something_the_document_lacks_is_a_refusal + +improvise-31oe.2 and its siblings: `set-axis`, `set-page`, `hide-item`, +`show-item`, `toggle-group` and `switch-view`, each naming a missing category, +item, group or view. Red before the fix at the first row, `set-axis Regoin +column`, with an empty status. Each row asserts the refusal flag, the missing +name in the message, and that the document's `.improv` text and the active view +are unchanged. The last two catch a refusal written after the write. The +control is `a_command_naming_what_the_document_has_is_not_refused`, whose group +row is the one that caught `unknown_group` reading only `Category::groups`: +`add_item_in_group` registers no `Group`. Rivals, each run: `unknown_item` +answering `None` fails at `set-page ["Type", "Nowhere"]`. `unknown_category` +answering `None` fails at the first row. The `switch-view` check disabled fails +at its row. + +### a_colon_line_naming_something_missing_refuses_on_the_status_line + +The TUI path for both tickets, through `ExecuteCommand` and `App::apply_effects`, +not through the command alone. It asserts the refusal flag, the name on the +status line, and the mode back at Normal. Rivals: an unknown category accepted +fails at `:set-axis Regoin row`, and the arity check removed from the +interactive arm fails at `:create-view By area`. + +### an_unknown_category_axis_or_item_exits_non_zero_and_names_it + +The three repros from improvise-31oe.2, at the process boundary. Red against +the `gh/main` source with this test file: `set-axis Regoin row` exited 0 with an +empty stderr. The control, `set-page Region East`, must exit 0. Without it, a +fix that refused every `set-page` would pass. + +### a_pipe_quoted_name_reaches_the_document_whole + +improvise-31oe.3's repro at the process boundary: the saved document must carry +`## View: By area` and no `|By`. The second half, `create-view By area`, must +exit non-zero naming `"area"`. Red against the `gh/main` source: the document +carried the view `|By`.