From 081fc927a71c5b8afec01f2ea06d27f2cb42ae09 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 12 Jul 2023 08:38:00 +0200 Subject: [PATCH] Improve query inteface (#1447) * Improve :query interface Previously, the query command of the REPL would accept a first argument that could be a record access chain, followed by a second one which could also be a field path. This means that, for querying a given path `foo.bar.baz.blo`, there are 3 different but equivalent query invocations: - :query foo bar.baz.blo - :query foo.bar baz.blo - :query foo.bar.baz blo Moreover, `:query foo.bar.baz.blo` without a second argument wouldn't fail but pretend that there are no metadata, even if there are. This commit simplify the overall interfaceo of the query command by just accepting one argument which is a field path. * Update core/src/program.rs Co-authored-by: Viktor Kleen * Update core/src/repl/mod.rs Co-authored-by: Viktor Kleen --------- Co-authored-by: Viktor Kleen --- core/src/program.rs | 6 ++++++ core/src/repl/command.rs | 22 ++-------------------- core/src/repl/mod.rs | 25 +++++++++++++++++-------- core/src/repl/rustyline_frontend.rs | 2 +- core/src/repl/simple_frontend.rs | 4 ++-- 5 files changed, 28 insertions(+), 31 deletions(-) diff --git a/core/src/program.rs b/core/src/program.rs index 10d8d7d7..7b3322f2 100644 --- a/core/src/program.rs +++ b/core/src/program.rs @@ -55,6 +55,12 @@ impl QueryPath { /// Identifiers can be enclosed by double quotes when they contain characters that aren't /// allowed inside bare identifiers. The accepted grammar is the same as a sequence of record /// accesses in Nickel, although string interpolation is forbidden. + /// + /// # Post-conditions + /// + /// If this function succeeds and returns `Ok(query_path)`, then `query_path.0` is non empty. + /// Indeed, there's no such thing as a valid empty field path. If `input` is empty, or consists + /// only of spaces, `parse` returns a parse error. pub fn parse(cache: &mut Cache, input: String) -> Result { use crate::parser::{ grammar::FieldPathParser, lexer::Lexer, utils::FieldPathElem, ErrorTolerantParser, diff --git a/core/src/repl/command.rs b/core/src/repl/command.rs index 03465e6a..ce3b0ac8 100644 --- a/core/src/repl/command.rs +++ b/core/src/repl/command.rs @@ -25,10 +25,7 @@ impl CommandType { pub enum Command { Load(OsString), Typecheck(String), - Query { - target: String, - path: Option, - }, + Query(String), Print(String), Help(Option), Exit, @@ -130,22 +127,7 @@ impl FromStr for Command { } CommandType::Query => { require_arg(cmd, &arg, None)?; - let mut args_iter = arg.chars(); - - let first_arg = args_iter.by_ref().take_while(|c| *c != ' ').collect(); - let rest = args_iter.collect::(); - let rest = rest.trim(); - - let path = if !rest.is_empty() { - Some(String::from(rest)) - } else { - None - }; - - Ok(Command::Query { - target: first_arg, - path, - }) + Ok(Command::Query(arg)) } CommandType::Print => { require_arg(cmd, &arg, None)?; diff --git a/core/src/repl/mod.rs b/core/src/repl/mod.rs index 10c98710..48890889 100644 --- a/core/src/repl/mod.rs +++ b/core/src/repl/mod.rs @@ -63,7 +63,7 @@ pub trait Repl { /// Typecheck an expression and return its [apparent type][crate::typecheck::ApparentType]. fn typecheck(&mut self, exp: &str) -> Result; /// Query the metadata of an expression. - fn query(&mut self, target: String, path: Option) -> Result; + fn query(&mut self, path: String) -> Result; /// Required for error reporting on the frontend. fn cache_mut(&mut self) -> &mut Cache; } @@ -285,15 +285,21 @@ impl Repl for ReplImpl { .into()) } - fn query(&mut self, target: String, path: Option) -> Result { + fn query(&mut self, path: String) -> Result { use crate::program; + let mut query_path = QueryPath::parse(self.vm.import_resolver_mut(), path)?; + + // remove(): this is safe because there is no such thing as an empty field path. If `path` + // is empty, the parser will error out. Hence, `QueryPath::parse` always returns a non-empty + // vector. + let target = query_path.0.remove(0); + let file_id = self .vm .import_resolver_mut() - .replace_string("", target); + .replace_string("", target.label().into()); - let query_path = QueryPath::parse_opt(self.vm.import_resolver_mut(), path)?; program::query(&mut self.vm, file_id, &self.env, query_path) } @@ -415,15 +421,18 @@ pub fn print_help(out: &mut impl Write, arg: Option<&str>) -> std::io::Result<() )?; } Ok(c @ CommandType::Query) => { - writeln!(out, ":{c} [field path]")?; + writeln!(out, ":{c} ")?; print_aliases(out, c)?; writeln!(out, "Print the metadata attached to a field")?; writeln!( out, - " is valid Nickel identifier representing the record to look into." + " is a dot-separated sequence of identifiers pointing to a field. \ + Fields can be quoted if they contain special characters, \ + just like in normal Nickel source code.\n" )?; - writeln!(out, " is a dot-separated sequence of identifiers pointing to a field.\n")?; - writeln!(out, "Example: `:{c} mylib contracts.\"special#chars\".bar`")?; + writeln!(out, "Examples:")?; + writeln!(out, "- `:{c} std.array.any`")?; + writeln!(out, "- `:{c} mylib.contracts.\"special#chars.\".bar`")?; } Ok(c @ CommandType::Load) => { writeln!(out, ":{c} ")?; diff --git a/core/src/repl/rustyline_frontend.rs b/core/src/repl/rustyline_frontend.rs index aeb5bf97..ef7cf0a5 100644 --- a/core/src/repl/rustyline_frontend.rs +++ b/core/src/repl/rustyline_frontend.rs @@ -88,7 +88,7 @@ pub fn repl(histfile: PathBuf, color_opt: ColorOpt) -> Result<(), InitError> { Ok(Command::Typecheck(exp)) => { repl.typecheck(&exp).map(|types| println!("Ok: {types}")) } - Ok(Command::Query {target, path}) => repl.query(target, path).map(|field| { + Ok(Command::Query(path)) => repl.query(path).map(|field| { query_print::write_query_result( &mut stdout, &field, diff --git a/core/src/repl/simple_frontend.rs b/core/src/repl/simple_frontend.rs index e32bcf91..331d8ef9 100644 --- a/core/src/repl/simple_frontend.rs +++ b/core/src/repl/simple_frontend.rs @@ -49,8 +49,8 @@ pub fn input(repl: &mut R, line: &str) -> Result repl - .query(target, path) + Ok(Command::Query(path)) => repl + .query(path) .map(|t| { let mut buffer = Cursor::new(Vec::::new()); query_print::write_query_result(