diff --git a/src/engraving/mod.rs b/src/engraving/mod.rs index 42b5179..5df974a 100644 --- a/src/engraving/mod.rs +++ b/src/engraving/mod.rs @@ -22,7 +22,9 @@ pub use record::{ }; pub use store::{Appender, Store}; -pub(crate) use record::{fail_reason, format_record, format_supplied, serialize_value}; +pub(crate) use record::{ + fail_reason, format_record, format_supplied, serialize_value, split_top_level, unescape_literal, +}; pub(crate) use store::{construct_state_path, parse_run_uri}; #[cfg(test)] diff --git a/src/engraving/record.rs b/src/engraving/record.rs index bd7490e..b464eec 100644 --- a/src/engraving/record.rs +++ b/src/engraving/record.rs @@ -263,7 +263,7 @@ fn escape_literal(out: &mut String, text: &str) { // Reverse `escape_literal`. An unknown escape (or a trailing backslash) is a // malformed record. -fn unescape_literal(text: &str) -> Result { +pub(crate) fn unescape_literal(text: &str) -> Result { let mut out = String::with_capacity(text.len()); let mut chars = text.chars(); while let Some(c) = chars.next() { @@ -644,7 +644,7 @@ pub(crate) fn deserialize_value(text: &str) -> Result // Split `text` on `delim`, but only at top level: not inside double quotes // (honouring `\"` escapes), not inside single-quoted Enumerati values (which // carry no escapes or interior quote), nor inside nested `[]`, `()`, or `{}`. -fn split_top_level(text: &str, delim: char) -> Result, RecordError> { +pub(crate) fn split_top_level(text: &str, delim: char) -> Result, RecordError> { let mut parts = Vec::new(); let bytes = text.as_bytes(); let mut depth = 0i32; diff --git a/src/main.rs b/src/main.rs index ae281de..e2f7886 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1209,8 +1209,8 @@ fn main() { debug!(?output); // Each output form selects the columns that suit it; --columns, - // when given, overrides that choice. PFFTT is the exception: its - // fields are defined by the format. + // when given, overrides that choice. `pfftt` and `native` are the + // exceptions: their fields are fixed by the format. let selected = submatches.get_many::("columns"); let columns: Vec = match (selected, &output) { (Some(_), Output::Store) => { @@ -1220,6 +1220,13 @@ fn main() { ); std::process::exit(1); } + (Some(_), Output::Native) => { + eprintln!( + "{}: --columns cannot be used with --output=native", + "error".bright_red() + ); + std::process::exit(1); + } (Some(names), _) => names .copied() .collect(), diff --git a/src/problem/messages.rs b/src/problem/messages.rs index 2f0db6c..c2e08de 100644 --- a/src/problem/messages.rs +++ b/src/problem/messages.rs @@ -1582,6 +1582,29 @@ parameters. .trim_ascii() .to_string(), ), + RunnerError::MalformedArgument { + parameter, + argument, + } => ( + format!("Malformed argument: {} given for {}", argument, parameter), + r#" +An argument written as a list must have its brackets and quotes balanced. +Quote an element only when it contains a comma or newlines; inside quotes a +backslash is an escape, and only \\, \", \n, and \r are recognized. + "# + .trim_ascii() + .to_string(), + ), + RunnerError::MalformedList { text } => ( + format!("Malformed list: {}", text), + r#" +A value written as a list must have its brackets and quotes balanced. Quote an +element only when it contains a comma or newlines; inside quotes a backslash is +an escape, and only \\, \", \n, and \r are recognized. + "# + .trim_ascii() + .to_string(), + ), RunnerError::NotIterable => ( "Iteration requires a list".to_string(), r#" diff --git a/src/runner/checks/driver.rs b/src/runner/checks/driver.rs index ff83796..fd664f0 100644 --- a/src/runner/checks/driver.rs +++ b/src/runner/checks/driver.rs @@ -643,31 +643,52 @@ fn list_prompt() -> Prompt { } } +// Type `text` into a list field and submit it, returning what the prompt +// settles on — None if it refused the buffer, leaving the edit open. +fn submit_list(text: &str) -> Option { + let mut it = list_prompt(); + for c in text.chars() { + it.handle(KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE)); + } + it.handle(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)) +} + +// As above, for the cases that are expected to settle. +fn gather_list(text: &str) -> Value { + match submit_list(text) { + Some(UserInput::Done(value)) => value, + other => panic!("expected Done, got {:?}", other), + } +} + #[test] fn list_prompt_empty_submits_empty_list() { - // Enter on an untouched list field yields `[]`, which coerce_to_list reads - // as zero iterations. - let mut it = list_prompt(); - assert_eq!( - it.handle(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), - Some(UserInput::Done(Value::Literali("[]".to_string()))) - ); + // Enter on an untouched list field yields the empty list, which + // coerce_to_list reads as zero iterations. + assert_eq!(gather_list(""), Value::Arraeum(Vec::new())); } #[test] -fn list_prompt_wraps_typed_buffer() { - // Whatever the user types is wrapped in brackets on submit, so the - // result parses through the existing list-literal path. - let mut it = list_prompt(); - for c in "east, west".chars() { - it.handle(KeyEvent::new(KeyCode::Char(c), KeyModifiers::NONE)); - } +fn list_prompt_gathers_elements() { + // What the user types is gathered as a list of its elements, not as one + // string: the structure survives into the binding and onto the record. + // Each element takes its natural type, the same as an argument would. assert_eq!( - it.handle(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)), - Some(UserInput::Done(Value::Literali("[east, west]".to_string()))) + gather_list("east, 5"), + Value::Arraeum(vec![ + Value::Literali("east".to_string()), + Value::Quanticle(Numeric::Integral(5)), + ]) ); } +#[test] +fn list_prompt_refuses_malformed_buffer() { + // A buffer that doesn't parse leaves the edit open rather than settling + // on a mangled list. + assert_eq!(submit_list(r#""east, west"#), None); +} + #[test] fn list_prompt_draws_bracketed_buffer() { let mut it = list_prompt(); diff --git a/src/runner/checks/evaluator.rs b/src/runner/checks/evaluator.rs index 9f1bf3a..86d9efb 100644 --- a/src/runner/checks/evaluator.rs +++ b/src/runner/checks/evaluator.rs @@ -1,7 +1,9 @@ use crate::language::{Identifier, Numeric as LangNumeric, Span}; use crate::program::{Entry, Executable, ExecutableRef, Fragment, Operation}; use crate::runner::context::Context; -use crate::runner::evaluator::{Environment, coerce_to_list, combine, evaluate}; +use crate::runner::evaluator::{ + Environment, coerce_to_list, combine, evaluate, is_list_literal, parse_list_literal, +}; use crate::runner::library::Library; use crate::runner::runner::RunnerError; use crate::value; @@ -538,6 +540,84 @@ fn coerce_parses_bracketed_literal() { let empty = coerce_to_list(value::Value::Literali("[]".to_string())).expect("coerced"); assert_eq!(empty, Vec::::new()); + + // Bracketed but malformed is an error, not one iteration over the raw + // text; text that was never bracketed is the one-element list. + let error = coerce_to_list(value::Value::Literali(r#"["Sydney, NSW]"#.to_string())) + .expect_err("expected malformed error"); + let RunnerError::MalformedList { text } = error else { + panic!("expected MalformedList, got {:?}", error); + }; + assert_eq!(text, r#"["Sydney, NSW]"#); + + let plain = coerce_to_list(value::Value::Literali("east".to_string())).expect("coerced"); + assert_eq!(plain, vec![value::Value::Literali("east".to_string())]); +} + +#[test] +fn parse_list_separates_elements_at_top_level() { + // A ',' inside a quoted element is part of that element, not a separator. + assert_eq!( + parse_list_literal(r#"["Sydney, NSW", "Hobart, TAS"]"#).expect("parsed"), + vec![ + value::Value::Literali("Sydney, NSW".to_string()), + value::Value::Literali("Hobart, TAS".to_string()), + ] + ); + + // Nor does a ',' inside a nested bracket; the element parses as a list in + // its own right. + assert_eq!( + parse_list_literal("[[a, b], c]").expect("parsed"), + vec![ + value::Value::Arraeum(vec![ + value::Value::Literali("a".to_string()), + value::Value::Literali("b".to_string()), + ]), + value::Value::Literali("c".to_string()), + ] + ); + + // A trailing separator adds no element. + assert_eq!( + parse_list_literal("[east, west,]").expect("parsed"), + vec![ + value::Value::Literali("east".to_string()), + value::Value::Literali("west".to_string()), + ] + ); +} + +#[test] +fn parse_list_applies_record_escapes() { + // Inside quotes a backslash escapes, using the same escapes the record + // format writes, so an element can hold a quote or a newline. + assert_eq!( + parse_list_literal(r#"["say \"hi\"", "one\ntwo"]"#).expect("parsed"), + vec![ + value::Value::Literali("say \"hi\"".to_string()), + value::Value::Literali("one\ntwo".to_string()), + ] + ); + + // Outside quotes it is an ordinary character. + assert_eq!( + parse_list_literal(r"[C:\path]").expect("parsed"), + vec![value::Value::Literali(r"C:\path".to_string())] + ); +} + +#[test] +fn parse_list_rejects_malformed() { + // An unbalanced quote, an unbalanced bracket, or an unknown escape is not + // a list, so callers can tell it apart from text that never was one. + assert_eq!(parse_list_literal(r#"["Sydney, NSW]"#), None); + assert_eq!(parse_list_literal("[[a, b]"), None); + assert_eq!(parse_list_literal(r#"["C:\path"]"#), None); + + assert!(is_list_literal("[a, b]")); + assert!(!is_list_literal("[a, b")); + assert!(!is_list_literal("sydney")); } #[test] diff --git a/src/runner/checks/runner.rs b/src/runner/checks/runner.rs index f0bd3ca..1de8345 100644 --- a/src/runner/checks/runner.rs +++ b/src/runner/checks/runner.rs @@ -2270,6 +2270,54 @@ test : ); } +#[test] +fn list_argument_binds_as_a_list() { + let source = r#" +% technique v1 + +sweep(regions) : + +1. step + "# + .trim_ascii(); + let document = parsing::parse(Path::new("Test.tq"), source).expect("parse"); + let mut program = translate(&document).expect("translate"); + resolve(&mut program).expect("resolve"); + + // A bracketed argument binds as a list. + let args = ["[sydney, hobart]".to_string()]; + let env = bind_parameters(&program, &args).expect("bind"); + assert_eq!( + env.lookup("regions"), + Some(&Value::Arraeum(vec![ + Value::Literali("sydney".to_string()), + Value::Literali("hobart".to_string()), + ])) + ); + + // Text that was never a list stays text, brackets being the cue. + let args = ["sydney".to_string()]; + let env = bind_parameters(&program, &args).expect("bind"); + assert_eq!( + env.lookup("regions"), + Some(&Value::Literali("sydney".to_string())) + ); + + // An argument that reads as a list but doesn't parse is rejected rather + // than silently taken as text: here the quote is unbalanced. + let args = [r#"["Sydney, NSW]"#.to_string()]; + let error = bind_parameters(&program, &args).expect_err("expected malformed error"); + let RunnerError::MalformedArgument { + parameter, + argument, + } = error + else { + panic!("expected MalformedArgument, got {:?}", error); + }; + assert_eq!(parameter, "regions"); + assert_eq!(argument, r#"["Sydney, NSW]"#); +} + #[test] fn argument_echo_binds_each_parameter() { let source = r#" diff --git a/src/runner/driver.rs b/src/runner/driver.rs index 933c4fd..23bba25 100644 --- a/src/runner/driver.rs +++ b/src/runner/driver.rs @@ -1139,8 +1139,8 @@ enum Field { cursor: usize, edited: bool, original: Value, - /// A list field renders its buffer between `[` and `]` and submits the - /// buffer wrapped as `[buffer]`, so an empty answer yields `[]`. + /// A list field renders its buffer between `[` and `]` and submits it + /// as a list, so an empty answer yields the empty list. bracketed: bool, }, Frozen { @@ -1414,19 +1414,19 @@ impl Prompt { } => match code { KeyCode::Enter => { if *bracketed { - // A list field always submits its buffer wrapped, so an - // empty answer is `[]` and `coerce_to_list` iterates it - // zero times. - Some(UserInput::Done(Value::Literali(format!("[{}]", buffer)))) + // A list field submits its buffer as elements, the + // same way a command-line argument is read. A buffer + // that does not parse is not accepted. + super::evaluator::parse_list_literal(&format!("[{}]", buffer)) + .map(|items| UserInput::Done(Value::Arraeum(items))) } else if !*edited { // Unchanged: return the original value verbatim, with // its type and exact value intact. Some(UserInput::Done(std::mem::replace(original, Value::Unitus))) } else if let Value::Quanticle(_) = original { // An edited numeric value stays numeric: re-parse the - // buffer with the language's own number grammar. A - // buffer that is not a valid number is not accepted — - // the edit stays open for correction. + // buffer. A buffer that is not a valid number is not + // accepted; the edit stays open for correction. match crate::parsing::parse_numeric(buffer) { Some(numeric) => Some(UserInput::Done(Value::Quanticle( crate::value::Numeric::from(&numeric), diff --git a/src/runner/evaluator.rs b/src/runner/evaluator.rs index 6ebed26..f7dd3ed 100644 --- a/src/runner/evaluator.rs +++ b/src/runner/evaluator.rs @@ -224,9 +224,9 @@ pub fn evaluate<'i>( /// Reduce a value to the elements a `foreach` iterates. A list yields its /// members; `Unit` (the absence of a value) is empty; a blank string (an empty /// prompt answer) is likewise empty, so a `foreach` over it runs zero times; a -/// non-blank string may be a `[a, b]` literal, which parses into its elements, -/// else it is a one-element list; a bare quantity widens likewise. A tablet, -/// tuple, or future is not iterable. +/// non-blank string may be a `[a, b]` literal, which parses into its elements +/// and is an error if it doesn't, else it is a one-element list; a bare +/// quantity widens likewise. A tablet, tuple, or future is not iterable. pub(super) fn coerce_to_list(value: Value) -> Result, RunnerError> { match value { Value::Arraeum(items) => Ok(items), @@ -238,34 +238,44 @@ pub(super) fn coerce_to_list(value: Value) -> Result, RunnerError> { { Ok(Vec::new()) } - Value::Literali(text) => match parse_list_literal(&text) { - Some(items) => Ok(items), - None => Ok(vec![Value::Literali(text)]), - }, + Value::Literali(text) if is_list_literal(&text) => { + parse_list_literal(&text).ok_or(RunnerError::MalformedList { text }) + } + Value::Literali(text) => Ok(vec![Value::Literali(text)]), value @ Value::Quanticle(_) => Ok(vec![value]), _ => Err(RunnerError::NotIterable), } } -/// Coerce a raw user-supplied string — a command-line argument or an unquoted -/// list element — into its natural Value type: a `[ … ]` literal becomes a -/// list, a number becomes a quantity, anything else stays a string. -pub(super) fn parse_value(text: &str) -> Value { +/// Coerce a raw user-supplied string (a command-line argument or an unquoted +/// list element) into its natural Value type. A `[ ... ]` literal becomes a +/// list, a number becomes a quantity, anything else stays a string. `None` if +/// the text appears to be a list but does not parse. +pub(super) fn parse_value(text: &str) -> Option { let trimmed = text.trim(); - if let Some(items) = parse_list_literal(trimmed) { - return Value::Arraeum(items); + if is_list_literal(trimmed) { + return parse_list_literal(trimmed).map(Value::Arraeum); } if let Some(numeric) = crate::parsing::parse_numeric(trimmed) { - return Value::Quanticle(Numeric::from(&numeric)); + return Some(Value::Quanticle(Numeric::from(&numeric))); } - Value::Literali(text.to_string()) + Some(Value::Literali(text.to_string())) +} + +/// Whether text reads as a list literal, the guard distinguishing a malformed +/// list from ordinary text that was never one. +pub(super) fn is_list_literal(text: &str) -> bool { + let text = text.trim(); + text.starts_with('[') && text.ends_with(']') } -/// Parse a `[ "a", b, ... ]` literal into its elements. A quoted element is a -/// string verbatim; an unquoted one takes its natural type via `parse_value`. -/// Returns `None` for text that is not bracketed. TODO This splits naively on -/// ',' so commas inside element text are not supported. -fn parse_list_literal(text: &str) -> Option> { +/// Parse a user-input `[ "a", b, ... ]` string into its elements. Elements +/// are separated at top level only, so a ',' inside a quoted element or a +/// nested bracket does not split. A quoted element is a string carrying the +/// record format's escapes; an unquoted one takes its natural type via +/// `parse_value`. Returns `None` for text that is not bracketed, and for text +/// that is malformed (an unbalanced quote or bracket, or an unknown escape). +pub(super) fn parse_list_literal(text: &str) -> Option> { let inner = text .trim() .strip_prefix('[')? @@ -276,20 +286,31 @@ fn parse_list_literal(text: &str) -> Option> { { return Some(Vec::new()); } - let items = inner - .split(',') + let mut elements = crate::engraving::split_top_level(inner, ',').ok()?; + // A trailing separator is admitted, but does not add an element. + if let Some(last) = elements.last() { + if last + .trim() + .is_empty() + { + elements.pop(); + } + } + elements + .into_iter() .map(|element| { let element = element.trim(); match element .strip_prefix('"') .and_then(|e| e.strip_suffix('"')) { - Some(unquoted) => Value::Literali(unquoted.to_string()), + Some(quoted) => crate::engraving::unescape_literal(quoted) + .ok() + .map(Value::Literali), None => parse_value(element), } }) - .collect(); - Some(items) + .collect() } /// Bind names to a value, shared by `Bind` evaluation and `foreach` diff --git a/src/runner/runner.rs b/src/runner/runner.rs index a5068bd..f599849 100644 --- a/src/runner/runner.rs +++ b/src/runner/runner.rs @@ -87,6 +87,13 @@ pub enum RunnerError { procedure: String, actual: usize, }, + MalformedArgument { + parameter: String, + argument: String, + }, + MalformedList { + text: String, + }, TerminalRequired, UserQuit, } @@ -2159,12 +2166,16 @@ pub(super) fn bind_parameters( .iter() .zip(arguments) { - env.extend( - param - .value - .to_string(), - super::evaluator::parse_value(argument), - ); + let parameter = param + .value + .to_string(); + let value = super::evaluator::parse_value(argument).ok_or_else(|| { + RunnerError::MalformedArgument { + parameter: parameter.clone(), + argument: argument.to_string(), + } + })?; + env.extend(parameter, value); } Ok(env) }