From 197ce61d65edb768d7a2240e704a3b45a790de6d Mon Sep 17 00:00:00 2001 From: "Claude (agent-cmdlang)" Date: Thu, 24 Sep 2026 21:43:18 -0700 Subject: [PATCH] fix(command): read |pipe quoting| and refuse surplus arguments (improvise-31oe.3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The command-line lexer reads |…| as one argument through the shared unescape_pipe_body, anywhere in a word, so a coordinate can quote each half. Double quotes stay verbatim. Every registration now declares MaxArgs, and the registry refuses arguments beyond it, naming them, on both arms. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze --- .../improvise-command/src/command/cmd/core.rs | 43 ++- .../src/command/cmd/effect_cmds.rs | 32 +- .../improvise-command/src/command/cmd/fit.rs | 4 +- .../src/command/cmd/registry.rs | 206 +++++++++++-- .../src/command/cmd/text_buffer.rs | 9 +- crates/improvise-command/src/command/parse.rs | 281 ++++++++++++------ crates/improvise-command/src/ui/app.rs | 4 +- src/main.rs | 16 +- 8 files changed, 461 insertions(+), 134 deletions(-) diff --git a/crates/improvise-command/src/command/cmd/core.rs b/crates/improvise-command/src/command/cmd/core.rs index 7b9808e9..dd6a02c2 100644 --- a/crates/improvise-command/src/command/cmd/core.rs +++ b/crates/improvise-command/src/command/cmd/core.rs @@ -250,12 +250,37 @@ pub trait AtCursor: Cmd { fn cursor_mut(&mut self) -> &mut CursorState; } +/// How many arguments a command takes. The registry refuses any beyond it, for +/// every command in one place — `context/code/cli.md` §12. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +pub enum MaxArgs { + AtMost(usize), + Any, +} + +impl MaxArgs { + fn admit(self, args: &[String]) -> Result<(), String> { + match self { + MaxArgs::AtMost(n) if args.len() > n => { + let extra: Vec = args[n..].iter().map(|a| format!("{a:?}")).collect(); + Err(format!( + "takes at most {n} argument(s), got {}; unexpected {}", + args.len(), + extra.join(" ") + )) + } + MaxArgs::AtMost(_) | MaxArgs::Any => Ok(()), + } + } +} + type BoxParseFn = Box Result, String>>; type BoxInteractiveFn = Box Result, String>>; /// A registered command entry with both text and interactive constructors. struct CmdEntry { name: &'static str, + max: MaxArgs, parse: BoxParseFn, interactive: BoxInteractiveFn, } @@ -295,10 +320,11 @@ impl CmdRegistry { pub fn register( &mut self, prototype: &C, + max: MaxArgs, parse: fn(&[String]) -> Result, fallback: Option>, ) { - self.push(prototype.name(), parse, fallback, |_, _| {}); + self.push(prototype.name(), max, parse, fallback, |_, _| {}); } /// As [`register`](Self::register), then the interactive arm moves the @@ -306,10 +332,11 @@ impl CmdRegistry { pub fn register_at_cursor( &mut self, prototype: &C, + max: MaxArgs, parse: fn(&[String]) -> Result, fallback: Option>, ) { - self.push(prototype.name(), parse, fallback, |cmd, ctx| { + self.push(prototype.name(), max, parse, fallback, |cmd, ctx| { *cmd.cursor_mut() = CursorState::from_ctx(ctx); }); } @@ -317,12 +344,14 @@ impl CmdRegistry { fn push( &mut self, name: &'static str, + max: MaxArgs, parse: fn(&[String]) -> Result, fallback: Option>, at_cursor: fn(&mut C, &CmdContext), ) { self.entries.push(CmdEntry { name, + max, parse: Box::new(move |args| parse(args).map(|c| Box::new(c) as Box)), interactive: Box::new(move |args, ctx| { let mut built = match (parse(args), fallback) { @@ -338,9 +367,10 @@ impl CmdRegistry { /// Register a command that needs no interactive context: BOTH arms are /// `parse`, so neither can demote the other — `context/code/command.md`. - pub fn register_pure(&mut self, prototype: &dyn Cmd, parse: ParseFn) { + pub fn register_pure(&mut self, prototype: &dyn Cmd, max: MaxArgs, parse: ParseFn) { self.entries.push(CmdEntry { name: prototype.name(), + max, parse: Box::new(parse), interactive: Box::new(move |args, _ctx| parse(args)), }); @@ -352,6 +382,7 @@ impl CmdRegistry { let name = f().name(); self.entries.push(CmdEntry { name, + max: MaxArgs::AtMost(0), parse: Box::new(move |_| Ok(f())), interactive: Box::new(move |_, _| Ok(f())), }); @@ -364,7 +395,9 @@ impl CmdRegistry { let canonical = self.resolve(name); for e in &self.entries { if e.name == canonical { - return (e.parse)(args).map_err(|err| format!("{name}: {err}")); + return (e.parse)(args) + .and_then(|cmd| e.max.admit(args).map(|()| cmd)) + .map_err(|err| format!("{name}: {err}")); } } Err(format!("Unknown command: {name}")) @@ -381,7 +414,7 @@ impl CmdRegistry { let name = self.resolve(name); for e in &self.entries { if e.name == name { - return (e.interactive)(args, ctx); + return (e.interactive)(args, ctx).and_then(|cmd| e.max.admit(args).map(|()| cmd)); } } Err(format!("Unknown command: {name}")) diff --git a/crates/improvise-command/src/command/cmd/effect_cmds.rs b/crates/improvise-command/src/command/cmd/effect_cmds.rs index 44d93e32..38c8961d 100644 --- a/crates/improvise-command/src/command/cmd/effect_cmds.rs +++ b/crates/improvise-command/src/command/cmd/effect_cmds.rs @@ -2,7 +2,7 @@ use crate::model::cell::CellValue; use crate::ui::effect::{self, Effect, ViewTarget}; use crate::view::Axis; -use super::core::{Cmd, CmdContext, require_args}; +use super::core::{Cmd, CmdContext, MaxArgs, require_args}; #[cfg(test)] mod tests { @@ -129,7 +129,7 @@ mod tests { // Thin Cmd wrappers around effects, on the same path as keymap dispatch. macro_rules! effect_cmd { - ($name:ident, $cmd_name:expr, $parse:expr, $exec:expr) => { + ($name:ident, $cmd_name:expr, $max:expr, $parse:expr, $exec:expr) => { #[derive(Debug)] pub struct $name(pub Vec); impl Cmd for $name { @@ -143,6 +143,7 @@ macro_rules! effect_cmd { } } impl $name { + pub const MAX_ARGS: MaxArgs = $max; pub fn parse(args: &[String]) -> Result, String> { #[allow(clippy::redundant_closure_call)] ($parse)(args)?; @@ -155,6 +156,7 @@ macro_rules! effect_cmd { effect_cmd!( AddCategoryCmd, "add-category", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::AddCategory(args[0].clone()))] @@ -164,6 +166,7 @@ effect_cmd!( effect_cmd!( AddItemCmd, "add-item", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::AddItem { @@ -176,6 +179,7 @@ effect_cmd!( effect_cmd!( AddItemsCmd, "add-items", + MaxArgs::Any, |args: &[String]| { if args.len() < 2 { Err("add-items requires a category and at least one item".to_string()) @@ -200,6 +204,7 @@ effect_cmd!( effect_cmd!( AddItemInGroupCmd, "add-item-in-group", + MaxArgs::AtMost(3), |args: &[String]| require_args(args, 3), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::AddItemInGroup { @@ -213,6 +218,7 @@ effect_cmd!( effect_cmd!( SetCellCmd, "set-cell", + MaxArgs::Any, |args: &[String]| { if args.len() < 2 { Err("set-cell requires a value and at least one Cat/Item coordinate".to_string()) @@ -241,6 +247,7 @@ effect_cmd!( effect_cmd!( AddFormulaCmd, "add-formula", + MaxArgs::AtMost(2), |args: &[String]| { if args.is_empty() || args.len() > 2 { // The arity alone does not tell anyone to quote — improvise-z05h. @@ -269,6 +276,7 @@ effect_cmd!( effect_cmd!( ClearBufferCmd, "clear-buffer", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![super::core::write_buffer(&args[0], String::new())] @@ -278,6 +286,7 @@ effect_cmd!( effect_cmd!( RemoveFormulaCmd, "remove-formula", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, ctx: &CmdContext| -> Vec> { let (category, target) = (&args[0], &args[1]); @@ -298,6 +307,7 @@ effect_cmd!( effect_cmd!( CreateViewCmd, "create-view", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::CreateView(args[0].clone()))] @@ -307,6 +317,7 @@ effect_cmd!( effect_cmd!( DeleteViewCmd, "delete-view", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::DeleteView(args[0].clone()))] @@ -316,6 +327,7 @@ effect_cmd!( effect_cmd!( SwitchViewCmd, "switch-view", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::SwitchView(args[0].clone()))] @@ -325,6 +337,7 @@ effect_cmd!( effect_cmd!( SetAxisCmd, "set-axis", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { let axis = match args[1].to_lowercase().as_str() { @@ -345,6 +358,7 @@ effect_cmd!( effect_cmd!( SetPageCmd, "set-page", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::SetPageSelection { @@ -358,6 +372,7 @@ effect_cmd!( effect_cmd!( ToggleGroupCmd, "toggle-group", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::ToggleGroup { @@ -371,6 +386,7 @@ effect_cmd!( effect_cmd!( HideItemCmd, "hide-item", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::HideItem { @@ -384,6 +400,7 @@ effect_cmd!( effect_cmd!( ShowItemCmd, "show-item", + MaxArgs::AtMost(2), |args: &[String]| require_args(args, 2), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::ShowItem { @@ -397,6 +414,7 @@ effect_cmd!( effect_cmd!( SaveAsCmd, "save-as", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::SaveAs(std::path::PathBuf::from(&args[0])))] @@ -406,6 +424,7 @@ effect_cmd!( effect_cmd!( SetFormatCmd, "set-format", + MaxArgs::Any, |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::SetNumberFormat( @@ -418,6 +437,7 @@ effect_cmd!( effect_cmd!( ImportCmd, "import", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::StartImportWizard(args[0].clone()))] @@ -427,6 +447,7 @@ effect_cmd!( effect_cmd!( ExportCmd, "export", + MaxArgs::AtMost(1), |_args: &[String]| -> Result<(), String> { Ok(()) }, |args: &Vec, _ctx: &CmdContext| -> Vec> { let path = args.first().map(|s| s.as_str()).unwrap_or("export.csv"); @@ -437,6 +458,7 @@ effect_cmd!( effect_cmd!( WriteCmd, "w", + MaxArgs::AtMost(1), |_args: &[String]| -> Result<(), String> { Ok(()) }, |args: &Vec, _ctx: &CmdContext| -> Vec> { if args.is_empty() { @@ -450,6 +472,7 @@ effect_cmd!( effect_cmd!( HelpCmd, "help", + MaxArgs::AtMost(0), |_args: &[String]| -> Result<(), String> { Ok(()) }, |_args: &Vec, _ctx: &CmdContext| -> Vec> { vec![ @@ -462,6 +485,7 @@ effect_cmd!( effect_cmd!( HelpPageNextCmd, "help-page-next", + MaxArgs::AtMost(0), |_args: &[String]| -> Result<(), String> { Ok(()) }, |_args: &Vec, _ctx: &CmdContext| -> Vec> { vec![effect::help_page_next()] @@ -471,6 +495,7 @@ effect_cmd!( effect_cmd!( HelpPagePrevCmd, "help-page-prev", + MaxArgs::AtMost(0), |_args: &[String]| -> Result<(), String> { Ok(()) }, |_args: &Vec, _ctx: &CmdContext| -> Vec> { vec![effect::help_page_prev()] @@ -480,6 +505,7 @@ effect_cmd!( effect_cmd!( LoadModelCmd, "load", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::LoadModel(std::path::PathBuf::from( @@ -491,6 +517,7 @@ effect_cmd!( effect_cmd!( ExportCsvCmd, "export-csv", + MaxArgs::AtMost(1), |args: &[String]| require_args(args, 1), |args: &Vec, _ctx: &CmdContext| -> Vec> { vec![Box::new(effect::ExportCsv(std::path::PathBuf::from( @@ -502,6 +529,7 @@ effect_cmd!( effect_cmd!( ImportJsonCmd, "import-json", + MaxArgs::AtMost(3), |args: &[String]| { if args.is_empty() { Err("import-json requires a path".to_string()) diff --git a/crates/improvise-command/src/command/cmd/fit.rs b/crates/improvise-command/src/command/cmd/fit.rs index da7fb3c3..8c2fb087 100644 --- a/crates/improvise-command/src/command/cmd/fit.rs +++ b/crates/improvise-command/src/command/cmd/fit.rs @@ -4,7 +4,7 @@ use improvise_gp::fit::one_line; use improvise_gp::{FitRequest, fit_measure, fitted_measure_name}; -use crate::command::cmd::core::{Cmd, CmdContext}; +use crate::command::cmd::core::{Cmd, CmdContext, MaxArgs}; use crate::ui::effect::{self, Effect}; /// A seed as the status line spells it (`0x…`, either case) or in decimal. The hex @@ -27,6 +27,8 @@ pub struct FitMeasure { } impl FitMeasure { + pub const MAX_ARGS: MaxArgs = MaxArgs::AtMost(3); + /// A nameless instance for `register_pure`, which reads a command's name /// off a prototype rather than off a string literal (gotcha 15). pub fn prototype() -> Self { diff --git a/crates/improvise-command/src/command/cmd/registry.rs b/crates/improvise-command/src/command/cmd/registry.rs index e394c118..069d3fcc 100644 --- a/crates/improvise-command/src/command/cmd/registry.rs +++ b/crates/improvise-command/src/command/cmd/registry.rs @@ -81,16 +81,33 @@ pub fn default_registry() -> CmdRegistry { let mut r = CmdRegistry::new(); // ── Model mutations (effect_cmd! wrappers) ─────────────────────────── - r.register_pure(&AddCategoryCmd(vec![]), AddCategoryCmd::parse); - r.register_pure(&AddItemCmd(vec![]), AddItemCmd::parse); - r.register_pure(&AddItemsCmd(vec![]), AddItemsCmd::parse); - r.register_pure(&AddItemInGroupCmd(vec![]), AddItemInGroupCmd::parse); - r.register_pure(&SetCellCmd(vec![]), SetCellCmd::parse); - r.register_pure(&ClearBufferCmd(vec![]), ClearBufferCmd::parse); + r.register_pure( + &AddCategoryCmd(vec![]), + AddCategoryCmd::MAX_ARGS, + AddCategoryCmd::parse, + ); + r.register_pure(&AddItemCmd(vec![]), AddItemCmd::MAX_ARGS, AddItemCmd::parse); + r.register_pure( + &AddItemsCmd(vec![]), + AddItemsCmd::MAX_ARGS, + AddItemsCmd::parse, + ); + r.register_pure( + &AddItemInGroupCmd(vec![]), + AddItemInGroupCmd::MAX_ARGS, + AddItemInGroupCmd::parse, + ); + r.register_pure(&SetCellCmd(vec![]), SetCellCmd::MAX_ARGS, SetCellCmd::parse); + r.register_pure( + &ClearBufferCmd(vec![]), + ClearBufferCmd::MAX_ARGS, + ClearBufferCmd::parse, + ); r.register( &ClearCellCommand { key: CellKey::new(vec![]), }, + MaxArgs::Any, |args| { if args.is_empty() { return Err("clear-cell requires at least one Cat/Item coordinate".into()); @@ -105,28 +122,88 @@ pub fn default_registry() -> CmdRegistry { }) }), ); - r.register_pure(&AddFormulaCmd(vec![]), AddFormulaCmd::parse); - r.register_pure(&FitMeasure::prototype(), FitMeasure::parse); - r.register_pure(&RemoveFormulaCmd(vec![]), RemoveFormulaCmd::parse); - r.register_pure(&CreateViewCmd(vec![]), CreateViewCmd::parse); - r.register_pure(&DeleteViewCmd(vec![]), DeleteViewCmd::parse); - r.register_pure(&SwitchViewCmd(vec![]), SwitchViewCmd::parse); - r.register_pure(&SetAxisCmd(vec![]), SetAxisCmd::parse); - r.register_pure(&SetPageCmd(vec![]), SetPageCmd::parse); - r.register_pure(&ToggleGroupCmd(vec![]), ToggleGroupCmd::parse); - r.register_pure(&HideItemCmd(vec![]), HideItemCmd::parse); - r.register_pure(&ShowItemCmd(vec![]), ShowItemCmd::parse); - r.register_pure(&SaveAsCmd(vec![]), SaveAsCmd::parse); - r.register_pure(&LoadModelCmd(vec![]), LoadModelCmd::parse); - r.register_pure(&ExportCsvCmd(vec![]), ExportCsvCmd::parse); - r.register_pure(&ImportJsonCmd(vec![]), ImportJsonCmd::parse); - r.register_pure(&SetFormatCmd(vec![]), SetFormatCmd::parse); - r.register_pure(&ImportCmd(vec![]), ImportCmd::parse); - r.register_pure(&ExportCmd(vec![]), ExportCmd::parse); - r.register_pure(&WriteCmd(vec![]), WriteCmd::parse); - r.register_pure(&HelpCmd(vec![]), HelpCmd::parse); - r.register_pure(&HelpPageNextCmd(vec![]), HelpPageNextCmd::parse); - r.register_pure(&HelpPagePrevCmd(vec![]), HelpPagePrevCmd::parse); + r.register_pure( + &AddFormulaCmd(vec![]), + AddFormulaCmd::MAX_ARGS, + AddFormulaCmd::parse, + ); + r.register_pure( + &FitMeasure::prototype(), + FitMeasure::MAX_ARGS, + FitMeasure::parse, + ); + r.register_pure( + &RemoveFormulaCmd(vec![]), + RemoveFormulaCmd::MAX_ARGS, + RemoveFormulaCmd::parse, + ); + r.register_pure( + &CreateViewCmd(vec![]), + CreateViewCmd::MAX_ARGS, + CreateViewCmd::parse, + ); + r.register_pure( + &DeleteViewCmd(vec![]), + DeleteViewCmd::MAX_ARGS, + DeleteViewCmd::parse, + ); + r.register_pure( + &SwitchViewCmd(vec![]), + SwitchViewCmd::MAX_ARGS, + SwitchViewCmd::parse, + ); + r.register_pure(&SetAxisCmd(vec![]), SetAxisCmd::MAX_ARGS, SetAxisCmd::parse); + r.register_pure(&SetPageCmd(vec![]), SetPageCmd::MAX_ARGS, SetPageCmd::parse); + r.register_pure( + &ToggleGroupCmd(vec![]), + ToggleGroupCmd::MAX_ARGS, + ToggleGroupCmd::parse, + ); + r.register_pure( + &HideItemCmd(vec![]), + HideItemCmd::MAX_ARGS, + HideItemCmd::parse, + ); + r.register_pure( + &ShowItemCmd(vec![]), + ShowItemCmd::MAX_ARGS, + ShowItemCmd::parse, + ); + r.register_pure(&SaveAsCmd(vec![]), SaveAsCmd::MAX_ARGS, SaveAsCmd::parse); + r.register_pure( + &LoadModelCmd(vec![]), + LoadModelCmd::MAX_ARGS, + LoadModelCmd::parse, + ); + r.register_pure( + &ExportCsvCmd(vec![]), + ExportCsvCmd::MAX_ARGS, + ExportCsvCmd::parse, + ); + r.register_pure( + &ImportJsonCmd(vec![]), + ImportJsonCmd::MAX_ARGS, + ImportJsonCmd::parse, + ); + r.register_pure( + &SetFormatCmd(vec![]), + SetFormatCmd::MAX_ARGS, + SetFormatCmd::parse, + ); + r.register_pure(&ImportCmd(vec![]), ImportCmd::MAX_ARGS, ImportCmd::parse); + r.register_pure(&ExportCmd(vec![]), ExportCmd::MAX_ARGS, ExportCmd::parse); + r.register_pure(&WriteCmd(vec![]), WriteCmd::MAX_ARGS, WriteCmd::parse); + r.register_pure(&HelpCmd(vec![]), HelpCmd::MAX_ARGS, HelpCmd::parse); + r.register_pure( + &HelpPageNextCmd(vec![]), + HelpPageNextCmd::MAX_ARGS, + HelpPageNextCmd::parse, + ); + r.register_pure( + &HelpPagePrevCmd(vec![]), + HelpPagePrevCmd::MAX_ARGS, + HelpPagePrevCmd::parse, + ); // ── Navigation (unified Move) ────────────────────────────────────── r.register_at_cursor( @@ -135,6 +212,7 @@ pub fn default_registry() -> CmdRegistry { cursor: CursorState::default(), cmd_name: "move-selection", }, + MaxArgs::AtMost(2), |args| { require_args(args, 2)?; let dr = args[0].parse::().map_err(|e| e.to_string())?; @@ -160,6 +238,7 @@ pub fn default_registry() -> CmdRegistry { cursor: CursorState::default(), cmd_name: $name, }, + MaxArgs::AtMost(0), |_| { Ok(Move { kind: if $to_end { @@ -185,6 +264,7 @@ pub fn default_registry() -> CmdRegistry { cursor: CursorState::default(), cmd_name: "scroll-rows", }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let n = args[0].parse::().map_err(|e| e.to_string())?; @@ -202,6 +282,7 @@ pub fn default_registry() -> CmdRegistry { cursor: CursorState::default(), cmd_name: "page-scroll", }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let dir = args[0].parse::().map_err(|e| e.to_string())?; @@ -217,6 +298,7 @@ pub fn default_registry() -> CmdRegistry { &EnterAdvance { cursor: CursorState::default(), }, + MaxArgs::AtMost(0), |_| { Ok(EnterAdvance { cursor: CursorState { @@ -239,6 +321,7 @@ pub fn default_registry() -> CmdRegistry { &YankCell { key: CellKey::new(vec![]), }, + MaxArgs::Any, |args| { if args.is_empty() { return Err("yank requires at least one Cat/Item coordinate".into()); @@ -257,6 +340,7 @@ pub fn default_registry() -> CmdRegistry { &PasteCell { at: PasteAt::Spelled(CellKey::new(vec![])), }, + MaxArgs::Any, |args| { if args.is_empty() { return Err("paste requires at least one Cat/Item coordinate".into()); @@ -288,6 +372,7 @@ pub fn default_registry() -> CmdRegistry { &EditOrDrill { edit_mode: AppMode::Normal, }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let edit_mode = parse_mode_name(&args[0])?; @@ -298,6 +383,7 @@ pub fn default_registry() -> CmdRegistry { &EnterEditAtCursorCmd { target_mode: AppMode::Normal, }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let target_mode = parse_mode_name(&args[0])?; @@ -309,6 +395,7 @@ pub fn default_registry() -> CmdRegistry { &DrillIntoCell { key: crate::model::cell::CellKey::new(vec![]), }, + MaxArgs::Any, |args| { if args.is_empty() { return Err("drill-into-cell requires Cat/Item coordinates".into()); @@ -325,13 +412,13 @@ pub fn default_registry() -> CmdRegistry { ); r.register_nullary(|| Box::new(ViewNavigate { forward: false })); r.register_nullary(|| Box::new(ViewNavigate { forward: true })); - r.register_pure(&EnterMode(AppMode::Normal), |args| { + r.register_pure(&EnterMode(AppMode::Normal), MaxArgs::AtMost(1), |args| { require_args(args, 1)?; Ok(Box::new(EnterMode(parse_mode_name(&args[0])?))) }); // ── Search ─────────────────────────────────────────────────────────── - r.register_pure(&SearchNavigate(true), |args| { + r.register_pure(&SearchNavigate(true), MaxArgs::AtMost(1), |args| { let forward = args.first().map(|s| s != "backward").unwrap_or(true); Ok(Box::new(SearchNavigate(forward))) }); @@ -344,6 +431,7 @@ pub fn default_registry() -> CmdRegistry { panel: Panel::Formula, target: None, }, + MaxArgs::AtMost(3), |args| { // Parse: toggle-panel-and-focus [open] [focused] require_args(args, 1)?; @@ -360,6 +448,7 @@ pub fn default_registry() -> CmdRegistry { &TogglePanelVisibility { panel: Panel::Formula, }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let panel = parse_panel(&args[0])?; @@ -373,6 +462,7 @@ pub fn default_registry() -> CmdRegistry { panel: Panel::Formula, delta: 0, }, + MaxArgs::AtMost(2), |args| { require_args(args, 2)?; let panel = parse_panel(&args[0])?; @@ -396,7 +486,7 @@ pub fn default_registry() -> CmdRegistry { r.register_nullary(|| Box::new(DeleteViewAtCursor)); // ── Tile select ────────────────────────────────────────────────────── - r.register_pure(&MoveTileCursor(0), |args| { + r.register_pure(&MoveTileCursor(0), MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let delta = args[0].parse::().map_err(|e| e.to_string())?; Ok(Box::new(MoveTileCursor(delta))) @@ -406,6 +496,7 @@ pub fn default_registry() -> CmdRegistry { &TileAxisOp { axis: Some(crate::view::Axis::Row), }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let axis = parse_axis(&args[0])?; @@ -423,6 +514,7 @@ pub fn default_registry() -> CmdRegistry { &AppendChar { buffer: String::new(), }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; Ok(Box::new(AppendChar { @@ -434,6 +526,7 @@ pub fn default_registry() -> CmdRegistry { &PopChar { buffer: String::new(), }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; Ok(Box::new(PopChar { @@ -454,6 +547,7 @@ pub fn default_registry() -> CmdRegistry { edit_mode: AppMode::editing(), opened: None, }, + MaxArgs::Any, |args| { if args.len() < 3 { return Err("commit-cell-edit requires a mode, value, and coords".into()); @@ -479,6 +573,7 @@ pub fn default_registry() -> CmdRegistry { edit_mode: AppMode::editing(), opened: None, }, + MaxArgs::AtMost(1), |_| Err("commit-and-advance-right requires context".into()), Some(|args, ctx| commit_at_cursor(args, ctx, AdvanceDir::Right)), ); @@ -486,6 +581,7 @@ pub fn default_registry() -> CmdRegistry { &CommitBuffer { kind: CommitKind::Category, }, + MaxArgs::AtMost(1), |args| { require_args(args, 1)?; let kind = parse_commit_kind(&args[0], &AppMode::Normal)?; @@ -592,6 +688,54 @@ pub(super) mod tests { ); } + /// Evidence: `context/test-evidence/cli.md`. + #[test] + fn no_command_silently_drops_an_argument_it_was_given() { + let r = default_registry(); + let wb = two_cat_model(); + let layout = make_layout(&wb); + let reg = make_registry(); + let ctx = make_ctx(&wb, &layout, ®); + let variadic = [ + "add-items", + "set-cell", + "set-format", + "clear-cell", + "yank", + "paste", + "drill-into-cell", + "commit-cell-edit", + ]; + // Their own refusal states the shape, and it runs before the rule. + let own_shape = ["add-formula", "fit-measure", "commit-and-advance-right"]; + let mut checked = 0; + for name in r.names().collect::>() { + if variadic.contains(&name) { + continue; + } + for args in candidate_args() { + // Four more than any list accepted, so an optional argument + // cannot absorb them; the last one is surplus whatever the command. + let mut more = args.clone(); + more.extend(["EXTRA1", "EXTRA2", "EXTRA3", "EXTRA4"].map(String::from)); + let arms = [ + r.parse(name, &args).map(|_| r.parse(name, &more)), + r.interactive(name, &args, &ctx) + .map(|_| r.interactive(name, &more, &ctx)), + ]; + for extended in arms.into_iter().flatten() { + checked += 1; + match extended { + Ok(_) => panic!("`{name} {more:?}` dropped arguments without a word"), + Err(e) if own_shape.contains(&name) || e.contains("\"EXTRA4\"") => {} + Err(e) => panic!("`{name}` refused an extra argument without naming it: {e}"), + } + } + } + } + assert!(checked > 100, "only {checked} accepted lists were extended"); + } + /// A variant mix-up shows up as a **collision**, not a mismatch: the wrong entry /// takes its sibling's name and the sibling vanishes. Uniqueness catches it. #[test] diff --git a/crates/improvise-command/src/command/cmd/text_buffer.rs b/crates/improvise-command/src/command/cmd/text_buffer.rs index 300b1336..5ea886e2 100644 --- a/crates/improvise-command/src/command/cmd/text_buffer.rs +++ b/crates/improvise-command/src/command/cmd/text_buffer.rs @@ -372,9 +372,12 @@ impl Cmd for ExecuteCommand { // Built against the live cursor, exactly as a script's is — improvise-6uva. let built: Result>, String> = crate::command::parse::segments(&raw) - .into_iter() - .map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx)) - .collect::, _>>() + .and_then(|segments| { + segments + .into_iter() + .map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx)) + .collect::, _>>() + }) .map(|built| built.into_iter().flatten().collect()); match built { Ok(cmds) => { diff --git a/crates/improvise-command/src/command/parse.rs b/crates/improvise-command/src/command/parse.rs index 3f236d47..f64eea7a 100644 --- a/crates/improvise-command/src/command/parse.rs +++ b/crates/improvise-command/src/command/parse.rs @@ -1,5 +1,10 @@ //! Quasi-lisp prefix command parser: `word arg1 arg2 …`, `.` between commands, -//! `Category/Item` coordinate pairs, `"quoted strings"`. +//! `Category/Item` coordinate pairs, `"quoted strings"` and `|pipe-quoted names|`. + +use std::iter::Peekable; +use std::str::CharIndices; + +use improvise_formula::quote::unescape_pipe_body; use super::cmd::{Cmd, CmdContext, CmdRegistry, default_registry}; @@ -11,39 +16,19 @@ pub fn parse_line(line: &str) -> Result>, String> { /// Parse a line into commands using a given registry. pub fn parse_line_with(registry: &CmdRegistry, line: &str) -> Result>, String> { - let line = line.trim(); - if line.is_empty() || line.starts_with('#') || line.starts_with("//") { - return Ok(vec![]); - } - - let mut commands = Vec::new(); - for segment in split_on_dot(line) { - let segment = segment.trim(); - if segment.is_empty() { - continue; - } - let tokens = tokenize(segment); - if tokens.is_empty() { - continue; - } - let word = &tokens[0]; - let args = &tokens[1..]; - commands.push(registry.parse(word, args)?); - } - Ok(commands) + groups(line.trim())? + .iter() + .map(|words| registry.parse(&words[0].text, &texts(&words[1..]))) + .collect() } /// The command segments of a line, with comments and blank segments dropped. -pub fn segments(line: &str) -> Vec<&str> { +pub fn segments(line: &str) -> Result, String> { let line = line.trim(); - if line.is_empty() || line.starts_with('#') || line.starts_with("//") { - return Vec::new(); - } - split_on_dot(line) - .into_iter() - .map(str::trim) - .filter(|s| !s.is_empty()) - .collect() + Ok(groups(line)? + .iter() + .map(|words| &line[words[0].start..words[words.len() - 1].end]) + .collect()) } /// Build one segment's command against a LIVE context — `context/code/cli.md`. @@ -51,7 +36,7 @@ pub fn parse_segment_interactive( segment: &str, ctx: &CmdContext, ) -> Result>, String> { - let tokens = tokenize(segment.trim()); + let tokens = texts(&lex(segment.trim())?); let Some(word) = tokens.first() else { return Ok(None); }; @@ -67,65 +52,99 @@ pub fn parse_segment_interactive( }) } -/// Split a line on ` . ` separators (dot must be a standalone word, -/// surrounded by whitespace or at line boundaries). Respects quoted strings. -fn split_on_dot(line: &str) -> Vec<&str> { - let mut segments = Vec::new(); - let mut start = 0; - let mut in_quote = false; - let bytes = line.as_bytes(); - - for (i, c) in line.char_indices() { - match c { - '"' => in_quote = !in_quote, - '.' if !in_quote => { - let before_ws = i == 0 || bytes[i - 1].is_ascii_whitespace(); - let after_ws = i + 1 >= bytes.len() || bytes[i + 1].is_ascii_whitespace(); - if before_ws && after_ws { - segments.push(&line[start..i]); - start = i + 1; - } - } - _ => {} - } - } - segments.push(&line[start..]); - segments +/// One word of a line and the byte span it was read from. A `separator` is a +/// bare `.`; a quoted one is an argument like any other. +struct Word { + text: String, + start: usize, + end: usize, + separator: bool, } -/// Tokenize a command segment into words, handling quoted strings. -fn tokenize(input: &str) -> Vec { - let mut tokens = Vec::new(); - let mut chars = input.chars().peekable(); +fn texts(words: &[Word]) -> Vec { + words.iter().map(|w| w.text.clone()).collect() +} - while let Some(&c) = chars.peek() { +/// The words of a trimmed line, split at separators, blank commands and +/// comment lines dropped. +fn groups(line: &str) -> Result>, String> { + if line.starts_with('#') || line.starts_with("//") { + return Ok(Vec::new()); + } + let mut groups: Vec> = vec![Vec::new()]; + for word in lex(line)? { + match word.separator { + true => groups.push(Vec::new()), + false => groups.last_mut().expect("never empty").push(word), + } + } + groups.retain(|g| !g.is_empty()); + Ok(groups) +} + +/// Split `line` into words. The quoting rules: `context/code/cli.md` §12. +fn lex(line: &str) -> Result, String> { + let mut words = Vec::new(); + let mut chars = line.char_indices().peekable(); + while let Some(&(start, c)) = chars.peek() { if c.is_whitespace() { chars.next(); continue; } - if c == '"' { - chars.next(); // consume opening quote - let mut s = String::new(); - for ch in chars.by_ref() { - if ch == '"' { - break; - } - s.push(ch); + let (text, bare) = match c { + '"' => (double_quoted(&mut chars), false), + _ => bare_word(&mut chars, line)?, + }; + let end = chars.peek().map_or(line.len(), |&(i, _)| i); + words.push(Word { + separator: bare && text == ".", + text, + start, + end, + }); + } + Ok(words) +} + +/// `"…"` read verbatim: no escapes, and a `|` inside stays a `|`. +fn double_quoted(chars: &mut Peekable) -> String { + chars.next(); + chars + .by_ref() + .map(|(_, c)| c) + .take_while(|&c| c != '"') + .collect() +} + +/// Text up to whitespace, each `|…|` piece unquoted in place; `bare` is false +/// once a piece was quoted, so a quoted `|.|` is an argument and not a separator. +fn bare_word(chars: &mut Peekable, line: &str) -> Result<(String, bool), String> { + let mut text = String::new(); + let mut bare = true; + while let Some((at, c)) = chars.next_if(|&(_, c)| !c.is_whitespace()) { + match c { + '|' => { + text.push_str(&pipe_body(chars, line, at)?); + bare = false; } - tokens.push(s); - } else { - let mut s = String::new(); - while let Some(&ch) = chars.peek() { - if ch.is_whitespace() { - break; - } - s.push(ch); - chars.next(); - } - tokens.push(s); + _ => text.push(c), } } - tokens + Ok((text, bare)) +} + +/// The body of the `|…|` opened at `open`, escapes applied by the one shared +/// reader (gotcha 34). +fn pipe_body(chars: &mut Peekable, line: &str, open: usize) -> Result { + let mut body = String::new(); + while let Some((_, c)) = chars.next() { + match c { + '|' => return Ok(unescape_pipe_body(&body)), + '\\' => body.extend(std::iter::once(c).chain(chars.next().map(|(_, e)| e))), + _ => body.push(c), + } + } + Err(format!("unterminated |…| in `{}`", &line[open..])) } #[cfg(test)] @@ -302,11 +321,14 @@ mod tests { #[test] fn segments_drops_comments_and_blanks() { - assert_eq!(segments("# a comment"), Vec::<&str>::new()); - assert_eq!(segments("// also a comment"), Vec::<&str>::new()); - assert_eq!(segments(" "), Vec::<&str>::new()); - assert_eq!(segments("a . . b"), vec!["a", "b"]); - assert_eq!(segments("move-selection 2 0"), vec!["move-selection 2 0"]); + assert_eq!(segments("# a comment"), Ok(Vec::<&str>::new())); + assert_eq!(segments("// also a comment"), Ok(Vec::<&str>::new())); + assert_eq!(segments(" "), Ok(Vec::<&str>::new())); + assert_eq!(segments("a . . b"), Ok(vec!["a", "b"])); + assert_eq!( + segments("move-selection 2 0"), + Ok(vec!["move-selection 2 0"]) + ); } /// Evidence: `context/test-evidence/cli.md`. @@ -347,4 +369,93 @@ mod tests { .expect_err("an unknown word is refused"); assert!(err.contains("no-such-command"), "got: {err}"); } + + // ── pipe quoting and arity (improvise-31oe.3) ───────────────────────── + + /// The one command `line` builds, as its `Debug` shows it. + fn built(line: &str) -> String { + let cmds = parse_line(line).unwrap_or_else(|e| panic!("`{line}` must parse: {e}")); + assert_eq!(cmds.len(), 1, "`{line}` is one command"); + format!("{:?}", cmds[0]) + } + + /// Evidence: `context/test-evidence/cli.md`. + #[test] + fn a_pipe_quoted_argument_is_one_argument_without_its_pipes() { + assert_eq!( + built("create-view |By area|"), + r#"CreateViewCmd(["By area"])"# + ); + assert_eq!( + built("set-page |Sales Region| |East Coast|"), + r#"SetPageCmd(["Sales Region", "East Coast"])"# + ); + } + + /// Evidence: `context/test-evidence/cli.md`. + #[test] + fn a_pipe_quoted_argument_reads_the_escapes_improv_does() { + assert_eq!(built(r"create-view |a\|b|"), r#"CreateViewCmd(["a|b"])"#); + assert_eq!( + built(r"create-view |back\\slash|"), + r#"CreateViewCmd(["back\\slash"])"# + ); + assert_eq!( + built(r"create-view |two\nlines|"), + r#"CreateViewCmd(["two\nlines"])"# + ); + } + + /// A coordinate quotes each half, as a `.improv` data line does. + #[test] + fn a_coordinate_may_pipe_quote_either_half() { + assert_eq!( + built("set-cell 5 |Sales Region|/|East Coast| Measure/Revenue"), + r#"SetCellCmd(["5", "Sales Region/East Coast", "Measure/Revenue"])"# + ); + } + + #[test] + fn a_dot_inside_pipes_does_not_split_the_line() { + assert_eq!(built("create-view |a . b|"), r#"CreateViewCmd(["a . b"])"#); + assert_eq!( + segments("create-view |a . b|"), + Ok(vec!["create-view |a . b|"]) + ); + } + + /// Double quotes stay verbatim: formula text carries its own pipes. + #[test] + fn a_double_quoted_argument_keeps_the_pipes_inside_it() { + assert_eq!( + built(r#"add-formula Measure "Net = |Gross Rev| - Cost""#), + r#"AddFormulaCmd(["Measure", "Net = |Gross Rev| - Cost"])"# + ); + } + + #[test] + fn an_unterminated_pipe_is_refused() { + let err = parse_line("create-view |By area").expect_err("an open pipe is refused"); + assert!( + err.contains("|By area"), + "the refusal names the text: {err}" + ); + } + + /// Evidence: `context/test-evidence/cli.md`. + #[test] + fn a_command_given_more_arguments_than_it_takes_is_refused() { + let err = parse_line("create-view By area").expect_err("two words for one argument"); + assert!(err.starts_with("create-view:"), "names the command: {err}"); + assert!(err.contains("\"area\""), "names the extra argument: {err}"); + let err = parse_line("set-axis Type row sideways").expect_err("a third argument"); + assert!( + err.contains("\"sideways\""), + "names the extra argument: {err}" + ); + assert!( + parse_line("create-view |By area|").is_ok(), + "the control parses" + ); + } } diff --git a/crates/improvise-command/src/ui/app.rs b/crates/improvise-command/src/ui/app.rs index 89406d91..6f19d1bd 100644 --- a/crates/improvise-command/src/ui/app.rs +++ b/crates/improvise-command/src/ui/app.rs @@ -3281,7 +3281,9 @@ mod tests { /// when any segment refused, which is what its exit code reads. fn run_line(app: &mut App, line: &str) -> bool { let mut refused = false; - for segment in crate::command::parse::segments(line) { + let segments = crate::command::parse::segments(line) + .unwrap_or_else(|e| panic!("`{line}` must lex: {e}")); + for segment in segments { let effects = { let ctx = app.cmd_context(KeyCode::Null, KeyModifiers::NONE); crate::command::parse::parse_segment_interactive(segment, &ctx) diff --git a/src/main.rs b/src/main.rs index f2144274..dfb88e68 100644 --- a/src/main.rs +++ b/src/main.rs @@ -396,12 +396,16 @@ fn run_headless_commands(cmds: &[String], file: &Option) -> Result<()> for line in cmds { // The whole line is syntax-checked before any of it runs, exactly as // before; only CONSTRUCTION moved — `context/code/cli.md`. - if let Err(e) = command::parse_line(line) { - eprintln!("Parse error: {e}"); - exit_code = 1; - continue; - } - for segment in command::parse::segments(line) { + let segments = command::parse_line(line).and_then(|_| command::parse::segments(line)); + let segments = match segments { + Ok(segments) => segments, + Err(e) => { + eprintln!("Parse error: {e}"); + exit_code = 1; + continue; + } + }; + for segment in segments { let mut failed = None; let effects = { // Built against the cursor it is about to run against, so a