fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(command): read |pipe quoting| and refuse surplus arguments (improvise-31oe.3)

Browse Source 
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VdfEGXA7aJ6ftMsCc9PJze
This commit is contained in:
Claude (agent-cmdlang)
2026-09-24 21:43:18 -07:00
committed by Edward Langley
parent b639266b0d
commit 197ce61d65
8 changed files with 461 additions and 134 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/improvise-command/src/command/cmd/core.rs
+38 -5
View File
@@ -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<String> = 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<dyn Fn(&[String]) -> Result<Box<dyn Cmd>, String>>;
type BoxInteractiveFn = Box<dyn Fn(&[String], &CmdContext) -> Result<Box<dyn Cmd>, 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<C: Cmd + 'static>(
&mut self,
prototype: &C,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
) {
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<C: AtCursor + 'static>(
&mut self,
prototype: &C,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
) {
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<C: Cmd + 'static>(
&mut self,
name: &'static str,
max: MaxArgs,
parse: fn(&[String]) -> Result<C, String>,
fallback: Option<FallbackFn<C>>,
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<dyn Cmd>)),
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}"))
crates/improvise-command/src/command/cmd/effect_cmds.rs
+30 -2
View File
@@ -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<String>);
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<Box<dyn Cmd>, 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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
if args.is_empty() {
@@ -450,6 +472,7 @@ effect_cmd!(
effect_cmd!(
HelpCmd,
"help",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
vec![
@@ -462,6 +485,7 @@ effect_cmd!(
effect_cmd!(
HelpPageNextCmd,
"help-page-next",
MaxArgs::AtMost(0),
|_args: &[String]| -> Result<(), String> { Ok(()) },
|_args: &Vec<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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<String>, _ctx: &CmdContext| -> Vec<Box<dyn Effect>> {
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())
crates/improvise-command/src/command/cmd/fit.rs
+3 -1
View File
@@ -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 {
crates/improvise-command/src/command/cmd/registry.rs
+175 -31
View File
@@ -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::<i32>().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::<i32>().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::<i32>().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 <panel> [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::<i32>().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, &reg);
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::<Vec<_>>() {
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]
crates/improvise-command/src/command/cmd/text_buffer.rs
+6 -3
View File
@@ -372,9 +372,12 @@ impl Cmd for ExecuteCommand {
// Built against the live cursor, exactly as a script's is — improvise-6uva.
let built: Result<Vec<Box<dyn Cmd>>, String> = crate::command::parse::segments(&raw)
.into_iter()
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
.collect::<Result<Vec<_>, _>>()
.and_then(|segments| {
segments
.into_iter()
.map(|seg| crate::command::parse::parse_segment_interactive(seg, ctx))
.collect::<Result<Vec<_>, _>>()
})
.map(|built| built.into_iter().flatten().collect());
match built {
Ok(cmds) => {
crates/improvise-command/src/command/parse.rs
+196 -85
View File
@@ -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<Vec<Box<dyn Cmd>>, String> {
/// Parse a line into commands using a given registry.
pub fn parse_line_with(registry: &CmdRegistry, line: &str) -> Result<Vec<Box<dyn Cmd>>, 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<Vec<&str>, 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<Option<Box<dyn Cmd>>, 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<String> {
let mut tokens = Vec::new();
let mut chars = input.chars().peekable();
fn texts(words: &[Word]) -> Vec<String> {
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<Vec<Vec<Word>>, String> {
if line.starts_with('#') || line.starts_with("//") {
return Ok(Vec::new());
}
let mut groups: Vec<Vec<Word>> = 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<Vec<Word>, 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<CharIndices>) -> 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<CharIndices>, 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<CharIndices>, line: &str, open: usize) -> Result<String, String> {
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"
);
}
}
crates/improvise-command/src/ui/app.rs
+3 -1
View File
@@ -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)
src/main.rs
+10 -6
View File
@@ -396,12 +396,16 @@ fn run_headless_commands(cmds: &[String], file: &Option<PathBuf>) -> 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