From 5066cde52bfeaf217f13dcfea9f90e0a300b8a21 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Sun, 29 Oct 2023 20:54:05 +0100 Subject: [PATCH 01/19] Customize mode new assignment syntax, part 1 --- cli/src/customize.rs | 588 --------------------------------- cli/src/customize/interface.rs | 380 +++++++++++++++++++++ cli/src/customize/mod.rs | 249 ++++++++++++++ cli/src/error.rs | 36 +- core/src/cache.rs | 13 +- core/src/program.rs | 2 +- 6 files changed, 670 insertions(+), 598 deletions(-) delete mode 100644 cli/src/customize.rs create mode 100644 cli/src/customize/interface.rs create mode 100644 cli/src/customize/mod.rs diff --git a/cli/src/customize.rs b/cli/src/customize.rs deleted file mode 100644 index d853da9c18..0000000000 --- a/cli/src/customize.rs +++ /dev/null @@ -1,588 +0,0 @@ -use std::{ - collections::{BTreeMap, HashMap}, - ffi::OsString, -}; - -use clap::{parser::ValueSource, ArgAction, Command}; -use nickel_lang_core::{ - combine::Combine, - eval::cache::lazy::CBNCache, - identifier::LocIdent, - program::{FieldOverride, Program}, - repl::query_print::{write_query_result, Attributes}, - term::{ - record::{Field, RecordData}, - LabeledType, MergePriority, RuntimeContract, Term, TypeAnnotation, - }, - typ::{RecordRowF, RecordRowsIteratorItem, Type, TypeF}, -}; - -use crate::error::CliResult; - -/// The maximal number of overridable fields displayed. Because there might be a lot of them, we -/// don't list them all by default. -const OVERRIDES_LIST_MAX_COUNT: usize = 15; - -/// The value name used through the CLI to indicate that an option accepts any Nickel expression as -/// a value. -const NICKEL_VALUE_NAME: &str = "NICKEL EXPRESSION"; - -const EXPERIMENTAL_MSG: &str = - "[WARNING] Customize mode is experimental. Its interface is subject to breaking changes."; - -#[derive(clap::Parser, Debug)] -pub struct CustomizeMode { - /// \[WARNING\] Customize mode is experimental. Its interface is subject to breaking changes. - /// - /// Customize mode turns the nickel invocation into a new CLI based on the configuration to be - /// exported, where the value of fields can be set directly through CLI arguments. Print the - /// corresponding help (`nickel export -- --help`) to see available options. - #[arg(last = true)] - pub customize_mode: Vec, -} -/// The interface of a configuration (a Nickel program) which represents all nested field paths -/// that are accessible from the root term together with their associated metadata. -/// -/// Interface is used to derive a command-line interface from a configuration when using the -/// `customize_mode` option. -#[derive(Debug, Clone, Default)] -struct TermInterface { - // We use a BTreeMap so that the end result is being sorted as we build the interface. - fields: BTreeMap, -} - -/// The interface of a specific input field. -#[derive(Debug, Clone, Default)] -struct FieldInterface { - /// The interface of the subfields of this field, if it's a record itself. - subfields: Option, - field: Field, -} - -/// The interface of a non-input field (overridable). -#[derive(Debug, Clone, Default)] -struct OverrideInterface { - /// The path of the overridable field. - path: Vec, - /// An optional description, built from the field's metadata, and inserted into the general - /// help message of the `--override` flag. - // help isn't used yet, hence the leading underscore. We need to think first about how to - // display it without cluttering the customize mode's help message. - _help: Option, -} - -impl TermInterface { - /// Build a command description from this interface. - /// - /// This method recursively lists all existing field paths, and reports input fields (as - /// defined per [FieldInterface::is_input]) as stand-alone arguments. For example, if the - /// configuration contains: - /// - /// ```nickel - /// foo.bar.baz | Number, - /// ``` - /// - /// This will give rise to a corresponding `--foo.bar.baz` argument listed in the `help` message - /// which can be set. - /// - /// Non-input fields are still listed in the description of the `--override` command, which - /// has the ability of setting any field. - /// - /// # Return - /// - /// In addition to the updated command, `build_cmd` returns a mapping from clap argument ids to - /// their corresponding full field path as an array of fields. - fn build_cmd(&self) -> TermCommand { - let clap_cmd = Command::new("customize-mode") - .about("Customize a Nickel configuration through the command line before exporting") - .after_help(EXPERIMENTAL_MSG) - .no_binary_name(true); - - let mut term_cmd = TermCommand::new(clap_cmd); - - for (id, field) in &self.fields { - term_cmd = field.add_args(term_cmd, vec![id.to_string()]) - } - - let mut overrides_list: Vec = term_cmd - .overrides - .keys() - .take(OVERRIDES_LIST_MAX_COUNT) - .map(|field_path| format!("- {field_path}")) - .collect(); - - if term_cmd.overrides.len() > OVERRIDES_LIST_MAX_COUNT { - overrides_list.push("- ...".into()); - } - - let has_override = term_cmd.inputs.contains_key("override"); - let has_help = term_cmd.inputs.contains_key("help"); - - let override_arg_label = "override"; - let override_help = format!( - "Override any field of the configuration with a valid Nickel expression provided as \ - a string. The new value will be merged with the configuration with a `force` \ - priority.\n\n\ - Overridable fields:\n{}\n\n", - overrides_list.join("\n") - ); - let override_arg = clap::Arg::new(override_arg_label) - .long(override_arg_label) - .value_name(NICKEL_VALUE_NAME.to_owned()) - .number_of_values(2) - .value_names(["field", "value"]) - .action(ArgAction::Append) - .required(false) - .help(override_help); - - term_cmd.clap_cmd = term_cmd.clap_cmd.arg(override_arg); - - if has_help || has_override { - let conflict_field = if has_override { - "override" - } else if has_help { - "help" - } else { - // We tested in the parent `if` that one of those two booleans must be set - unreachable!() - }; - - let extra = if has_help && has_override { - ". The same applies to the conflicting `help` field" - } else { - "" - }; - - term_cmd.clap_cmd = term_cmd.clap_cmd.after_long_help(format!( - "\ - [CONFLICT] This configuration has a field named `{conflict_field}` which conflicts \ - with the built-in `--{conflict_field}` argument. To set this field to e.g. \ - \"some_value\", use `--override {conflict_field} \"some_value\"` instead of \ - `--{conflict_field} \"some_value\"`\ - {extra}\n\n\ - {EXPERIMENTAL_MSG}" - )); - } - - term_cmd - } -} - -impl Combine for TermInterface { - fn combine(first: Self, second: Self) -> Self { - let TermInterface { mut fields } = first; - - for (id, field) in second.fields.into_iter() { - if let Some(prev) = fields.remove(&id) { - fields.insert(id, Combine::combine(prev, field)); - } else { - fields.insert(id, field); - } - } - - TermInterface { fields } - } -} - -impl Combine for FieldInterface { - fn combine(first: Self, second: Self) -> Self { - FieldInterface { - subfields: Combine::combine(first.subfields, second.subfields), - field: Field { - metadata: Combine::combine(first.field.metadata, second.field.metadata), - // Value is used only to show a default value, and to determine if a field has a - // definition. We don't bother actually merging the content, but just keep any - // side that is defined. - value: first.field.value.or(second.field.value), - ..Default::default() - }, - } - } -} - -impl From<&RecordData> for TermInterface { - fn from(value: &RecordData) -> Self { - TermInterface { - fields: value - .fields - .iter() - .map(|(id, field)| (*id, field.into())) - .collect(), - } - } -} - -impl From<&Term> for TermInterface { - fn from(term: &Term) -> Self { - term.extract_interface().unwrap_or_default() - } -} - -trait ExtractInterface { - fn extract_interface(&self) -> Option; -} - -impl ExtractInterface for &Term { - fn extract_interface(&self) -> Option { - if let Term::Record(rd) = self { - Some(TermInterface::from(rd)) - } else { - None - } - } -} - -impl ExtractInterface for Field { - fn extract_interface(&self) -> Option { - self.value.as_ref().map(|t| TermInterface::from(t.as_ref())) - } -} - -impl ExtractInterface for Type { - fn extract_interface(&self) -> Option { - match &self.typ { - TypeF::Record(rrows) => Some(TermInterface { - fields: rrows - .iter() - .filter_map(|item| { - if let RecordRowsIteratorItem::Row(rrow) = item { - Some((rrow.id, FieldInterface::from(&rrow))) - } else { - None - } - }) - .collect(), - }), - // Contract information is already extracted from runtime contracts, which are - // evaluated. Here, we focus on pure static type annotations and ignore flat types as - // well - _ => None, - } - } -} - -impl ExtractInterface for RuntimeContract { - fn extract_interface(&self) -> Option { - self.contract.as_ref().extract_interface() - } -} - -impl ExtractInterface for LabeledType { - fn extract_interface(&self) -> Option { - self.typ.extract_interface() - } -} - -impl From<&Field> for FieldInterface { - fn from(field: &Field) -> Self { - // We collect field information from all the sources we can: static types and contracts - // (both either as type annotations or contract annotations), and the value of the field if - // it's a record. - - let subfields_from_types = field - .pending_contracts - .iter() - .map(ExtractInterface::extract_interface); - - let subfields_from_contracts = field - .metadata - .annotation - .iter() - .map(ExtractInterface::extract_interface); - - let subfields_from_value = std::iter::once(field.extract_interface()); - - let subfields = subfields_from_types - .chain(subfields_from_contracts) - .chain(subfields_from_value) - .reduce(Combine::combine) - .flatten(); - - FieldInterface { - subfields, - field: field.clone(), - } - } -} - -impl From<&RecordRowF<&Type>> for FieldInterface { - fn from(rrow: &RecordRowF<&Type>) -> Self { - FieldInterface { - subfields: rrow.typ.extract_interface(), - ..Default::default() - } - } -} - -impl FieldInterface { - /// Take a clap command and enrich it with either one argument if this subfield is an input - /// field, or with all the subfields that are inputs. If this field or some of its subfields - /// aren't inputs, they are pushed to `overrides`. See [TermInterface::build_cmd]. - /// - /// `add_args` updates `paths` as well, which maps clap argument ids to the corresponding field - /// path represented as an array of string names. - fn add_args(&self, mut term_cmd: TermCommand, path: Vec) -> TermCommand { - // This is a terminal field, which gives rise to an argument or an overridable value. - if !self.has_subfields() { - if self.is_input() { - match path.as_slice() { - [ref field] if field == "override" || field == "help" => { - term_cmd.register_input(path.clone()) - } - _ => { - term_cmd.register_arg(path.clone()); - term_cmd.clap_cmd = self.add_arg(term_cmd.clap_cmd, path.join(".")) - } - }; - } else { - term_cmd.register_override(path.clone(), self.help()); - } - } - - for (id, field) in self.subfields.iter().flat_map(|intf| intf.fields.iter()) { - let mut path = path.clone(); - path.push(id.to_string()); - - term_cmd = field.add_args(term_cmd, path); - } - - term_cmd - } - - /// Add a single argument to the CLI `cmd`, based on the interface of this field, and return - /// the updated command. - fn add_arg(&self, cmd: clap::Command, path: String) -> clap::Command { - let mut arg = clap::Arg::new(&path) - .long(path) - .value_name(get_value_name(&self.field.metadata.annotation)) - // TODO: Create clap argument groups - .required(!self.is_default()); - - if let Some(help) = &self.help() { - arg = arg.help(help); - }; - - if let Some(default) = self.default_value() { - arg = arg.default_value(default); - } - - cmd.arg(arg) - } - - /// Define if a field is an input of a configuration that is intended to be filled, and will be - /// given a dedicated CLI argument. If the field is not an input, it can only be overridden via - /// `--override`. - /// - /// Currently, the difference is mostly conceptual: in practice, we simply gather the arguments - /// and their values, either via direct arguments or `--override` (although the latter sets a - /// different priority), and then merge the elaborated record with original term. - /// - /// However, from a user experience point of view, we want to make a clear distinction between - /// filling a bespoke input of a configuration and overriding an existing value. The latter is - /// more "low-level" and should only be done knowingly. - /// - /// Currently, the logic is simply that a field is an input if it doesn't have a definition or - /// it has a default priority. This definition might evolve, as there are ongoing discussions - /// on what is should be the meaning of "input", "output", and if those concept should be made - /// first-class ([related issue](https://github.com/tweag/nickel/issues/1505)). For now, this - /// logic seems to be a reasonable first approximation. - fn is_input(&self) -> bool { - !self.is_defined() || self.is_default() - } - - /// Return `true` is the field has a value. - fn is_defined(&self) -> bool { - self.field.value.is_some() - } - - /// Return true is the field's merge priority is `default`. - fn is_default(&self) -> bool { - matches!(self.field.metadata.priority, MergePriority::Bottom) - } - - fn has_subfields(&self) -> bool { - matches!(&self.subfields, Some(ref intf) if !intf.fields.is_empty()) - } - - /// Return the default value, if any. - fn default_value(&self) -> Option { - match (&self.field.metadata.priority, &self.field.value) { - (MergePriority::Bottom, Some(value)) => Some(value.to_string()), - _ => None, - } - } - - /// Render a help message similar to the output of a metadata query to serve as an help text - /// for this argument. - fn help(&self) -> Option { - let mut output: Vec = Vec::new(); - - // We only need to render the documentation: the rest is printed separately as part of the - // clap command that is built. - let attributes = Attributes { - doc: true, - contract: false, - typ: false, - default: false, - value: false, - }; - - write_query_result(&mut output, &self.field, attributes) - .unwrap_or(false) - .then(|| { - String::from_utf8(output) - .expect("the query printer should always output valid utf8") - }) - } -} - -fn get_value_name(annotation: &TypeAnnotation) -> String { - if annotation.is_empty() { - NICKEL_VALUE_NAME.into() - } else { - let anns: Vec = annotation - .iter() - .map(|ctr| ctr.label.typ.to_string()) - .collect(); - anns.join(",") - } -} - -/// A command dynamically built from a Nickel term. -struct TermCommand { - /// The corresponding clap command - clap_cmd: clap::Command, - /// A mapping between clap argument ids and the corresponding field path (for inputs). - args: HashMap>, - /// Mapping between existing input fields (as a dot-separated path string) and their path - /// represented as a vector. This is used both as a `HashSet` to check the existence of an - /// input field, and to sidestep the issue of parsing a path again when reading an input - /// argument. - inputs: HashMap>, - /// A mapping between a field path and the corresponding override data. - overrides: BTreeMap, -} - -impl TermCommand { - fn new(command: clap::Command) -> Self { - TermCommand { - clap_cmd: command, - overrides: BTreeMap::new(), - inputs: HashMap::new(), - args: HashMap::new(), - } - } - - /// Register a new argument corresponding to an input. Add the paths and the argument's id to - /// the relevant maps. - fn register_arg(&mut self, path: Vec) { - let path_string = path.join("."); - let arg_id = clap::Id::from(path_string.clone()); - - debug_assert!(!self.args.contains_key(&arg_id)); - debug_assert!(!self.inputs.contains_key(&path_string)); - - self.args.insert(arg_id, path.clone()); - self.inputs.insert(path_string, path); - } - - /// Register an input, but don't register the corresponding CLI argument. Used for `override` - /// and `help` which conflict with the built-in `--override` and `--help` arguments. - fn register_input(&mut self, path: Vec) { - let path_string = path.join("."); - - debug_assert!(!self.inputs.contains_key(&path_string)); - - self.inputs.insert(path_string, path); - } - - fn register_override(&mut self, path: Vec, help: Option) { - let path_string = path.join("."); - - self.overrides - .insert(path_string, OverrideInterface { path, _help: help }); - } -} - -pub trait Customize { - fn customize(&self, program: Program) -> CliResult>; -} - -impl Customize for CustomizeMode { - // XXX: we should give a nice error message when someone tries to evaluate some - // expression that has unset values, telling them they can set them using - // this method - fn customize(&self, mut program: Program) -> CliResult> { - if self.customize_mode.is_empty() { - return Ok(program); - } - - let evaled = match program.eval_record_spine() { - Ok(evaled) => evaled, - // We need a `return` control-flow to be able to take `program` out - Err(error) => return CliResult::Err(crate::error::Error::Program { error, program }), - }; - - let cmd = TermInterface::from(evaled.as_ref()).build_cmd(); - let arg_matches = cmd.clap_cmd.get_matches_from(self.customize_mode.iter()); - - let force_overrides: Result, super::error::CliUsageError> = arg_matches - .get_occurrences("override") - .into_iter() - .flat_map(|occurs| { - occurs.map(|mut pair| { - let (path, value): (&String, &String) = - (pair.next().unwrap(), pair.next().unwrap()); - - cmd.overrides - .get(path) - .map(|intf| intf.path.clone()) - // We allow --override to set inputs as well, in particular as an - // escape mechanism to set an input field named `override` or `help`, - // which would conflict with the corresponding builtin flags. - .or(cmd.inputs.get(path).cloned()) - .map(|path| FieldOverride { - path, - value: value.clone(), - priority: MergePriority::Top, - }) - .ok_or_else(|| super::error::CliUsageError::InvalidOverride { - path: path.clone(), - }) - }) - }) - .collect(); - - let force_overrides = match force_overrides { - Ok(force_overrides) => force_overrides, - Err(error) => return CliResult::Err(crate::error::Error::CliUsage { error, program }), - }; - - program.add_overrides( - arg_matches - .ids() - .filter(|id| { - !matches!( - arg_matches.value_source(id.as_str()), - Some(ValueSource::DefaultValue) - ) && id.as_str() != "override" - }) - .map(|id| FieldOverride { - path: cmd.args.get(id).unwrap().clone(), - value: arg_matches.get_one::(id.as_str()).unwrap().clone(), - priority: MergePriority::default(), - }) - .chain(force_overrides), - ); - Ok(program) - } -} - -#[derive(clap::Args, Debug)] -pub struct NoCustomizeMode; - -impl Customize for NoCustomizeMode { - fn customize(&self, program: Program) -> CliResult> { - Ok(program) - } -} diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs new file mode 100644 index 0000000000..6f2c27e0f8 --- /dev/null +++ b/cli/src/customize/interface.rs @@ -0,0 +1,380 @@ +//! This module defines the interface of a Nickel configuration which is used to derive the +//! customize mode of the command-line. +use super::*; + +const NICKEL_VALUE_NAME: &str = "NICKEL EXPRESSION"; + +/// The interface of a configuration (a Nickel program) which represents all nested field paths +/// that are accessible from the root term together with their associated metadata. +/// +/// Interface is used to derive a command-line interface from a configuration when using the +/// `customize_mode` option. +#[derive(Debug, Clone, Default)] +pub(super) struct TermInterface { + // We use a BTreeMap so that the end result is being sorted as we build the interface. + pub(super) fields: BTreeMap, +} + +/// The interface of a specific field. This fields can be itself a record and contain subfields. +#[derive(Debug, Clone, Default)] +pub(super) struct FieldInterface { + /// The interface of the subfields of this field, if it's a record itself. + pub(super) subfields: Option, + pub(super) field: Field, +} + +/// Description of a field which is a leaf, i.e. which doesn't contain other fields. Those are the +/// fields that can be set from the customize command line. +#[derive(Debug, Clone, Default)] +pub(super) struct CustomizableField { + /// The path of the field. + pub(super) path: Vec, + pub(super) interface: FieldInterface, +} + +impl TermInterface { + // /// Build a command description from this interface. + // /// + // /// This method recursively lists all existing field paths, and reports input fields (as + // /// defined per [FieldInterface::is_input]) as stand-alone arguments. For example, if the + // /// configuration contains: + // /// + // /// ```nickel + // /// foo.bar.baz | Number, + // /// ``` + // /// + // /// This will give rise to a corresponding `--foo.bar.baz` argument listed in the `help` message + // /// which can be set. + // /// + // /// Non-input fields are still listed in the description of the `--override` command, which + // /// has the ability of setting any field. + // /// + // /// # Return + // /// + // /// In addition to the updated command, `build_cmd` returns a mapping from clap argument ids to + // /// their corresponding full field path as an array of fields. + // pub(super) fn build_cmd(&self) -> TermCommand { + // todo!() + + // let clap_cmd = Command::new("customize-mode") + // .about("Customize a Nickel configuration through the command line before exporting") + // .after_help(EXPERIMENTAL_MSG) + // .no_binary_name(true); + // + // let mut term_cmd = TermCommand::new(clap_cmd); + // + // for (id, field) in &self.fields { + // term_cmd = field.add_args(term_cmd, vec![id.to_string()]) + // } + // + // let mut overrides_list: Vec = term_cmd + // .overrides + // .keys() + // .take(OVERRIDES_LIST_MAX_COUNT) + // .map(|field_path| format!("- {field_path}")) + // .collect(); + // + // if term_cmd.overrides.len() > OVERRIDES_LIST_MAX_COUNT { + // overrides_list.push("- ...".into()); + // } + // + // let has_override = term_cmd.inputs.contains_key("override"); + // let has_help = term_cmd.inputs.contains_key("help"); + // + // let override_arg_label = "override"; + // let override_help = format!( + // "Override any field of the configuration with a valid Nickel expression provided as \ + // a string. The new value will be merged with the configuration with a `force` \ + // priority.\n\n\ + // Overridable fields:\n{}\n\n", + // overrides_list.join("\n") + // ); + // let override_arg = clap::Arg::new(override_arg_label) + // .long(override_arg_label) + // .value_name(NICKEL_VALUE_NAME.to_owned()) + // .number_of_values(2) + // .value_names(["field", "value"]) + // .action(ArgAction::Append) + // .required(false) + // .help(override_help); + // + // term_cmd.clap_cmd = term_cmd.clap_cmd.arg(override_arg); + // + // if has_help || has_override { + // let conflict_field = if has_override { + // "override" + // } else if has_help { + // "help" + // } else { + // // We tested in the parent `if` that one of those two booleans must be set + // unreachable!() + // }; + // + // let extra = if has_help && has_override { + // ". The same applies to the conflicting `help` field" + // } else { + // "" + // }; + // + // term_cmd.clap_cmd = term_cmd.clap_cmd.after_long_help(format!( + // "\ + // [CONFLICT] This configuration has a field named `{conflict_field}` which conflicts \ + // with the built-in `--{conflict_field}` argument. To set this field to e.g. \ + // \"some_value\", use `--override {conflict_field} \"some_value\"` instead of \ + // `--{conflict_field} \"some_value\"`\ + // {extra}\n\n\ + // {EXPERIMENTAL_MSG}" + // )); + // } + // + // term_cmd + // } +} + +impl Combine for TermInterface { + fn combine(first: Self, second: Self) -> Self { + let TermInterface { mut fields } = first; + + for (id, field) in second.fields.into_iter() { + if let Some(prev) = fields.remove(&id) { + fields.insert(id, Combine::combine(prev, field)); + } else { + fields.insert(id, field); + } + } + + TermInterface { fields } + } +} + +impl Combine for FieldInterface { + fn combine(first: Self, second: Self) -> Self { + FieldInterface { + subfields: Combine::combine(first.subfields, second.subfields), + field: Field { + metadata: Combine::combine(first.field.metadata, second.field.metadata), + // Value is used only to show a default value, and to determine if a field has a + // definition. We don't bother actually merging the content, but just keep any + // side that is defined. + value: first.field.value.or(second.field.value), + ..Default::default() + }, + } + } +} + +impl From<&RecordData> for TermInterface { + fn from(value: &RecordData) -> Self { + TermInterface { + fields: value + .fields + .iter() + .map(|(id, field)| (*id, field.into())) + .collect(), + } + } +} + +impl From<&Term> for TermInterface { + fn from(term: &Term) -> Self { + term.extract_interface().unwrap_or_default() + } +} + +trait ExtractInterface { + fn extract_interface(&self) -> Option; +} + +impl ExtractInterface for &Term { + fn extract_interface(&self) -> Option { + if let Term::Record(rd) = self { + Some(TermInterface::from(rd)) + } else { + None + } + } +} + +impl ExtractInterface for Field { + fn extract_interface(&self) -> Option { + self.value.as_ref().map(|t| TermInterface::from(t.as_ref())) + } +} + +impl ExtractInterface for Type { + fn extract_interface(&self) -> Option { + match &self.typ { + TypeF::Record(rrows) => Some(TermInterface { + fields: rrows + .iter() + .filter_map(|item| { + if let RecordRowsIteratorItem::Row(rrow) = item { + Some((rrow.id, FieldInterface::from(&rrow))) + } else { + None + } + }) + .collect(), + }), + // Contract information is already extracted from runtime contracts, which are + // evaluated. Here, we focus on pure static type annotations and ignore flat types as + // well + _ => None, + } + } +} + +impl ExtractInterface for RuntimeContract { + fn extract_interface(&self) -> Option { + self.contract.as_ref().extract_interface() + } +} + +impl ExtractInterface for LabeledType { + fn extract_interface(&self) -> Option { + self.typ.extract_interface() + } +} + +impl From<&Field> for FieldInterface { + fn from(field: &Field) -> Self { + // We collect field information from all the sources we can: static types and contracts + // (both either as type annotations or contract annotations), and the value of the field if + // it's a record. + + let subfields_from_types = field + .pending_contracts + .iter() + .map(ExtractInterface::extract_interface); + + let subfields_from_contracts = field + .metadata + .annotation + .iter() + .map(ExtractInterface::extract_interface); + + let subfields_from_value = std::iter::once(field.extract_interface()); + + let subfields = subfields_from_types + .chain(subfields_from_contracts) + .chain(subfields_from_value) + .reduce(Combine::combine) + .flatten(); + + FieldInterface { + subfields, + field: field.clone(), + } + } +} + +impl From<&RecordRowF<&Type>> for FieldInterface { + fn from(rrow: &RecordRowF<&Type>) -> Self { + FieldInterface { + subfields: rrow.typ.extract_interface(), + ..Default::default() + } + } +} + +impl FieldInterface { + /// Add a single argument to the CLI `cmd`, based on the interface of this field, and return + /// the updated command. + fn add_arg(&self, cmd: clap::Command, path: String) -> clap::Command { + let mut arg = clap::Arg::new(&path) + .long(path) + .value_name(self.value_name()) + // TODO: Create clap argument groups + .required(!self.is_default()); + + if let Some(help) = &self.help() { + arg = arg.help(help); + }; + + if let Some(default) = self.default_value() { + arg = arg.default_value(default); + } + + cmd.arg(arg) + } + + /// Define if a field is an input of a configuration that is intended to be filled, and will be + /// given a dedicated CLI argument. If the field is not an input, it can only be overridden via + /// `--override`. + /// + /// Currently, the difference is mostly conceptual: in practice, we simply gather the arguments + /// and their values, either via direct arguments or `--override` (although the latter sets a + /// different priority), and then merge the elaborated record with original term. + /// + /// However, from a user experience point of view, we want to make a clear distinction between + /// filling a bespoke input of a configuration and overriding an existing value. The latter is + /// more "low-level" and should only be done knowingly. + /// + /// Currently, the logic is simply that a field is an input if it doesn't have a definition or + /// it has a default priority. This definition might evolve, as there are ongoing discussions + /// on what is should be the meaning of "input", "output", and if those concept should be made + /// first-class ([related issue](https://github.com/tweag/nickel/issues/1505)). For now, this + /// logic seems to be a reasonable first approximation. + pub(super) fn is_input(&self) -> bool { + !self.is_defined() || self.is_default() + } + + /// Return `true` is the field has a value. + pub(super) fn is_defined(&self) -> bool { + self.field.value.is_some() + } + + /// Return true is the field's merge priority is `default`. + pub(super) fn is_default(&self) -> bool { + matches!(self.field.metadata.priority, MergePriority::Bottom) + } + + pub(super) fn has_subfields(&self) -> bool { + matches!(&self.subfields, Some(ref intf) if !intf.fields.is_empty()) + } + + /// Return the default value, if any. + pub(super) fn default_value(&self) -> Option { + match (&self.field.metadata.priority, &self.field.value) { + (MergePriority::Bottom, Some(value)) => Some(value.to_string()), + _ => None, + } + } + + /// Render a help message similar to the output of a metadata query to serve as an help text + /// for this argument. + pub(super) fn help(&self) -> Option { + let mut output: Vec = Vec::new(); + + // We only need to render the documentation: the rest is printed separately as part of the + // clap command that is built. + let attributes = Attributes { + doc: true, + contract: false, + typ: false, + default: false, + value: false, + }; + + write_query_result(&mut output, &self.field, attributes) + .unwrap_or(false) + .then(|| { + String::from_utf8(output) + .expect("the query printer should always output valid utf8") + }) + } + + fn value_name(&self) -> String { + let annotation = &self.field.metadata.annotation; + + if annotation.is_empty() { + NICKEL_VALUE_NAME.into() + } else { + let anns: Vec = annotation + .iter() + .map(|ctr| ctr.label.typ.to_string()) + .collect(); + anns.join(",") + } + } +} diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs new file mode 100644 index 0000000000..b042fbdd94 --- /dev/null +++ b/cli/src/customize/mod.rs @@ -0,0 +1,249 @@ +//! Customize mode parser and evaluation logic. +//! +//! The customize mode is a way to interact with a specific Nickel program on the command line by +//! setting or overriding values directly from the command line. It's currently enabled by the +//! freeform marker `--`. +use std::{ + collections::{BTreeMap, HashMap}, + ffi::OsString, +}; + +use clap::{parser::ValueSource, ArgAction, Command}; +use nickel_lang_core::{ + combine::Combine, + eval::cache::lazy::CBNCache, + identifier::LocIdent, + program::{FieldOverride, Program}, + repl::query_print::{write_query_result, Attributes}, + term::{ + record::{Field, RecordData}, + LabeledType, MergePriority, RuntimeContract, Term, TypeAnnotation, + }, + typ::{RecordRowF, RecordRowsIteratorItem, Type, TypeF}, +}; + +use crate::error::CliResult; + +pub mod interface; + +use interface::{CustomizableField, FieldInterface, TermInterface}; + +/// The maximal number of overridable fields displayed. Because there might be a lot of them, we +/// don't list them all by default. +const OVERRIDES_LIST_MAX_COUNT: usize = 15; + +/// The value name used through the CLI to indicate that an option accepts any Nickel expression as +/// a value. +const ASSIGNMENT_SYNTAX: &str = "FIELD_PATH=NICKEL EXPRESSION"; + +const EXPERIMENTAL_MSG: &str = + "[WARNING] Customize mode is experimental. Its interface is subject to breaking changes."; + +#[derive(clap::Parser, Debug)] +pub struct CustomizeMode { + /// \[WARNING\] Customize mode is experimental. Its interface is subject to breaking changes. + /// + /// Customize mode turns the nickel invocation into a new CLI based on the configuration to be + /// exported, where the value of fields can be set directly through CLI arguments. Print the + /// corresponding help (`nickel export -- --help`) to see available options. + #[arg(last = true)] + pub customize_mode: Vec, +} + +impl CustomizeMode { + fn make_cmd() -> clap::Command { + let assignment = clap::Arg::new("assignment").action(ArgAction::Append); + + let override_arg = clap::Arg::new("override") + .long("override") + .value_name(ASSIGNMENT_SYNTAX.to_owned()) + .action(ArgAction::Append) + .help( + "\ + Override any field of the configuration with a valid Nickel expression provided as \ + a string. The new value will be merged with the configuration with a `force` \ + priority.", + ); + + Command::new("customize-mode") + .about("Customize a Nickel configuration through the command line before exporting") + .after_help(EXPERIMENTAL_MSG) + .no_binary_name(true) + .arg(override_arg) + } +} + +/// An assignment of the form `path.to.field=value`. +struct FieldAssignment { + path: Vec, + value: String, +} + +/// Fields of the configuration which aren't themselves records and can be assigned or overridden +/// through the customize mode. +#[derive(Clone, Debug, Default)] +struct CustomizableFields { + inputs: BTreeMap, CustomizableField>, + overrides: BTreeMap, CustomizableField>, +} + +impl CustomizableFields { + /// Create data from a term interface. + fn new(term_iface: TermInterface) -> Self { + let mut this = Self::default(); + + for (id, field_iface) in term_iface.fields { + this.extend_with(field_iface, vec![id]); + } + + this + } + + /// Register a customizable field that can be assigned (input). + fn register_input(&mut self, path: Vec, interface: FieldInterface) { + debug_assert!(!self.inputs.contains_key(&path)); + debug_assert!(!self.overrides.contains_key(&path)); + + self.inputs + .insert(path.clone(), CustomizableField { path, interface }); + } + + /// Register a customizable field that can be overridden. + fn register_override(&mut self, path: Vec, interface: FieldInterface) { + debug_assert!(!self.inputs.contains_key(&path)); + debug_assert!(!self.overrides.contains_key(&path)); + + self.overrides + .insert(path.clone(), CustomizableField { path, interface }); + } + + /// Enrich the current list of customizable fields with the field(s) exposed by a given field + /// interface. + fn extend_with(&mut self, field_iface: FieldInterface, path: Vec) { + // This is a terminal field, which gives rise to an argument or an overridable value. + if !field_iface.has_subfields() { + if field_iface.is_input() { + self.register_input(path.clone(), field_iface); + } else { + self.register_override(path.clone(), field_iface); + } + } else { + for (id, child_iface) in field_iface + .subfields + .into_iter() + .flat_map(|intf| intf.fields.into_iter()) + { + let mut path = path.clone(); + path.push(id); + + self.extend_with(child_iface, path); + } + } + } +} + +pub trait Customize { + fn customize(&self, program: Program) -> CliResult>; +} + +impl Customize for CustomizeMode { + // XXX: we should give a nice error message when someone tries to evaluate some + // expression that has unset values, telling them they can set them using + // this method + fn customize(&self, mut program: Program) -> CliResult> { + if self.customize_mode.is_empty() { + return Ok(program); + } + + let evaled = match program.eval_record_spine() { + Ok(evaled) => evaled, + // We need a `return` control-flow to be able to take `program` out + Err(error) => return CliResult::Err(crate::error::Error::Program { error, program }), + }; + + let customizable_fields = CustomizableFields::new(TermInterface::from(evaled.as_ref())); + let clap_cmd = Self::make_cmd(); + let arg_matches = clap_cmd.get_matches_from(self.customize_mode.iter()); + + let assignment_overrides: Result, super::error::CliUsageError> = arg_matches + .get_occurrences("assignment") + .into_iter() + .flat_map(|occurs| { + occurs.map(|mut occurrence_value| { + let assignment: &String = occurrence_value.next().unwrap(); + let (path_str, value_str): (&String, &String) = todo!(); + let path: Vec = todo!(); + + customizable_fields + .inputs + .get(&path) + .map(|intf| FieldOverride { + path: intf.path.clone(), + value: value_str.clone(), + priority: MergePriority::default(), + }) + .ok_or_else(|| { + if customizable_fields.inputs.contains_key(&path) { + super::error::CliUsageError::CantAssignNonInput { + path: path_str.clone(), + assignment: todo!(), + } + } else { + super::error::CliUsageError::UnknownFieldOverride { + path: path_str.clone(), + } + } + }) + }) + }) + .collect(); + + let assignment_overrides = match assignment_overrides { + Ok(assignment_overrides) => assignment_overrides, + Err(error) => return CliResult::Err(crate::error::Error::CliUsage { error, program }), + }; + + let force_overrides: Result, super::error::CliUsageError> = arg_matches + .get_occurrences("override") + .into_iter() + .flat_map(|occurs| { + occurs.map(|mut pair| { + let (path_str, value_str): (&String, &String) = + (pair.next().unwrap(), pair.next().unwrap()); + let path: Vec = todo!(); + + customizable_fields + .overrides + .get(&path) + // We allow --override to set inputs as well. + .or(customizable_fields.inputs.get(&path)) + .map(|intf| FieldOverride { + path: intf.path.clone(), + value: value_str.clone(), + priority: MergePriority::Top, + }) + .ok_or_else(|| super::error::CliUsageError::UnknownFieldOverride { + path: path_str.clone(), + }) + }) + }) + .collect(); + + let force_overrides = match force_overrides { + Ok(force_overrides) => force_overrides, + Err(error) => return CliResult::Err(crate::error::Error::CliUsage { error, program }), + }; + + program.add_overrides(assignment_overrides.into_iter().chain(force_overrides)); + Ok(program) + } +} + +#[derive(clap::Args, Debug)] +pub struct NoCustomizeMode; + +impl Customize for NoCustomizeMode { + fn customize(&self, program: Program) -> CliResult> { + Ok(program) + } +} diff --git a/cli/src/error.rs b/cli/src/error.rs index 937ab85fae..8fd7ba7d51 100644 --- a/cli/src/error.rs +++ b/cli/src/error.rs @@ -6,7 +6,12 @@ use nickel_lang_core::{ /// Errors related to mishandling the CLI. pub enum CliUsageError { - InvalidOverride { path: String }, + /// Tried to override a field which doesn't exist. + UnknownFieldOverride { path: String }, + /// Tried to assign a field which doesn't exist. + UnknownFieldAssignment { path: String }, + /// Tried to override an defined field without the `--override` argument. + CantAssignNonInput { path: String, assignment: String }, } pub enum Error { @@ -39,14 +44,31 @@ impl IntoDiagnostics for CliUsageError { _files: &mut Files, _stdlib_ids: Option<&Vec>, ) -> Vec> { + fn mk_unknown_diags(path: String, method: &str) -> Vec> { + vec![Diagnostic::error() + .with_message(format!("invalid {method}: unknown field `{path}`")) + .with_notes(vec![format!( + "`{path}` doesn't refer to record field accessible from the root of the \ + configuration." + )])] + } + match self { - CliUsageError::InvalidOverride { path } => { + CliUsageError::UnknownFieldOverride { path } => mk_unknown_diags(path, "override"), + CliUsageError::UnknownFieldAssignment { path } => mk_unknown_diags(path, "assignment"), + CliUsageError::CantAssignNonInput { path, assignment } => { vec![Diagnostic::error() - .with_message(format!("invalid override: unknown field `{path}`")) - .with_notes(vec![format!( - "`{path}` doesn't refer to record field accessible from the root of the \ - configuration and thus can't be the target of `--override`." - )])] + .with_message(format!("invalid assignment: `{path}` isn't an input")) + .with_notes(vec![ + format!( + "`{path}` already has a value and thus can't be assigned \ + without `--override`." + ), + format!( + "If you really want to override this field, please use \ + `--override {assignment}` instead." + ), + ])] } } } diff --git a/core/src/cache.rs b/core/src/cache.rs index 13b5603dae..f931d6c323 100644 --- a/core/src/cache.rs +++ b/core/src/cache.rs @@ -3,6 +3,7 @@ use crate::error::{Error, ImportError, ParseError, ParseErrors, TypecheckError}; use crate::eval::cache::Cache as EvalCache; use crate::eval::Closure; +use crate::identifier::LocIdent; #[cfg(feature = "nix-experimental")] use crate::nix_ffi; use crate::parser::{lexer::Lexer, ErrorTolerantParser}; @@ -15,6 +16,7 @@ use crate::transform::import_resolution; use crate::typ::UnboundTypeVariableError; use crate::typecheck::{self, type_check, Wildcards}; use crate::{eval, parser, transform}; + use codespan::{FileId, Files}; use io::Read; use serde::Deserialize; @@ -267,7 +269,7 @@ pub enum SourcePath { ReplInput(usize), ReplTypecheck, ReplQuery, - Override(Vec), + Override(Vec), Generated(String), } @@ -295,7 +297,14 @@ impl From for OsString { SourcePath::ReplInput(idx) => format!("").into(), SourcePath::ReplTypecheck => "".into(), SourcePath::ReplQuery => "".into(), - SourcePath::Override(path) => format!("", path.join(".")).into(), + SourcePath::Override(path) => format!( + "", + path.iter() + .map(ToString::to_string) + .collect::>() + .join(".") + ) + .into(), SourcePath::Generated(description) => format!("", description).into(), } } diff --git a/core/src/program.rs b/core/src/program.rs index fa3672772d..d200ef7ca0 100644 --- a/core/src/program.rs +++ b/core/src/program.rs @@ -114,7 +114,7 @@ impl QueryPath { #[derive(Clone)] pub struct FieldOverride { /// The field path identifying the (potentially nested) field to override. - pub path: Vec, + pub path: Vec, /// The overriding value. pub value: String, /// The priority associated with this override. From c4165bd94ce0636c30922335f54cc7fa45c9f5b3 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Thu, 2 Nov 2023 16:54:56 +0100 Subject: [PATCH 02/19] Add StaticPathField parsing rule --- core/src/error/mod.rs | 20 +++++++++++++------- core/src/parser/error.rs | 5 +++++ core/src/parser/grammar.lalrpop | 23 ++++++++++++++++++++++- core/src/parser/mod.rs | 2 +- core/src/program.rs | 24 ++++-------------------- 5 files changed, 45 insertions(+), 29 deletions(-) diff --git a/core/src/error/mod.rs b/core/src/error/mod.rs index 41983386f2..88da74fbee 100644 --- a/core/src/error/mod.rs +++ b/core/src/error/mod.rs @@ -494,11 +494,11 @@ pub enum ParseError { /// The position of the type annotation. annot_span: RawSpan, }, - /// The user provided a field path to run a metadata query, but the parsed field path contains - /// string interpolation. - InterpolationInQuery { + /// The user provided a field path on the CLI, which is expected to be only composed of + /// literals, but the parsed field path contains string interpolation. + InterpolationInStaticPath { input: String, - pos_path_elem: TermPos, + path_elem_span: RawSpan, }, /// A duplicate binding was encountered in a record destructuring pattern. DuplicateIdentInRecordPattern { @@ -712,6 +712,12 @@ impl ParseError { InternalParseError::DisabledFeature { feature, span } => { ParseError::DisabledFeature { feature, span } } + InternalParseError::InterpolationInStaticPath { path_elem_span } => { + ParseError::InterpolationInStaticPath { + input: String::new(), + path_elem_span, + } + } }, } } @@ -1788,12 +1794,12 @@ impl IntoDiagnostics for ParseError { record type." .into(), ]), - ParseError::InterpolationInQuery { + ParseError::InterpolationInStaticPath { input, - pos_path_elem: path_elem_pos, + path_elem_span, } => Diagnostic::error() .with_message("string interpolation is forbidden within a query") - .with_labels(vec![primary_alt(path_elem_pos.into_opt(), input, files)]) + .with_labels(vec![primary(&path_elem_span)]) .with_notes(vec![ "Field paths don't support string interpolation when querying \ metadata." diff --git a/core/src/parser/error.rs b/core/src/parser/error.rs index 509762fbf7..30108efd78 100644 --- a/core/src/parser/error.rs +++ b/core/src/parser/error.rs @@ -137,6 +137,11 @@ pub enum ParseError { /// The position of the type annotation. annot_span: RawSpan, }, + /// The user provided a field path on the CLI, which is expected to be only composed of + /// literals, but the parsed field path contains string interpolation. + InterpolationInStaticPath { + path_elem_span: RawSpan, + }, DisabledFeature { feature: String, span: RawSpan, diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop index f7ae156f3f..0d0fb55362 100644 --- a/core/src/parser/grammar.lalrpop +++ b/core/src/parser/grammar.lalrpop @@ -444,13 +444,34 @@ RecordLastField: RecordLastField = { }; // A field path syntax in a field definition, as in `{foo."bar bar".baz = "value"}`. -pub FieldPath: FieldPath = { +FieldPath: FieldPath = { ".")*> => { elems.push(last); elems } }; +// A field path which only contains static string literals, that is, without any +// interpolated expression in it. +pub StaticFieldPath: Vec = =>? { + field_path + .into_iter() + .map(|elem| match elem { + FieldPathElem::Ident(ident) => Ok(ident), + FieldPathElem::Expr(expr) => { + let as_string = expr.as_ref().try_str_chunk_as_static_str().ok_or( + ParseError::InterpolationInStaticPath { + path_elem_span: expr.pos + .into_opt() + .unwrap_or_else(|| mk_span(src_id, start, end)), + }, + )?; + Ok(LocIdent::new_with_pos(as_string, expr.pos)) + } + }) + .collect() +}; + FieldPathElem: FieldPathElem = { => FieldPathElem::Ident(<>), > => FieldPathElem::Expr(<>), diff --git a/core/src/parser/mod.rs b/core/src/parser/mod.rs index e1d5672170..40fa2a4356 100644 --- a/core/src/parser/mod.rs +++ b/core/src/parser/mod.rs @@ -89,7 +89,7 @@ macro_rules! generate_lalrpop_parser_impl { generate_lalrpop_parser_impl!(grammar::ExtendedTermParser, ExtendedTerm); generate_lalrpop_parser_impl!(grammar::TermParser, RichTerm); generate_lalrpop_parser_impl!(grammar::FixedTypeParser, Type); -generate_lalrpop_parser_impl!(grammar::FieldPathParser, utils::FieldPath); +generate_lalrpop_parser_impl!(grammar::StaticFieldPathParser, Vec); /// Generic interface of the various specialized Nickel parsers. /// diff --git a/core/src/program.rs b/core/src/program.rs index d200ef7ca0..083e1a867e 100644 --- a/core/src/program.rs +++ b/core/src/program.rs @@ -63,40 +63,24 @@ impl QueryPath { /// 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, + grammar::StaticFieldPathParser, lexer::Lexer, utils::FieldPathElem, ErrorTolerantParser, }; let input_id = cache.replace_string(SourcePath::Query, input); let s = cache.source(input_id); - let parser = FieldPathParser::new(); + let parser = StaticFieldPathParser::new(); let field_path = parser .parse_strict(input_id, Lexer::new(s)) // We just need to report an error here .map_err(|mut errs| { errs.errors.pop().expect( "because parsing of the query path failed, the error \ - list must be non-empty, put .pop() failed", + list must be non-empty, put .pop() failed", ) })?; - let path_as_idents: Result, ParseError> = field_path - .into_iter() - .map(|elem| match elem { - FieldPathElem::Ident(ident) => Ok(ident), - FieldPathElem::Expr(expr) => { - let as_string = expr.as_ref().try_str_chunk_as_static_str().ok_or( - ParseError::InterpolationInQuery { - input: s.into(), - pos_path_elem: expr.pos, - }, - )?; - Ok(LocIdent::from(as_string)) - } - }) - .collect(); - - path_as_idents.map(QueryPath) + Ok(QueryPath(field_path)) } /// As [`Self::parse`], but accepts an `Option` to accomodate for the absence of path. If the From 06de91848f9fc8c98d182516c10bd48b07663c43 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Thu, 2 Nov 2023 18:38:04 +0100 Subject: [PATCH 03/19] Add cli assignment parsing rule --- core/src/parser/grammar.lalrpop | 25 +++++++++++++++++++++++++ core/src/parser/mod.rs | 1 + 2 files changed, 26 insertions(+) diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop index 0d0fb55362..2134e25e56 100644 --- a/core/src/parser/grammar.lalrpop +++ b/core/src/parser/grammar.lalrpop @@ -472,6 +472,31 @@ pub StaticFieldPath: Vec = rule produces a +// RichTerm anyway, so we just return it, instead of artificially deconstructing +// it. +// +// This construct is only for the CLI, and isn't currently part of the grammar +// for normal Nickel source code. +pub CliFieldAssignment: (Vec, RichTerm, RawSpan) = + "=" > + => (path, value, mk_span(src_id, start, end)); + FieldPathElem: FieldPathElem = { => FieldPathElem::Ident(<>), > => FieldPathElem::Expr(<>), diff --git a/core/src/parser/mod.rs b/core/src/parser/mod.rs index 40fa2a4356..7faf87f2ad 100644 --- a/core/src/parser/mod.rs +++ b/core/src/parser/mod.rs @@ -90,6 +90,7 @@ generate_lalrpop_parser_impl!(grammar::ExtendedTermParser, ExtendedTerm); generate_lalrpop_parser_impl!(grammar::TermParser, RichTerm); generate_lalrpop_parser_impl!(grammar::FixedTypeParser, Type); generate_lalrpop_parser_impl!(grammar::StaticFieldPathParser, Vec); +generate_lalrpop_parser_impl!(grammar::CliFieldAssignmentParser, (Vec, RichTerm)); /// Generic interface of the various specialized Nickel parsers. /// From 96389b408af606db64f1e62673c4b521117c3632 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Nov 2023 18:21:44 +0100 Subject: [PATCH 04/19] Use new assignment parsing rule, refactor common code --- cli/src/customize/interface.rs | 4 +- cli/src/customize/mod.rs | 328 +++++++++++++++++++++------------ cli/src/error.rs | 72 ++++++-- cli/src/main.rs | 13 +- core/src/cache.rs | 15 +- core/src/eval/mod.rs | 6 +- core/src/parser/mod.rs | 6 +- core/src/position.rs | 8 +- core/src/program.rs | 101 ++++++++-- core/src/repl/mod.rs | 4 +- 10 files changed, 397 insertions(+), 160 deletions(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index 6f2c27e0f8..d6a32bee69 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -12,7 +12,7 @@ const NICKEL_VALUE_NAME: &str = "NICKEL EXPRESSION"; #[derive(Debug, Clone, Default)] pub(super) struct TermInterface { // We use a BTreeMap so that the end result is being sorted as we build the interface. - pub(super) fields: BTreeMap, + pub(super) fields: HashMap, } /// The interface of a specific field. This fields can be itself a record and contain subfields. @@ -28,7 +28,7 @@ pub(super) struct FieldInterface { #[derive(Debug, Clone, Default)] pub(super) struct CustomizableField { /// The path of the field. - pub(super) path: Vec, + pub(super) path: FieldPath, pub(super) interface: FieldInterface, } diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index b042fbdd94..74de61152d 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -3,21 +3,19 @@ //! The customize mode is a way to interact with a specific Nickel program on the command line by //! setting or overriding values directly from the command line. It's currently enabled by the //! freeform marker `--`. -use std::{ - collections::{BTreeMap, HashMap}, - ffi::OsString, -}; +use std::{collections::HashMap, ffi::OsString}; -use clap::{parser::ValueSource, ArgAction, Command}; +use clap::Parser; use nickel_lang_core::{ combine::Combine, eval::cache::lazy::CBNCache, identifier::LocIdent, - program::{FieldOverride, Program}, + program::{FieldPath, Program}, + repl::query_print, repl::query_print::{write_query_result, Attributes}, term::{ record::{Field, RecordData}, - LabeledType, MergePriority, RuntimeContract, Term, TypeAnnotation, + LabeledType, MergePriority, RuntimeContract, Term, }, typ::{RecordRowF, RecordRowsIteratorItem, Type, TypeF}, }; @@ -28,13 +26,9 @@ pub mod interface; use interface::{CustomizableField, FieldInterface, TermInterface}; -/// The maximal number of overridable fields displayed. Because there might be a lot of them, we -/// don't list them all by default. -const OVERRIDES_LIST_MAX_COUNT: usize = 15; - /// The value name used through the CLI to indicate that an option accepts any Nickel expression as /// a value. -const ASSIGNMENT_SYNTAX: &str = "FIELD_PATH=NICKEL EXPRESSION"; +const ASSIGNMENT_SYNTAX: &str = "FIELD_PATH=NICKEL_EXPRESSION"; const EXPERIMENTAL_MSG: &str = "[WARNING] Customize mode is experimental. Its interface is subject to breaking changes."; @@ -50,41 +44,151 @@ pub struct CustomizeMode { pub customize_mode: Vec, } -impl CustomizeMode { - fn make_cmd() -> clap::Command { - let assignment = clap::Arg::new("assignment").action(ArgAction::Append); - - let override_arg = clap::Arg::new("override") - .long("override") - .value_name(ASSIGNMENT_SYNTAX.to_owned()) - .action(ArgAction::Append) - .help( - "\ - Override any field of the configuration with a valid Nickel expression provided as \ - a string. The new value will be merged with the configuration with a `force` \ - priority.", - ); - - Command::new("customize-mode") - .about("Customize a Nickel configuration through the command line before exporting") - .after_help(EXPERIMENTAL_MSG) - .no_binary_name(true) - .arg(override_arg) +#[derive(clap::Parser)] +#[command( + name = "customize-mode", + after_help = EXPERIMENTAL_MSG, + no_binary_name = true, +)] +/// Customize a Nickel configuration through the command line. +struct CustomizeOptions { + /// Assign a valid Nickel expression to an input field of the configuration. The new value + /// will be merged with the configuration with priority 0 (the one assigned by default in + /// Nickel when no explicit merge priority is provided). + /// + /// Assignment can only set input fields, that is fields without definition or fields with + /// a default value. To override an existing value, use `--override` instead. + /// + /// Note that you might have to escape special characters or enclose assignments in quotes to + /// prevent shell interpretation. + /// + /// Example: `nickel eval config.ncl -- 'http.enabled="yes"' protocol=\'ftp` + #[clap(value_name = ASSIGNMENT_SYNTAX)] + assignment: Vec, + + /// Override any field of the configuration with a valid Nickel expression. The new value + /// will be merged with the configuration with a `force` priority. + /// + /// Note that you might have to escape special characters or enclose assignments in quotes to + /// prevent shell interpretation. + /// + /// Example: `-- input.value=false --override m.count=2 --override m.type=\'server` + #[clap(long = "override", value_name = ASSIGNMENT_SYNTAX)] + ovd: Vec, + + #[command(subcommand)] + pub command: Option, +} + +#[derive(clap::Subcommand)] +enum CustomizeCommand { + /// Show the documentation of a particular field. + Show(ShowCommand), + /// List the input fields and the overridable fields of the configuration. + List(ListCommand), +} + +#[derive(clap::Parser)] +struct ListCommand; + +impl ListCommand { + fn run(self, customize_fields: &CustomizableFields) -> CliResult<()> { + let mut inputs = customize_fields + .inputs + .values() + .map(|field| field.path.to_string()) + .collect::>(); + inputs.sort(); + + let mut overrides = customize_fields + .overrides + .values() + .map(|field| field.path.to_string()) + .collect::>(); + overrides.sort(); + + println!("Input fields:"); + for input in inputs { + println!("- {}", input); + } + + println!("\nOverridable fields (require `--override`):"); + for override_ in overrides { + println!("- {}", override_); + } + + Ok(()) } } -/// An assignment of the form `path.to.field=value`. -struct FieldAssignment { - path: Vec, - value: String, +#[derive(clap::Parser)] +struct DoCommand { + /// Assign a valid Nickel expression to an input field of the configuration. The new value + /// will be merged with the configuration with priority 0 (the one assigned by default in + /// Nickel when no explicit merge priority is provided). + /// + /// Assignment can only set input fields, that is fields without definition or fields with + /// a default value. To override an existing value, use `--override` instead. + #[clap(value_name = ASSIGNMENT_SYNTAX)] + assignment: Vec, + + /// Override any field of the configuration with a valid Nickel expression. The new value + /// will be merged with the configuration with a `force` priority. + #[clap(long = "override", value_name = ASSIGNMENT_SYNTAX)] + ovd: Vec, +} + +impl DoCommand {} + +#[derive(clap::Parser)] +struct ShowCommand { + /// Print the documentation and metadata of a particular overridable field. + #[clap(value_name = "FIELD_PATH")] + field: String, +} + +impl ShowCommand { + fn run( + self, + mut program: Program, + customizable_fields: &CustomizableFields, + ) -> CliResult<()> { + let path = match program.parse_field_path(self.field) { + Ok(path) => path, + Err(parse_error) => { + return CliResult::Err(crate::error::Error::CliUsage { + error: crate::error::CliUsageError::FieldPathParseError { error: parse_error }, + program, + }) + } + }; + + let field = match customizable_fields + .inputs + .get(&path) + .or(customizable_fields.overrides.get(&path)) + { + Some(field) => &field.interface.field, + None => { + return CliResult::Err(crate::error::Error::CliUsage { + error: crate::error::CliUsageError::UnknownField { path }, + program, + }) + } + }; + + query_print::write_query_result(&mut std::io::stdout(), field, Default::default()).unwrap(); + + Ok(()) + } } /// Fields of the configuration which aren't themselves records and can be assigned or overridden /// through the customize mode. #[derive(Clone, Debug, Default)] struct CustomizableFields { - inputs: BTreeMap, CustomizableField>, - overrides: BTreeMap, CustomizableField>, + inputs: HashMap, + overrides: HashMap, } impl CustomizableFields { @@ -93,14 +197,14 @@ impl CustomizableFields { let mut this = Self::default(); for (id, field_iface) in term_iface.fields { - this.extend_with(field_iface, vec![id]); + this.extend_with(field_iface, FieldPath(vec![id])); } this } /// Register a customizable field that can be assigned (input). - fn register_input(&mut self, path: Vec, interface: FieldInterface) { + fn register_input(&mut self, path: FieldPath, interface: FieldInterface) { debug_assert!(!self.inputs.contains_key(&path)); debug_assert!(!self.overrides.contains_key(&path)); @@ -109,7 +213,7 @@ impl CustomizableFields { } /// Register a customizable field that can be overridden. - fn register_override(&mut self, path: Vec, interface: FieldInterface) { + fn register_override(&mut self, path: FieldPath, interface: FieldInterface) { debug_assert!(!self.inputs.contains_key(&path)); debug_assert!(!self.overrides.contains_key(&path)); @@ -119,7 +223,7 @@ impl CustomizableFields { /// Enrich the current list of customizable fields with the field(s) exposed by a given field /// interface. - fn extend_with(&mut self, field_iface: FieldInterface, path: Vec) { + fn extend_with(&mut self, field_iface: FieldInterface, path: FieldPath) { // This is a terminal field, which gives rise to an argument or an overridable value. if !field_iface.has_subfields() { if field_iface.is_input() { @@ -134,7 +238,7 @@ impl CustomizableFields { .flat_map(|intf| intf.fields.into_iter()) { let mut path = path.clone(); - path.push(id); + path.0.push(id); self.extend_with(child_iface, path); } @@ -146,55 +250,29 @@ pub trait Customize { fn customize(&self, program: Program) -> CliResult>; } -impl Customize for CustomizeMode { - // XXX: we should give a nice error message when someone tries to evaluate some - // expression that has unset values, telling them they can set them using - // this method - fn customize(&self, mut program: Program) -> CliResult> { - if self.customize_mode.is_empty() { - return Ok(program); - } - - let evaled = match program.eval_record_spine() { - Ok(evaled) => evaled, - // We need a `return` control-flow to be able to take `program` out - Err(error) => return CliResult::Err(crate::error::Error::Program { error, program }), - }; - - let customizable_fields = CustomizableFields::new(TermInterface::from(evaled.as_ref())); - let clap_cmd = Self::make_cmd(); - let arg_matches = clap_cmd.get_matches_from(self.customize_mode.iter()); - - let assignment_overrides: Result, super::error::CliUsageError> = arg_matches - .get_occurrences("assignment") +impl CustomizeOptions { + fn do_customize( + self, + customizable_fields: CustomizableFields, + mut program: Program, + ) -> CliResult> { + let assignment_overrides: Result, super::error::CliUsageError> = self + .assignment .into_iter() - .flat_map(|occurs| { - occurs.map(|mut occurrence_value| { - let assignment: &String = occurrence_value.next().unwrap(); - let (path_str, value_str): (&String, &String) = todo!(); - let path: Vec = todo!(); - - customizable_fields - .inputs - .get(&path) - .map(|intf| FieldOverride { - path: intf.path.clone(), - value: value_str.clone(), - priority: MergePriority::default(), - }) - .ok_or_else(|| { - if customizable_fields.inputs.contains_key(&path) { - super::error::CliUsageError::CantAssignNonInput { - path: path_str.clone(), - assignment: todo!(), - } - } else { - super::error::CliUsageError::UnknownFieldOverride { - path: path_str.clone(), - } - } - }) - }) + .map(|assignment| { + let ovd = program + .parse_override(assignment, MergePriority::default()) + .map_err(|error| super::error::CliUsageError::AssignmentParseError { error })?; + + if !customizable_fields.inputs.contains_key(&ovd.path) { + if customizable_fields.overrides.contains_key(&ovd.path) { + Err(super::error::CliUsageError::CantAssignNonInput { ovd }) + } else { + Err(super::error::CliUsageError::UnknownFieldAssignment { path: ovd.path }) + } + } else { + Ok(ovd) + } }) .collect(); @@ -203,29 +281,21 @@ impl Customize for CustomizeMode { Err(error) => return CliResult::Err(crate::error::Error::CliUsage { error, program }), }; - let force_overrides: Result, super::error::CliUsageError> = arg_matches - .get_occurrences("override") + let force_overrides: Result, super::error::CliUsageError> = self + .ovd .into_iter() - .flat_map(|occurs| { - occurs.map(|mut pair| { - let (path_str, value_str): (&String, &String) = - (pair.next().unwrap(), pair.next().unwrap()); - let path: Vec = todo!(); - - customizable_fields - .overrides - .get(&path) - // We allow --override to set inputs as well. - .or(customizable_fields.inputs.get(&path)) - .map(|intf| FieldOverride { - path: intf.path.clone(), - value: value_str.clone(), - priority: MergePriority::Top, - }) - .ok_or_else(|| super::error::CliUsageError::UnknownFieldOverride { - path: path_str.clone(), - }) - }) + .map(|assignment| { + let ovd = program + .parse_override(assignment, MergePriority::Top) + .map_err(|error| super::error::CliUsageError::AssignmentParseError { error })?; + + if !customizable_fields.inputs.contains_key(&ovd.path) + && !customizable_fields.overrides.contains_key(&ovd.path) + { + Err(super::error::CliUsageError::UnknownFieldOverride { path: ovd.path }) + } else { + Ok(ovd) + } }) .collect(); @@ -239,6 +309,38 @@ impl Customize for CustomizeMode { } } +impl Customize for CustomizeMode { + // XXX: we should give a nice error message when someone tries to evaluate some + // expression that has unset values, telling them they can set them using + // this method + fn customize(&self, mut program: Program) -> CliResult> { + if self.customize_mode.is_empty() { + return Ok(program); + } + + let evaled = match program.eval_record_spine() { + Ok(evaled) => evaled, + // We need a `return` control-flow to be able to take `program` out + Err(error) => return CliResult::Err(crate::error::Error::Program { error, program }), + }; + + let customizable_fields = CustomizableFields::new(TermInterface::from(evaled.as_ref())); + let opts = CustomizeOptions::parse_from(self.customize_mode.iter()); + + match opts.command { + None => opts.do_customize(customizable_fields, program), + Some(CustomizeCommand::List(list_command)) => { + list_command.run(&customizable_fields)?; + Err(crate::error::Error::CustomizeInfoPrinted) + } + Some(CustomizeCommand::Show(show_command)) => { + show_command.run(program, &customizable_fields)?; + Err(crate::error::Error::CustomizeInfoPrinted) + } + } + } +} + #[derive(clap::Args, Debug)] pub struct NoCustomizeMode; diff --git a/cli/src/error.rs b/cli/src/error.rs index 8fd7ba7d51..08975f5b63 100644 --- a/cli/src/error.rs +++ b/cli/src/error.rs @@ -1,17 +1,24 @@ use nickel_lang_core::{ - error::{Diagnostic, Files, IntoDiagnostics}, + error::{Diagnostic, FileId, Files, IntoDiagnostics, ParseError}, eval::cache::lazy::CBNCache, program::Program, + program::{FieldOverride, FieldPath}, }; /// Errors related to mishandling the CLI. pub enum CliUsageError { /// Tried to override a field which doesn't exist. - UnknownFieldOverride { path: String }, + UnknownFieldOverride { path: FieldPath }, /// Tried to assign a field which doesn't exist. - UnknownFieldAssignment { path: String }, + UnknownFieldAssignment { path: FieldPath }, + /// Tried to show information about a field which doesn't exist. + UnknownField { path: FieldPath }, /// Tried to override an defined field without the `--override` argument. - CantAssignNonInput { path: String, assignment: String }, + CantAssignNonInput { ovd: FieldOverride }, + /// A parse error occurred when trying to parse an assignment. + AssignmentParseError { error: ParseError }, + /// A parse error occurred when trying to parse a field path. + FieldPathParseError { error: ParseError }, } pub enum Error { @@ -36,15 +43,21 @@ pub enum Error { program: Program, error: CliUsageError, }, + /// Not an actual failure but a special early return to indicate that information was printed + /// during the usage of the customize mode, because a subcommand such as `list`, `show`, etc. + /// was used, and thus no customized program can be returned. + /// + /// Upon receiving this error, the caller should simply exit without proceeding with evaluation. + CustomizeInfoPrinted, } -impl IntoDiagnostics for CliUsageError { +impl IntoDiagnostics for CliUsageError { fn into_diagnostics( self, - _files: &mut Files, - _stdlib_ids: Option<&Vec>, + files: &mut Files, + stdlib_ids: Option<&Vec>, ) -> Vec> { - fn mk_unknown_diags(path: String, method: &str) -> Vec> { + fn mk_unknown_diags(path: &FieldPath, method: &str) -> Vec> { vec![Diagnostic::error() .with_message(format!("invalid {method}: unknown field `{path}`")) .with_notes(vec![format!( @@ -54,9 +67,12 @@ impl IntoDiagnostics for CliUsageError { } match self { - CliUsageError::UnknownFieldOverride { path } => mk_unknown_diags(path, "override"), - CliUsageError::UnknownFieldAssignment { path } => mk_unknown_diags(path, "assignment"), - CliUsageError::CantAssignNonInput { path, assignment } => { + CliUsageError::UnknownFieldOverride { path } => mk_unknown_diags(&path, "override"), + CliUsageError::UnknownFieldAssignment { path } => mk_unknown_diags(&path, "assignment"), + CliUsageError::UnknownField { path } => mk_unknown_diags(&path, "query"), + CliUsageError::CantAssignNonInput { + ovd: FieldOverride { path, value, .. }, + } => { vec![Diagnostic::error() .with_message(format!("invalid assignment: `{path}` isn't an input")) .with_notes(vec![ @@ -66,10 +82,39 @@ impl IntoDiagnostics for CliUsageError { ), format!( "If you really want to override this field, please use \ - `--override {assignment}` instead." + `--override '{path}={value}'` instead." ), ])] } + CliUsageError::AssignmentParseError { error } => { + let mut diags = IntoDiagnostics::into_diagnostics(error, files, stdlib_ids); + diags.push( + Diagnostic::note() + .with_message("when parsing a field assignment on the command line") + .with_notes(vec![ + "A field assignment must be of the form `=`, \ + where `` is a dot-separated list of fields and `` \ + is a valid Nickel expression." + .to_owned(), + "For example: `config.database.\"$port\"=8080`".to_owned(), + ]), + ); + diags + } + CliUsageError::FieldPathParseError { error } => { + let mut diags = IntoDiagnostics::into_diagnostics(error, files, stdlib_ids); + diags.push( + Diagnostic::note() + .with_message("when parsing a field path on the command line") + .with_notes(vec![ + "A field path must be a dot-separated list of fields. Fields \ + with spaces or special characters must be properly quoted." + .to_owned(), + "For example: `config.database.\"$port\"`".to_owned(), + ]), + ); + diags + } } } } @@ -156,6 +201,9 @@ impl Error { #[cfg(feature = "format")] Error::Format { error } => eprintln!("{error}"), Error::CliUsage { error, mut program } => program.report(error), + Error::CustomizeInfoPrinted => { + // Nothing to do, the caller should simply exit. + } } } } diff --git a/cli/src/main.rs b/cli/src/main.rs index af35360891..bff75e0dc4 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -56,10 +56,13 @@ fn main() -> ExitCode { metrics.report(); } - if let Err(e) = result { - e.report(); - ExitCode::FAILURE - } else { - ExitCode::SUCCESS + match result { + // CustomizeInfoPrinted is used for early return, but it's not actually an error for the + // user's point of view. + Ok(()) | Err(error::Error::CustomizeInfoPrinted) => ExitCode::SUCCESS, + Err(error) => { + error.report(); + ExitCode::FAILURE + } } } diff --git a/core/src/cache.rs b/core/src/cache.rs index f931d6c323..e6baac4f7e 100644 --- a/core/src/cache.rs +++ b/core/src/cache.rs @@ -3,11 +3,11 @@ use crate::error::{Error, ImportError, ParseError, ParseErrors, TypecheckError}; use crate::eval::cache::Cache as EvalCache; use crate::eval::Closure; -use crate::identifier::LocIdent; #[cfg(feature = "nix-experimental")] use crate::nix_ffi; use crate::parser::{lexer::Lexer, ErrorTolerantParser}; use crate::position::TermPos; +use crate::program::FieldPath; use crate::stdlib::{self as nickel_stdlib, StdlibModule}; use crate::term::array::Array; use crate::term::record::{Field, RecordData}; @@ -269,7 +269,8 @@ pub enum SourcePath { ReplInput(usize), ReplTypecheck, ReplQuery, - Override(Vec), + CliFieldAssignment, + Override(FieldPath), Generated(String), } @@ -297,14 +298,8 @@ impl From for OsString { SourcePath::ReplInput(idx) => format!("").into(), SourcePath::ReplTypecheck => "".into(), SourcePath::ReplQuery => "".into(), - SourcePath::Override(path) => format!( - "", - path.iter() - .map(ToString::to_string) - .collect::>() - .join(".") - ) - .into(), + SourcePath::CliFieldAssignment => "".into(), + SourcePath::Override(path) => format!("",).into(), SourcePath::Generated(description) => format!("", description).into(), } } diff --git a/core/src/eval/mod.rs b/core/src/eval/mod.rs index 3b98f92879..c11bb71ae9 100644 --- a/core/src/eval/mod.rs +++ b/core/src/eval/mod.rs @@ -85,7 +85,7 @@ use crate::{ identifier::LocIdent, match_sharedterm, position::TermPos, - program::QueryPath, + program::FieldPath, term::{ array::ArrayAttrs, make as mk_term, @@ -235,13 +235,13 @@ impl VirtualMachine { /// `baz`. The content of `baz` is evaluated as well, and variables are substituted, in order /// to obtain a value that can be printed. The metadata and the evaluated value are returned as /// a new field. - pub fn query(&mut self, t: RichTerm, path: QueryPath) -> Result { + pub fn query(&mut self, t: RichTerm, path: FieldPath) -> Result { self.query_closure(Closure::atomic_closure(t), path) } /// Same as [VirtualMachine::query], but starts from a closure instead of a term in an empty /// environment. - pub fn query_closure(&mut self, closure: Closure, path: QueryPath) -> Result { + pub fn query_closure(&mut self, closure: Closure, path: FieldPath) -> Result { let mut prev_pos = closure.body.pos; let Closure { body: rt, mut env } = self.eval_closure(closure)?; diff --git a/core/src/parser/mod.rs b/core/src/parser/mod.rs index 7faf87f2ad..de16cbed2e 100644 --- a/core/src/parser/mod.rs +++ b/core/src/parser/mod.rs @@ -1,5 +1,6 @@ use crate::error::{ParseError, ParseErrors}; use crate::identifier::LocIdent; +use crate::position::RawSpan; use crate::term::RichTerm; use crate::typ::Type; use codespan::FileId; @@ -90,7 +91,10 @@ generate_lalrpop_parser_impl!(grammar::ExtendedTermParser, ExtendedTerm); generate_lalrpop_parser_impl!(grammar::TermParser, RichTerm); generate_lalrpop_parser_impl!(grammar::FixedTypeParser, Type); generate_lalrpop_parser_impl!(grammar::StaticFieldPathParser, Vec); -generate_lalrpop_parser_impl!(grammar::CliFieldAssignmentParser, (Vec, RichTerm)); +generate_lalrpop_parser_impl!( + grammar::CliFieldAssignmentParser, + (Vec, RichTerm, RawSpan) +); /// Generic interface of the various specialized Nickel parsers. /// diff --git a/core/src/position.rs b/core/src/position.rs index 88191c2d18..f84a9543f8 100644 --- a/core/src/position.rs +++ b/core/src/position.rs @@ -3,7 +3,7 @@ //! The positions defined in this module are represented by the id of the corresponding source and //! raw byte indices. They are prefixed with Raw to differentiate them from codespan's types and //! indicate that they do not store human friendly data like lines and columns. -use codespan::{ByteIndex, FileId}; +use codespan::{self, ByteIndex, FileId}; use std::cmp::{max, min, Ordering}; /// A position identified by a byte offset in a file. @@ -67,6 +67,12 @@ impl RawSpan { } } +impl From for codespan::Span { + fn from(span: RawSpan) -> Self { + codespan::Span::new(span.start, span.end) + } +} + /// The position span of a term. #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Default)] pub enum TermPos { diff --git a/core/src/program.rs b/core/src/program.rs index 083e1a867e..807ca63635 100644 --- a/core/src/program.rs +++ b/core/src/program.rs @@ -38,15 +38,16 @@ use std::path::PathBuf; use std::{ ffi::OsString, + fmt, io::{self, Cursor, Read, Write}, result::Result, }; -/// Attribute path provided when querying metadata. -#[derive(Clone, Default, PartialEq, Eq, Debug)] -pub struct QueryPath(pub Vec); +/// A path of fields, that is a list, locating this field from the root of the configuration. +#[derive(Clone, Default, PartialEq, Eq, Debug, Hash)] +pub struct FieldPath(pub Vec); -impl QueryPath { +impl FieldPath { pub fn new() -> Self { Self::default() } @@ -62,9 +63,7 @@ impl QueryPath { /// 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::StaticFieldPathParser, lexer::Lexer, utils::FieldPathElem, ErrorTolerantParser, - }; + use crate::parser::{grammar::StaticFieldPathParser, lexer::Lexer, ErrorTolerantParser}; let input_id = cache.replace_string(SourcePath::Query, input); let s = cache.source(input_id); @@ -80,7 +79,7 @@ impl QueryPath { ) })?; - Ok(QueryPath(field_path)) + Ok(FieldPath(field_path)) } /// As [`Self::parse`], but accepts an `Option` to accomodate for the absence of path. If the @@ -93,18 +92,76 @@ impl QueryPath { } } +impl fmt::Display for FieldPath { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + use crate::pretty::ident_quoted; + + write!( + f, + "{}", + self.0 + .iter() + .map(ident_quoted) + .collect::>() + .join(".") + ) + } +} + /// Several CLI commands accept additional overrides specified directly on the command line. They /// are represented by this structure. #[derive(Clone)] pub struct FieldOverride { /// The field path identifying the (potentially nested) field to override. - pub path: Vec, + pub path: FieldPath, /// The overriding value. pub value: String, /// The priority associated with this override. pub priority: MergePriority, } +impl FieldOverride { + /// Parse an assignment `path.to.field=value` to a field override, with the priority given as a + /// separate argument. + /// + /// The parser actually entirely parse the `value` part to a [crate::term::RichTerm] (have it + /// accept anything after the equal sign is harder than actually parsing it), but what we need + /// at this point is just a string. Thus, `parse` uses the span to extract back the `value` part + /// of the input string. + pub fn parse( + cache: &mut Cache, + assignment: String, + priority: MergePriority, + ) -> Result { + use crate::parser::{grammar::CliFieldAssignmentParser, lexer::Lexer, ErrorTolerantParser}; + + let input_id = cache.replace_string(SourcePath::CliFieldAssignment, assignment); + let s = cache.source(input_id); + + let parser = CliFieldAssignmentParser::new(); + let (path, _, span_value) = parser + .parse_strict(input_id, Lexer::new(s)) + // We just need to report an error here + .map_err(|mut errs| { + errs.errors.pop().expect( + "because parsing of the field assignment failed, the error \ + list must be non-empty, put .pop() failed", + ) + })?; + + let value = cache + .files() + .source_slice(span_value.src_id, span_value) + .expect("the span coming from the parser must be valid"); + + Ok(FieldOverride { + path: FieldPath(path), + value: value.to_owned(), + priority, + }) + } +} + /// A Nickel program. /// /// Manage a file database, which stores the original source code of the program and eventually the @@ -204,6 +261,28 @@ impl Program { }) } + /// Parse an assignment of the form `path.to_field=value` as an override, with the provided + /// merge priority. Assignments are typically provided by the user on the command line, as part + /// of the customize mode. + /// + /// This method simply calls [FieldOverride::parse] with the [crate::cache::Cache] of the + /// current program. + pub fn parse_override( + &mut self, + assignment: String, + priority: MergePriority, + ) -> Result { + FieldOverride::parse(self.vm.import_resolver_mut(), assignment, priority) + } + + /// Parse a dot-separated field path of the form `path.to.field`. + /// + /// This method simply calls [FieldPath::parse] with the [crate::cache::Cache] of the current + /// program. + pub fn parse_field_path(&mut self, path: String) -> Result { + FieldPath::parse(self.vm.import_resolver_mut(), path) + } + pub fn add_overrides(&mut self, overrides: impl IntoIterator) { self.overrides.extend(overrides); } @@ -241,7 +320,7 @@ impl Program { .add_string(SourcePath::Override(ovd.path.clone()), ovd.value); self.vm.prepare_eval(value_file_id)?; record = record - .path(ovd.path) + .path(ovd.path.0) .priority(ovd.priority) .value(Term::ResolvedImport(value_file_id)); } @@ -305,7 +384,7 @@ impl Program { /// Prepare for evaluation, then query the program for a field. pub fn query(&mut self, path: Option) -> Result { let rt = self.prepare_eval()?; - let query_path = QueryPath::parse_opt(self.vm.import_resolver_mut(), path)?; + let query_path = FieldPath::parse_opt(self.vm.import_resolver_mut(), path)?; Ok(self.vm.query(rt, query_path)?) } diff --git a/core/src/repl/mod.rs b/core/src/repl/mod.rs index e3c7817f88..0aea3834a5 100644 --- a/core/src/repl/mod.rs +++ b/core/src/repl/mod.rs @@ -12,7 +12,7 @@ use crate::eval::cache::Cache as EvalCache; use crate::eval::{Closure, VirtualMachine}; use crate::identifier::LocIdent; use crate::parser::{grammar, lexer, ErrorTolerantParser, ExtendedTerm}; -use crate::program::QueryPath; +use crate::program::FieldPath; use crate::term::TraverseOrder; use crate::term::{record::Field, RichTerm, Term, Traverse}; use crate::transform::import_resolution; @@ -310,7 +310,7 @@ impl Repl for ReplImpl { fn query(&mut self, path: String) -> Result { self.vm.reset(); - let mut query_path = QueryPath::parse(self.vm.import_resolver_mut(), path)?; + let mut query_path = FieldPath::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 From 8abbf77e6169c67e5aa322b4b736b6b0c022731a Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Nov 2023 18:35:50 +0100 Subject: [PATCH 05/19] Fix compilation warnings --- cli/src/customize/interface.rs | 184 ++++----------------------------- cli/src/customize/mod.rs | 1 - core/src/error/mod.rs | 2 +- 3 files changed, 21 insertions(+), 166 deletions(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index d6a32bee69..2a459e79f3 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -2,8 +2,6 @@ //! customize mode of the command-line. use super::*; -const NICKEL_VALUE_NAME: &str = "NICKEL EXPRESSION"; - /// The interface of a configuration (a Nickel program) which represents all nested field paths /// that are accessible from the root term together with their associated metadata. /// @@ -32,105 +30,6 @@ pub(super) struct CustomizableField { pub(super) interface: FieldInterface, } -impl TermInterface { - // /// Build a command description from this interface. - // /// - // /// This method recursively lists all existing field paths, and reports input fields (as - // /// defined per [FieldInterface::is_input]) as stand-alone arguments. For example, if the - // /// configuration contains: - // /// - // /// ```nickel - // /// foo.bar.baz | Number, - // /// ``` - // /// - // /// This will give rise to a corresponding `--foo.bar.baz` argument listed in the `help` message - // /// which can be set. - // /// - // /// Non-input fields are still listed in the description of the `--override` command, which - // /// has the ability of setting any field. - // /// - // /// # Return - // /// - // /// In addition to the updated command, `build_cmd` returns a mapping from clap argument ids to - // /// their corresponding full field path as an array of fields. - // pub(super) fn build_cmd(&self) -> TermCommand { - // todo!() - - // let clap_cmd = Command::new("customize-mode") - // .about("Customize a Nickel configuration through the command line before exporting") - // .after_help(EXPERIMENTAL_MSG) - // .no_binary_name(true); - // - // let mut term_cmd = TermCommand::new(clap_cmd); - // - // for (id, field) in &self.fields { - // term_cmd = field.add_args(term_cmd, vec![id.to_string()]) - // } - // - // let mut overrides_list: Vec = term_cmd - // .overrides - // .keys() - // .take(OVERRIDES_LIST_MAX_COUNT) - // .map(|field_path| format!("- {field_path}")) - // .collect(); - // - // if term_cmd.overrides.len() > OVERRIDES_LIST_MAX_COUNT { - // overrides_list.push("- ...".into()); - // } - // - // let has_override = term_cmd.inputs.contains_key("override"); - // let has_help = term_cmd.inputs.contains_key("help"); - // - // let override_arg_label = "override"; - // let override_help = format!( - // "Override any field of the configuration with a valid Nickel expression provided as \ - // a string. The new value will be merged with the configuration with a `force` \ - // priority.\n\n\ - // Overridable fields:\n{}\n\n", - // overrides_list.join("\n") - // ); - // let override_arg = clap::Arg::new(override_arg_label) - // .long(override_arg_label) - // .value_name(NICKEL_VALUE_NAME.to_owned()) - // .number_of_values(2) - // .value_names(["field", "value"]) - // .action(ArgAction::Append) - // .required(false) - // .help(override_help); - // - // term_cmd.clap_cmd = term_cmd.clap_cmd.arg(override_arg); - // - // if has_help || has_override { - // let conflict_field = if has_override { - // "override" - // } else if has_help { - // "help" - // } else { - // // We tested in the parent `if` that one of those two booleans must be set - // unreachable!() - // }; - // - // let extra = if has_help && has_override { - // ". The same applies to the conflicting `help` field" - // } else { - // "" - // }; - // - // term_cmd.clap_cmd = term_cmd.clap_cmd.after_long_help(format!( - // "\ - // [CONFLICT] This configuration has a field named `{conflict_field}` which conflicts \ - // with the built-in `--{conflict_field}` argument. To set this field to e.g. \ - // \"some_value\", use `--override {conflict_field} \"some_value\"` instead of \ - // `--{conflict_field} \"some_value\"`\ - // {extra}\n\n\ - // {EXPERIMENTAL_MSG}" - // )); - // } - // - // term_cmd - // } -} - impl Combine for TermInterface { fn combine(first: Self, second: Self) -> Self { let TermInterface { mut fields } = first; @@ -278,26 +177,6 @@ impl From<&RecordRowF<&Type>> for FieldInterface { } impl FieldInterface { - /// Add a single argument to the CLI `cmd`, based on the interface of this field, and return - /// the updated command. - fn add_arg(&self, cmd: clap::Command, path: String) -> clap::Command { - let mut arg = clap::Arg::new(&path) - .long(path) - .value_name(self.value_name()) - // TODO: Create clap argument groups - .required(!self.is_default()); - - if let Some(help) = &self.help() { - arg = arg.help(help); - }; - - if let Some(default) = self.default_value() { - arg = arg.default_value(default); - } - - cmd.arg(arg) - } - /// Define if a field is an input of a configuration that is intended to be filled, and will be /// given a dedicated CLI argument. If the field is not an input, it can only be overridden via /// `--override`. @@ -333,48 +212,25 @@ impl FieldInterface { matches!(&self.subfields, Some(ref intf) if !intf.fields.is_empty()) } - /// Return the default value, if any. - pub(super) fn default_value(&self) -> Option { - match (&self.field.metadata.priority, &self.field.value) { - (MergePriority::Bottom, Some(value)) => Some(value.to_string()), - _ => None, - } - } - - /// Render a help message similar to the output of a metadata query to serve as an help text - /// for this argument. - pub(super) fn help(&self) -> Option { - let mut output: Vec = Vec::new(); - - // We only need to render the documentation: the rest is printed separately as part of the - // clap command that is built. - let attributes = Attributes { - doc: true, - contract: false, - typ: false, - default: false, - value: false, - }; - - write_query_result(&mut output, &self.field, attributes) - .unwrap_or(false) - .then(|| { - String::from_utf8(output) - .expect("the query printer should always output valid utf8") - }) - } - - fn value_name(&self) -> String { - let annotation = &self.field.metadata.annotation; + // /// Return the default value, if any. + // pub(super) fn default_value(&self) -> Option { + // match (&self.field.metadata.priority, &self.field.value) { + // (MergePriority::Bottom, Some(value)) => Some(value.to_string()), + // _ => None, + // } + // } - if annotation.is_empty() { - NICKEL_VALUE_NAME.into() - } else { - let anns: Vec = annotation - .iter() - .map(|ctr| ctr.label.typ.to_string()) - .collect(); - anns.join(",") - } - } + // fn value_name(&self) -> String { + // let annotation = &self.field.metadata.annotation; + // + // if annotation.is_empty() { + // NICKEL_VALUE_NAME.into() + // } else { + // let anns: Vec = annotation + // .iter() + // .map(|ctr| ctr.label.typ.to_string()) + // .collect(); + // anns.join(",") + // } + // } } diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index 74de61152d..577104eb6f 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -12,7 +12,6 @@ use nickel_lang_core::{ identifier::LocIdent, program::{FieldPath, Program}, repl::query_print, - repl::query_print::{write_query_result, Attributes}, term::{ record::{Field, RecordData}, LabeledType, MergePriority, RuntimeContract, Term, diff --git a/core/src/error/mod.rs b/core/src/error/mod.rs index 88da74fbee..3d01994cf0 100644 --- a/core/src/error/mod.rs +++ b/core/src/error/mod.rs @@ -1795,7 +1795,7 @@ impl IntoDiagnostics for ParseError { .into(), ]), ParseError::InterpolationInStaticPath { - input, + input: _, path_elem_span, } => Diagnostic::error() .with_message("string interpolation is forbidden within a query") From caa749a0c5a8d39d31037d5efc0c60adb6882295 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Nov 2023 18:50:57 +0100 Subject: [PATCH 06/19] Improve the output of `-- list` including type and contracts --- cli/src/customize/interface.rs | 35 ++++++++++++++-------------------- cli/src/customize/mod.rs | 16 ++++++++++++---- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index 2a459e79f3..faadbfbbad 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -212,25 +212,18 @@ impl FieldInterface { matches!(&self.subfields, Some(ref intf) if !intf.fields.is_empty()) } - // /// Return the default value, if any. - // pub(super) fn default_value(&self) -> Option { - // match (&self.field.metadata.priority, &self.field.value) { - // (MergePriority::Bottom, Some(value)) => Some(value.to_string()), - // _ => None, - // } - // } - - // fn value_name(&self) -> String { - // let annotation = &self.field.metadata.annotation; - // - // if annotation.is_empty() { - // NICKEL_VALUE_NAME.into() - // } else { - // let anns: Vec = annotation - // .iter() - // .map(|ctr| ctr.label.typ.to_string()) - // .collect(); - // anns.join(",") - // } - // } + /// Return the list of the type and contract annotations joined as a comma-separated string, if + /// any. + pub(super) fn type_and_contracts(&self) -> Option { + let annotation = &self.field.metadata.annotation; + + (!annotation.is_empty()).then(|| { + let anns: Vec = annotation + .iter() + .map(|ctr| ctr.label.typ.to_string()) + .collect(); + + anns.join(",") + }) + } } diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index 577104eb6f..af729d0b09 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -94,16 +94,24 @@ impl ListCommand { fn run(self, customize_fields: &CustomizableFields) -> CliResult<()> { let mut inputs = customize_fields .inputs - .values() - .map(|field| field.path.to_string()) + .iter() + .map(|(path, field)| { + let extra = field.interface.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); + format!("{path}{extra}") + }) .collect::>(); + inputs.sort(); let mut overrides = customize_fields .overrides - .values() - .map(|field| field.path.to_string()) + .iter() + .map(|(path, field)| { + let extra = field.interface.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); + format!("{path}{extra}") + }) .collect::>(); + overrides.sort(); println!("Input fields:"); From dee18434c97a1c020c75d6a6c689e9f1c07df80d Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Nov 2023 18:54:01 +0100 Subject: [PATCH 07/19] Remove unused datatype --- cli/src/customize/interface.rs | 9 --------- cli/src/customize/mod.rs | 16 ++++++++-------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index faadbfbbad..91a70a2628 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -21,15 +21,6 @@ pub(super) struct FieldInterface { pub(super) field: Field, } -/// Description of a field which is a leaf, i.e. which doesn't contain other fields. Those are the -/// fields that can be set from the customize command line. -#[derive(Debug, Clone, Default)] -pub(super) struct CustomizableField { - /// The path of the field. - pub(super) path: FieldPath, - pub(super) interface: FieldInterface, -} - impl Combine for TermInterface { fn combine(first: Self, second: Self) -> Self { let TermInterface { mut fields } = first; diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index af729d0b09..6c0e45f87e 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -23,7 +23,7 @@ use crate::error::CliResult; pub mod interface; -use interface::{CustomizableField, FieldInterface, TermInterface}; +use interface::{FieldInterface, TermInterface}; /// The value name used through the CLI to indicate that an option accepts any Nickel expression as /// a value. @@ -96,7 +96,7 @@ impl ListCommand { .inputs .iter() .map(|(path, field)| { - let extra = field.interface.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); + let extra = field.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); format!("{path}{extra}") }) .collect::>(); @@ -107,7 +107,7 @@ impl ListCommand { .overrides .iter() .map(|(path, field)| { - let extra = field.interface.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); + let extra = field.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); format!("{path}{extra}") }) .collect::>(); @@ -175,7 +175,7 @@ impl ShowCommand { .get(&path) .or(customizable_fields.overrides.get(&path)) { - Some(field) => &field.interface.field, + Some(field) => &field.field, None => { return CliResult::Err(crate::error::Error::CliUsage { error: crate::error::CliUsageError::UnknownField { path }, @@ -194,8 +194,8 @@ impl ShowCommand { /// through the customize mode. #[derive(Clone, Debug, Default)] struct CustomizableFields { - inputs: HashMap, - overrides: HashMap, + inputs: HashMap, + overrides: HashMap, } impl CustomizableFields { @@ -216,7 +216,7 @@ impl CustomizableFields { debug_assert!(!self.overrides.contains_key(&path)); self.inputs - .insert(path.clone(), CustomizableField { path, interface }); + .insert(path, interface); } /// Register a customizable field that can be overridden. @@ -225,7 +225,7 @@ impl CustomizableFields { debug_assert!(!self.overrides.contains_key(&path)); self.overrides - .insert(path.clone(), CustomizableField { path, interface }); + .insert(path, interface); } /// Enrich the current list of customizable fields with the field(s) exposed by a given field From 0f1fe54308ef9be2e6d48ad7b926b6559d9ca033 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Nov 2023 18:54:13 +0100 Subject: [PATCH 08/19] Formatting --- cli/src/customize/mod.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index 6c0e45f87e..39f2af25c7 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -96,7 +96,10 @@ impl ListCommand { .inputs .iter() .map(|(path, field)| { - let extra = field.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); + let extra = field + .type_and_contracts() + .map(|ctrs| format!(": <{ctrs}>")) + .unwrap_or_default(); format!("{path}{extra}") }) .collect::>(); @@ -107,7 +110,10 @@ impl ListCommand { .overrides .iter() .map(|(path, field)| { - let extra = field.type_and_contracts().map(|ctrs| format!(": <{ctrs}>")).unwrap_or_default(); + let extra = field + .type_and_contracts() + .map(|ctrs| format!(": <{ctrs}>")) + .unwrap_or_default(); format!("{path}{extra}") }) .collect::>(); @@ -215,8 +221,7 @@ impl CustomizableFields { debug_assert!(!self.inputs.contains_key(&path)); debug_assert!(!self.overrides.contains_key(&path)); - self.inputs - .insert(path, interface); + self.inputs.insert(path, interface); } /// Register a customizable field that can be overridden. @@ -224,8 +229,7 @@ impl CustomizableFields { debug_assert!(!self.inputs.contains_key(&path)); debug_assert!(!self.overrides.contains_key(&path)); - self.overrides - .insert(path, interface); + self.overrides.insert(path, interface); } /// Enrich the current list of customizable fields with the field(s) exposed by a given field From aa2014931c503a1801420a241f6b964af274a844 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Mon, 6 Nov 2023 19:23:59 +0100 Subject: [PATCH 09/19] Fix old tests and add new for customize mode --- .../assignment_syntax_error.ncl | 24 ++++++++++ .../field_path_syntax_error.ncl | 22 +++++++++ .../inputs/customize-mode/has_help.ncl | 9 ---- .../inputs/customize-mode/has_override.ncl | 9 ---- .../snapshot/inputs/customize-mode/list.ncl | 18 ++++++++ .../inputs/customize-mode/not_an_input.ncl | 2 +- .../snapshot/inputs/customize-mode/show.ncl | 19 ++++++++ .../inputs/customize-mode/simple_adder.ncl | 2 +- .../customize-mode/unknown_override.ncl | 9 ++-- .../customize-mode/unkonwn_field_path.ncl | 22 +++++++++ ...rt_stderr_assignment_syntax_error.ncl.snap | 15 +++++++ ...rt_stderr_field_path_syntax_error.ncl.snap | 15 +++++++ ...pshot__export_stderr_not_an_input.ncl.snap | 9 ++-- ...t__export_stderr_unknown_override.ncl.snap | 2 +- ..._export_stderr_unkonwn_field_path.ncl.snap | 8 ++++ .../snapshot__export_stdout_has_help.ncl.snap | 28 ------------ ...pshot__export_stdout_has_override.ncl.snap | 28 ------------ .../snapshot__export_stdout_help.ncl.snap | 45 ++++++++++++------- .../snapshot__export_stdout_list.ncl.snap | 14 ++++++ .../snapshot__export_stdout_show.ncl.snap | 9 ++++ 20 files changed, 204 insertions(+), 105 deletions(-) create mode 100644 cli/tests/snapshot/inputs/customize-mode/assignment_syntax_error.ncl create mode 100644 cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl delete mode 100644 cli/tests/snapshot/inputs/customize-mode/has_help.ncl delete mode 100644 cli/tests/snapshot/inputs/customize-mode/has_override.ncl create mode 100644 cli/tests/snapshot/inputs/customize-mode/list.ncl create mode 100644 cli/tests/snapshot/inputs/customize-mode/show.ncl create mode 100644 cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl create mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stderr_assignment_syntax_error.ncl.snap create mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stderr_field_path_syntax_error.ncl.snap create mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stderr_unkonwn_field_path.ncl.snap delete mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stdout_has_help.ncl.snap delete mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stdout_has_override.ncl.snap create mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap create mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap diff --git a/cli/tests/snapshot/inputs/customize-mode/assignment_syntax_error.ncl b/cli/tests/snapshot/inputs/customize-mode/assignment_syntax_error.ncl new file mode 100644 index 0000000000..e18c4e5181 --- /dev/null +++ b/cli/tests/snapshot/inputs/customize-mode/assignment_syntax_error.ncl @@ -0,0 +1,24 @@ +# capture = 'stderr' +# command = ['export'] +# extra_args = [ +# '--', +# 'input.foo.bar=="hello"', +# 'input.foo.baz=[]', +# '--override', +# 'unkonwn.field.path=null', +# ] +{ + input.foo.bar | String, + input.foo.baz | Array Number, + input.defaulted.subfield + | doc "Some documentation" + | default = 2, + + override.first = 1, + override.second = { + subsecond = { + subsubsecond = "a", + other = [], + } + } +} diff --git a/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl b/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl new file mode 100644 index 0000000000..50fdfc5aa8 --- /dev/null +++ b/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl @@ -0,0 +1,22 @@ +# capture = 'stderr' +# command = ['export'] +# extra_args = [ +# '--', +# 'show', +# 'input.+foo.baz', +# ] +{ + input.foo.bar | String, + input.foo.baz | Array Number, + input.defaulted.subfield + | doc "Some documentation" + | default = 2, + + override.first = 1, + override.second = { + subsecond = { + subsubsecond = "a", + other = [], + } + } +} diff --git a/cli/tests/snapshot/inputs/customize-mode/has_help.ncl b/cli/tests/snapshot/inputs/customize-mode/has_help.ncl deleted file mode 100644 index 878dc186d6..0000000000 --- a/cli/tests/snapshot/inputs/customize-mode/has_help.ncl +++ /dev/null @@ -1,9 +0,0 @@ -# capture = 'stdout' -# command = ['export'] -# extra_args = ['--', '--help'] -{ - help | String, - input | Number, - - output = if input == 0 then help else "", -} diff --git a/cli/tests/snapshot/inputs/customize-mode/has_override.ncl b/cli/tests/snapshot/inputs/customize-mode/has_override.ncl deleted file mode 100644 index f0cea7e051..0000000000 --- a/cli/tests/snapshot/inputs/customize-mode/has_override.ncl +++ /dev/null @@ -1,9 +0,0 @@ -# capture = 'stdout' -# command = ['export'] -# extra_args = ['--', '--help'] -{ - override | String, - input | Number, - - output = if input == 0 then override else "", -} diff --git a/cli/tests/snapshot/inputs/customize-mode/list.ncl b/cli/tests/snapshot/inputs/customize-mode/list.ncl new file mode 100644 index 0000000000..b704371991 --- /dev/null +++ b/cli/tests/snapshot/inputs/customize-mode/list.ncl @@ -0,0 +1,18 @@ +# capture = 'stdout' +# command = ['export'] +# extra_args = ['--', 'list'] +{ + input.foo.bar | String, + input.foo.baz | Array Number, + input.defaulted.subfield + | doc "Some documentation" + | default = 2, + + override.first = 1, + override.second = { + subsecond = { + subsubsecond = "a", + other = [], + } + } +} diff --git a/cli/tests/snapshot/inputs/customize-mode/not_an_input.ncl b/cli/tests/snapshot/inputs/customize-mode/not_an_input.ncl index 1aad959e5b..6c811d15e5 100644 --- a/cli/tests/snapshot/inputs/customize-mode/not_an_input.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/not_an_input.ncl @@ -1,6 +1,6 @@ # capture = 'stderr' # command = ['export'] -# extra_args =['--', '--override.first', '"test"'] +# extra_args =['--', 'override.first="test"'] { input.foo.bar | String, input.foo.baz | Array Number, diff --git a/cli/tests/snapshot/inputs/customize-mode/show.ncl b/cli/tests/snapshot/inputs/customize-mode/show.ncl new file mode 100644 index 0000000000..7fcda1dc7c --- /dev/null +++ b/cli/tests/snapshot/inputs/customize-mode/show.ncl @@ -0,0 +1,19 @@ +# capture = 'stdout' +# command = ['export'] +# extra_args = ['--', 'show', 'input.defaulted.subfield'] +{ + input.foo.bar | String, + input.foo.baz | Array Number, + input.defaulted.subfield + | std.number.Integer + | doc "Some documentation" + | default = 2, + + override.first = 1, + override.second = { + subsecond = { + subsubsecond = "a", + other = [], + } + } +} diff --git a/cli/tests/snapshot/inputs/customize-mode/simple_adder.ncl b/cli/tests/snapshot/inputs/customize-mode/simple_adder.ncl index b6cf1ad08e..6852f34f85 100644 --- a/cli/tests/snapshot/inputs/customize-mode/simple_adder.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/simple_adder.ncl @@ -1,6 +1,6 @@ # capture = 'stdout' # command = ['export'] -# extra_args = ['--', '--input', '5'] +# extra_args = ['--', 'input=5'] { input | Number, output = input + 1, diff --git a/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl b/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl index b24bf72475..016f3f830b 100644 --- a/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl @@ -2,13 +2,10 @@ # command = ['export'] # extra_args = [ # '--', -# '--input.foo.bar', -# '"hello"', -# '--input.foo.baz', -# '[]', +# 'input.foo.bar="hello"', +# 'input.foo.baz=[]', # '--override', -# 'unkonwn.field.path', -# 'null' +# 'unkonwn.field.path=null', # ] { input.foo.bar | String, diff --git a/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl b/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl new file mode 100644 index 0000000000..2d4671d249 --- /dev/null +++ b/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl @@ -0,0 +1,22 @@ +# capture = 'stderr' +# command = ['export'] +# extra_args = [ +# '--', +# 'show', +# 'unkonwn.field.path', +# ] +{ + input.foo.bar | String, + input.foo.baz | Array Number, + input.defaulted.subfield + | doc "Some documentation" + | default = 2, + + override.first = 1, + override.second = { + subsecond = { + subsubsecond = "a", + other = [], + } + } +} diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_assignment_syntax_error.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_assignment_syntax_error.ncl.snap new file mode 100644 index 0000000000..2b36316513 --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_assignment_syntax_error.ncl.snap @@ -0,0 +1,15 @@ +--- +source: cli/tests/snapshot/main.rs +expression: err +--- +error: unexpected token + ┌─ :1:14 + │ +1 │ input.foo.bar=="hello" + │ ^^ + +note: when parsing a field assignment on the command line + = A field assignment must be of the form `=`, where `` is a dot-separated list of fields and `` is a valid Nickel expression. + = For example: `config.database."$port"=8080` + + diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_field_path_syntax_error.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_field_path_syntax_error.ncl.snap new file mode 100644 index 0000000000..a3b689fe38 --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_field_path_syntax_error.ncl.snap @@ -0,0 +1,15 @@ +--- +source: cli/tests/snapshot/main.rs +expression: err +--- +error: unexpected token + ┌─ :1:7 + │ +1 │ input.+foo.baz + │ ^ + +note: when parsing a field path on the command line + = A field path must be a dot-separated list of fields. Fields with spaces or special characters must be properly quoted. + = For example: `config.database."$port"` + + diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_not_an_input.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_not_an_input.ncl.snap index 1828a14aa1..a79eeee2d7 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stderr_not_an_input.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_not_an_input.ncl.snap @@ -2,11 +2,8 @@ source: cli/tests/snapshot/main.rs expression: err --- -error: unexpected argument '--override.first' found +error: invalid assignment: `override.first` isn't an input + = `override.first` already has a value and thus can't be assigned without `--override`. + = If you really want to override this field, please use `--override 'override.first="test"'` instead. - tip: a similar argument exists: '--override' - -Usage: customize-mode --input.foo.bar --input.foo.baz --override - -For more information, try '--help'. diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap index 62c8142eef..447cc68749 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap @@ -3,6 +3,6 @@ source: cli/tests/snapshot/main.rs expression: err --- error: invalid override: unknown field `unkonwn.field.path` - = `unkonwn.field.path` doesn't refer to record field accessible from the root of the configuration and thus can't be the target of `--override`. + = `unkonwn.field.path` doesn't refer to record field accessible from the root of the configuration. diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unkonwn_field_path.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unkonwn_field_path.ncl.snap new file mode 100644 index 0000000000..cb2b88ab5e --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unkonwn_field_path.ncl.snap @@ -0,0 +1,8 @@ +--- +source: cli/tests/snapshot/main.rs +expression: err +--- +error: invalid query: unknown field `unkonwn.field.path` + = `unkonwn.field.path` doesn't refer to record field accessible from the root of the configuration. + + diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_has_help.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_has_help.ncl.snap deleted file mode 100644 index feaf50259b..0000000000 --- a/cli/tests/snapshot/snapshots/snapshot__export_stdout_has_help.ncl.snap +++ /dev/null @@ -1,28 +0,0 @@ ---- -source: cli/tests/snapshot/main.rs -expression: out ---- -Customize a Nickel configuration through the command line before exporting - -Usage: customize-mode [OPTIONS] --input - -Options: - --input - - - --override - Override any field of the configuration with a valid Nickel expression provided as a - string. The new value will be merged with the configuration with a `force` priority. - - Overridable fields: - - output - - -h, --help - Print help (see a summary with '-h') - -[CONFLICT] This configuration has a field named `help` which conflicts with the built-in `--help` -argument. To set this field to e.g. "some_value", use `--override help "some_value"` instead of -`--help "some_value"` - -[WARNING] Customize mode is experimental. Its interface is subject to breaking changes. - diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_has_override.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_has_override.ncl.snap deleted file mode 100644 index 78ef119172..0000000000 --- a/cli/tests/snapshot/snapshots/snapshot__export_stdout_has_override.ncl.snap +++ /dev/null @@ -1,28 +0,0 @@ ---- -source: cli/tests/snapshot/main.rs -expression: out ---- -Customize a Nickel configuration through the command line before exporting - -Usage: customize-mode [OPTIONS] --input - -Options: - --input - - - --override - Override any field of the configuration with a valid Nickel expression provided as a - string. The new value will be merged with the configuration with a `force` priority. - - Overridable fields: - - output - - -h, --help - Print help (see a summary with '-h') - -[CONFLICT] This configuration has a field named `override` which conflicts with the built-in -`--override` argument. To set this field to e.g. "some_value", use `--override override -"some_value"` instead of `--override "some_value"` - -[WARNING] Customize mode is experimental. Its interface is subject to breaking changes. - diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap index 34a3453b05..eb93a77454 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap @@ -2,28 +2,41 @@ source: cli/tests/snapshot/main.rs expression: out --- -Customize a Nickel configuration through the command line before exporting +Customize a Nickel configuration through the command line -Usage: customize-mode [OPTIONS] --input.foo.bar --input.foo.baz +Usage: customize-mode [OPTIONS] [FIELD_PATH=NICKEL_EXPRESSION]... [COMMAND] -Options: - --input.defaulted.subfield - • documentation: Some documentation - [default: 2] - --input.foo.bar +Commands: + show Show the documentation of a particular field + list List the input fields and the overridable fields of the configuration + help Print this message or the help of the given subcommand(s) + +Arguments: + [FIELD_PATH=NICKEL_EXPRESSION]... + Assign a valid Nickel expression to an input field of the configuration. The new value + will be merged with the configuration with priority 0 (the one assigned by default in + Nickel when no explicit merge priority is provided). - --input.foo.baz + Assignment can only set input fields, that is fields without definition or fields with a + default value. To override an existing value, use `--override` instead. - --override - Override any field of the configuration with a valid Nickel expression provided as a - string. The new value will be merged with the configuration with a `force` priority. + Note that you might have to escape special characters or enclose assignments in quotes to + prevent shell interpretation. - Overridable fields: - - override.first - - override.second.subsecond.other - - override.second.subsecond.subsubsecond + Example: `nickel eval config.ncl -- 'http.enabled="yes"' protocol=\'ftp` + +Options: + --override + Override any field of the configuration with a valid Nickel expression. The new value will + be merged with the configuration with a `force` priority. + + Note that you might have to escape special characters or enclose assignments in quotes to + prevent shell interpretation. + + Example: `-- input.value=false --override m.count=2 --override m.type=\'server` + -h, --help - Print help + Print help (see a summary with '-h') [WARNING] Customize mode is experimental. Its interface is subject to breaking changes. diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap new file mode 100644 index 0000000000..341ec7249c --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap @@ -0,0 +1,14 @@ +--- +source: cli/tests/snapshot/main.rs +expression: out +--- +Input fields: +- input.defaulted.subfield +- input.foo.bar: +- input.foo.baz: + +Overridable fields (require `--override`): +- override.first +- override.second.subsecond.other +- override.second.subsecond.subsubsecond + diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap new file mode 100644 index 0000000000..161059b0cb --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap @@ -0,0 +1,9 @@ +--- +source: cli/tests/snapshot/main.rs +expression: out +--- +• contract: std.number.Integer +• default: 2 +• documentation: Some documentation + + From 92d08ff5fdc7d097fd5a8ee4a48b7e07722356e9 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 7 Nov 2023 09:00:25 +0100 Subject: [PATCH 10/19] Reword documentation of FieldInterface::is_input --- cli/src/customize/interface.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index 91a70a2628..7ac517452c 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -168,13 +168,12 @@ impl From<&RecordRowF<&Type>> for FieldInterface { } impl FieldInterface { - /// Define if a field is an input of a configuration that is intended to be filled, and will be - /// given a dedicated CLI argument. If the field is not an input, it can only be overridden via - /// `--override`. + /// Define if a field is an input of a configuration that is intended to be filled. If the + /// field is not an input, it can only be overridden via `--override`. /// /// Currently, the difference is mostly conceptual: in practice, we simply gather the arguments - /// and their values, either via direct arguments or `--override` (although the latter sets a - /// different priority), and then merge the elaborated record with original term. + /// and their values, either via direct positional arguments or `--override` (although the + /// latter sets a different priority), and then merge the elaborated record with original term. /// /// However, from a user experience point of view, we want to make a clear distinction between /// filling a bespoke input of a configuration and overriding an existing value. The latter is @@ -182,7 +181,7 @@ impl FieldInterface { /// /// Currently, the logic is simply that a field is an input if it doesn't have a definition or /// it has a default priority. This definition might evolve, as there are ongoing discussions - /// on what is should be the meaning of "input", "output", and if those concept should be made + /// on what is the meaning of "input", "output", and if those concept should be made /// first-class ([related issue](https://github.com/tweag/nickel/issues/1505)). For now, this /// logic seems to be a reasonable first approximation. pub(super) fn is_input(&self) -> bool { From bffa054cf6c831323f668b0c3f0843690c9f3579 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 7 Nov 2023 09:13:39 +0100 Subject: [PATCH 11/19] Pass on code documentation --- cli/src/customize/mod.rs | 2 +- cli/src/main.rs | 2 +- core/src/parser/grammar.lalrpop | 20 +++++++++++--------- core/src/program.rs | 11 +++++++---- 4 files changed, 20 insertions(+), 15 deletions(-) diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index 39f2af25c7..03f0a41974 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -155,7 +155,7 @@ impl DoCommand {} #[derive(clap::Parser)] struct ShowCommand { - /// Print the documentation and metadata of a particular overridable field. + /// Print the documentation and metadata of a particular field. #[clap(value_name = "FIELD_PATH")] field: String, } diff --git a/cli/src/main.rs b/cli/src/main.rs index bff75e0dc4..ff668c1b84 100644 --- a/cli/src/main.rs +++ b/cli/src/main.rs @@ -57,7 +57,7 @@ fn main() -> ExitCode { } match result { - // CustomizeInfoPrinted is used for early return, but it's not actually an error for the + // CustomizeInfoPrinted is used for early return, but it's not actually an error from the // user's point of view. Ok(()) | Err(error::Error::CustomizeInfoPrinted) => ExitCode::SUCCESS, Err(error) => { diff --git a/core/src/parser/grammar.lalrpop b/core/src/parser/grammar.lalrpop index 2134e25e56..6c8bf160cb 100644 --- a/core/src/parser/grammar.lalrpop +++ b/core/src/parser/grammar.lalrpop @@ -472,7 +472,7 @@ pub StaticFieldPath: Vec = = rule produces a -// RichTerm anyway, so we just return it, instead of artificially deconstructing -// it. +// RichTerm anyway, so it's simpler to just return it instead of artificially +// deconstructing it. // -// This construct is only for the CLI, and isn't currently part of the grammar +// This rule is currently only used for the CLI and isn't part of the grammar // for normal Nickel source code. pub CliFieldAssignment: (Vec, RichTerm, RawSpan) = "=" > diff --git a/core/src/program.rs b/core/src/program.rs index 807ca63635..4bbe4f6f55 100644 --- a/core/src/program.rs +++ b/core/src/program.rs @@ -124,10 +124,13 @@ impl FieldOverride { /// Parse an assignment `path.to.field=value` to a field override, with the priority given as a /// separate argument. /// - /// The parser actually entirely parse the `value` part to a [crate::term::RichTerm] (have it - /// accept anything after the equal sign is harder than actually parsing it), but what we need - /// at this point is just a string. Thus, `parse` uses the span to extract back the `value` part - /// of the input string. + /// Internally, the parser entirely parses the `value` part to a [crate::term::RichTerm] (have + /// it accept anything after the equal sign is in fact harder than actually parsing it), but + /// what we need at this point is just a string. Thus, `parse` uses the span to extract back + /// the `value` part of the input string. + /// + /// Theoretically, this means we parse two times the same string (the value part of an + /// assignment). In practice, we expect this cost to be completly neglectible. pub fn parse( cache: &mut Cache, assignment: String, From 21080c2bdfe63e010066d9b137509288120b5687 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 14 Nov 2023 22:28:37 +0100 Subject: [PATCH 12/19] Remove obsolete comment --- cli/src/customize/interface.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index 7ac517452c..2176189b02 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -9,7 +9,6 @@ use super::*; /// `customize_mode` option. #[derive(Debug, Clone, Default)] pub(super) struct TermInterface { - // We use a BTreeMap so that the end result is being sorted as we build the interface. pub(super) fields: HashMap, } From 09e8a4fbcd305ac8b0586f6489289a2df72acb27 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 14 Nov 2023 22:31:34 +0100 Subject: [PATCH 13/19] Update cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl Co-authored-by: jneem --- cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl b/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl index 016f3f830b..329f547421 100644 --- a/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/unknown_override.ncl @@ -5,7 +5,7 @@ # 'input.foo.bar="hello"', # 'input.foo.baz=[]', # '--override', -# 'unkonwn.field.path=null', +# 'unknown.field.path=null', # ] { input.foo.bar | String, From 123e6ade617c327f4006c00518c0cfeb8b4c1818 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 14 Nov 2023 22:31:45 +0100 Subject: [PATCH 14/19] Update cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl Co-authored-by: jneem --- cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl b/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl index 2d4671d249..fbf86775df 100644 --- a/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl @@ -3,7 +3,7 @@ # extra_args = [ # '--', # 'show', -# 'unkonwn.field.path', +# 'unknown.field.path', # ] { input.foo.bar | String, From 6d772c62c589bf5406f3961f2bc1110b2dd5fdd7 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 14 Nov 2023 22:32:08 +0100 Subject: [PATCH 15/19] Update cli/src/customize/interface.rs Co-authored-by: jneem --- cli/src/customize/interface.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cli/src/customize/interface.rs b/cli/src/customize/interface.rs index 2176189b02..54eeb6c6dc 100644 --- a/cli/src/customize/interface.rs +++ b/cli/src/customize/interface.rs @@ -12,7 +12,7 @@ pub(super) struct TermInterface { pub(super) fields: HashMap, } -/// The interface of a specific field. This fields can be itself a record and contain subfields. +/// The interface of a specific field. This field can be itself a record and contain subfields. #[derive(Debug, Clone, Default)] pub(super) struct FieldInterface { /// The interface of the subfields of this field, if it's a record itself. From ce6e61674f3e93959aaa968a7184bcadfac26701 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Tue, 14 Nov 2023 23:02:12 +0100 Subject: [PATCH 16/19] Fix typos in snapshot tests --- .../{unkonwn_field_path.ncl => unknown_field_path.ncl} | 0 .../snapshot__export_stderr_unknown_field_path.ncl.snap | 8 ++++++++ .../snapshot__export_stderr_unknown_override.ncl.snap | 4 ++-- 3 files changed, 10 insertions(+), 2 deletions(-) rename cli/tests/snapshot/inputs/customize-mode/{unkonwn_field_path.ncl => unknown_field_path.ncl} (100%) create mode 100644 cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap diff --git a/cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl b/cli/tests/snapshot/inputs/customize-mode/unknown_field_path.ncl similarity index 100% rename from cli/tests/snapshot/inputs/customize-mode/unkonwn_field_path.ncl rename to cli/tests/snapshot/inputs/customize-mode/unknown_field_path.ncl diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap new file mode 100644 index 0000000000..a9f829640d --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap @@ -0,0 +1,8 @@ +--- +source: cli/tests/snapshot/main.rs +expression: err +--- +error: invalid query: unknown field `unknown.field.path` + = `unknown.field.path` doesn't refer to record field accessible from the root of the configuration. + + diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap index 447cc68749..0aff5b6df4 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap @@ -2,7 +2,7 @@ source: cli/tests/snapshot/main.rs expression: err --- -error: invalid override: unknown field `unkonwn.field.path` - = `unkonwn.field.path` doesn't refer to record field accessible from the root of the configuration. +error: invalid override: unknown field `unknown.field.path` + = `unknown.field.path` doesn't refer to record field accessible from the root of the configuration. From cc80c989d474bd93607bcfad88327c3ac1b38429 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 15 Nov 2023 15:50:38 +0100 Subject: [PATCH 17/19] Add suggestion for customize mode unknown field errors --- cli/src/customize/mod.rs | 50 ++++++++++++++++++++++---------- cli/src/error.rs | 60 ++++++++++++++++++++++++++++++--------- core/src/error/mod.rs | 5 +--- core/src/error/suggest.rs | 13 +++++++++ 4 files changed, 95 insertions(+), 33 deletions(-) diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index 03f0a41974..8c0c89ec3c 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -19,7 +19,7 @@ use nickel_lang_core::{ typ::{RecordRowF, RecordRowsIteratorItem, Type, TypeF}, }; -use crate::error::CliResult; +use crate::error::{CliResult, CliUsageError, Error, UnknownFieldData}; pub mod interface; @@ -169,8 +169,8 @@ impl ShowCommand { let path = match program.parse_field_path(self.field) { Ok(path) => path, Err(parse_error) => { - return CliResult::Err(crate::error::Error::CliUsage { - error: crate::error::CliUsageError::FieldPathParseError { error: parse_error }, + return CliResult::Err(Error::CliUsage { + error: CliUsageError::FieldPathParseError { error: parse_error }, program, }) } @@ -183,8 +183,11 @@ impl ShowCommand { { Some(field) => &field.field, None => { - return CliResult::Err(crate::error::Error::CliUsage { - error: crate::error::CliUsageError::UnknownField { path }, + return CliResult::Err(Error::CliUsage { + error: CliUsageError::UnknownField(UnknownFieldData { + path, + field_list: customizable_fields.field_list(), + }), program, }) } @@ -255,6 +258,17 @@ impl CustomizableFields { } } } + + /// Return a vector of all the paths of the input fields and the overridable fields. In case of + /// a unknown field error, this list is used to suggest similar-looking field paths that the + /// user could have meant instead. + fn field_list(&self) -> Vec { + self.inputs + .keys() + .chain(self.overrides.keys()) + .cloned() + .collect() + } } pub trait Customize { @@ -277,9 +291,12 @@ impl CustomizeOptions { if !customizable_fields.inputs.contains_key(&ovd.path) { if customizable_fields.overrides.contains_key(&ovd.path) { - Err(super::error::CliUsageError::CantAssignNonInput { ovd }) + Err(CliUsageError::CantAssignNonInput { ovd }) } else { - Err(super::error::CliUsageError::UnknownFieldAssignment { path: ovd.path }) + Err(CliUsageError::UnknownFieldAssignment(UnknownFieldData { + path: ovd.path, + field_list: customizable_fields.field_list(), + })) } } else { Ok(ovd) @@ -289,21 +306,24 @@ impl CustomizeOptions { let assignment_overrides = match assignment_overrides { Ok(assignment_overrides) => assignment_overrides, - Err(error) => return CliResult::Err(crate::error::Error::CliUsage { error, program }), + Err(error) => return CliResult::Err(Error::CliUsage { error, program }), }; - let force_overrides: Result, super::error::CliUsageError> = self + let force_overrides: Result, CliUsageError> = self .ovd .into_iter() .map(|assignment| { let ovd = program .parse_override(assignment, MergePriority::Top) - .map_err(|error| super::error::CliUsageError::AssignmentParseError { error })?; + .map_err(|error| CliUsageError::AssignmentParseError { error })?; if !customizable_fields.inputs.contains_key(&ovd.path) && !customizable_fields.overrides.contains_key(&ovd.path) { - Err(super::error::CliUsageError::UnknownFieldOverride { path: ovd.path }) + Err(CliUsageError::UnknownFieldOverride(UnknownFieldData { + path: ovd.path, + field_list: customizable_fields.field_list(), + })) } else { Ok(ovd) } @@ -312,7 +332,7 @@ impl CustomizeOptions { let force_overrides = match force_overrides { Ok(force_overrides) => force_overrides, - Err(error) => return CliResult::Err(crate::error::Error::CliUsage { error, program }), + Err(error) => return CliResult::Err(Error::CliUsage { error, program }), }; program.add_overrides(assignment_overrides.into_iter().chain(force_overrides)); @@ -332,7 +352,7 @@ impl Customize for CustomizeMode { let evaled = match program.eval_record_spine() { Ok(evaled) => evaled, // We need a `return` control-flow to be able to take `program` out - Err(error) => return CliResult::Err(crate::error::Error::Program { error, program }), + Err(error) => return CliResult::Err(Error::Program { error, program }), }; let customizable_fields = CustomizableFields::new(TermInterface::from(evaled.as_ref())); @@ -342,11 +362,11 @@ impl Customize for CustomizeMode { None => opts.do_customize(customizable_fields, program), Some(CustomizeCommand::List(list_command)) => { list_command.run(&customizable_fields)?; - Err(crate::error::Error::CustomizeInfoPrinted) + Err(Error::CustomizeInfoPrinted) } Some(CustomizeCommand::Show(show_command)) => { show_command.run(program, &customizable_fields)?; - Err(crate::error::Error::CustomizeInfoPrinted) + Err(Error::CustomizeInfoPrinted) } } } diff --git a/cli/src/error.rs b/cli/src/error.rs index 08975f5b63..2a019ccce7 100644 --- a/cli/src/error.rs +++ b/cli/src/error.rs @@ -1,18 +1,27 @@ +//! Error handling for the CLI. + use nickel_lang_core::{ error::{Diagnostic, FileId, Files, IntoDiagnostics, ParseError}, eval::cache::lazy::CBNCache, - program::Program, - program::{FieldOverride, FieldPath}, + program::{FieldOverride, FieldPath, Program}, }; +/// Data about an unknown field error. +pub struct UnknownFieldData { + /// The field that was unknown. + pub path: FieldPath, + /// The list of customizable fields used to suggest similar alternative fields. + pub field_list: Vec, +} + /// Errors related to mishandling the CLI. pub enum CliUsageError { /// Tried to override a field which doesn't exist. - UnknownFieldOverride { path: FieldPath }, + UnknownFieldOverride(UnknownFieldData), /// Tried to assign a field which doesn't exist. - UnknownFieldAssignment { path: FieldPath }, + UnknownFieldAssignment(UnknownFieldData), /// Tried to show information about a field which doesn't exist. - UnknownField { path: FieldPath }, + UnknownField(UnknownFieldData), /// Tried to override an defined field without the `--override` argument. CantAssignNonInput { ovd: FieldOverride }, /// A parse error occurred when trying to parse an assignment. @@ -57,19 +66,42 @@ impl IntoDiagnostics for CliUsageError { files: &mut Files, stdlib_ids: Option<&Vec>, ) -> Vec> { - fn mk_unknown_diags(path: &FieldPath, method: &str) -> Vec> { + fn mk_unknown_diags( + data: UnknownFieldData, + method: &str, + ) -> Vec> { + let mut notes = vec![format!( + "`{path}` doesn't refer to a record field accessible from the root of the \ + configuration.", + path = data.path + )]; + + let fields_as_strings: Vec = + data.field_list.iter().map(ToString::to_string).collect(); + + nickel_lang_core::error::suggest::add_suggestion( + &mut notes, + &fields_as_strings, + &data.path.to_string(), + ); + + notes.push( + "Use `nickel [OPTIONS] -- list` to show a list of available fields." + .to_owned(), + ); + vec![Diagnostic::error() - .with_message(format!("invalid {method}: unknown field `{path}`")) - .with_notes(vec![format!( - "`{path}` doesn't refer to record field accessible from the root of the \ - configuration." - )])] + .with_message(format!( + "invalid {method}: unknown field `{path}`", + path = data.path + )) + .with_notes(notes)] } match self { - CliUsageError::UnknownFieldOverride { path } => mk_unknown_diags(&path, "override"), - CliUsageError::UnknownFieldAssignment { path } => mk_unknown_diags(&path, "assignment"), - CliUsageError::UnknownField { path } => mk_unknown_diags(&path, "query"), + CliUsageError::UnknownFieldOverride(data) => mk_unknown_diags(data, "override"), + CliUsageError::UnknownFieldAssignment(data) => mk_unknown_diags(data, "assignment"), + CliUsageError::UnknownField(data) => mk_unknown_diags(data, "query"), CliUsageError::CantAssignNonInput { ovd: FieldOverride { path, value, .. }, } => { diff --git a/core/src/error/mod.rs b/core/src/error/mod.rs index 3d01994cf0..bfe72d2bc3 100644 --- a/core/src/error/mod.rs +++ b/core/src/error/mod.rs @@ -1077,7 +1077,6 @@ impl IntoDiagnostics for EvalError { } => { let mut labels = Vec::new(); let mut notes = Vec::new(); - let suggestion = suggest::find_best_match(&field_names, &name); let field = escape(name.as_ref()); if let Some(span) = pos_op.into_opt() { @@ -1098,9 +1097,7 @@ impl IntoDiagnostics for EvalError { ); } - if let Some(suggestion) = suggestion { - notes.push(format!("Did you mean `{}`?", suggestion)); - } + suggest::add_suggestion(&mut notes, &field_names, &name); vec![Diagnostic::error() .with_message(format!("missing field `{field}`")) diff --git a/core/src/error/suggest.rs b/core/src/error/suggest.rs index 6673b97b37..900f095865 100644 --- a/core/src/error/suggest.rs +++ b/core/src/error/suggest.rs @@ -47,3 +47,16 @@ where arg_max } + +/// Take diagnostic notes and add a generic note suggesting existing symbols that are similar to +/// what the user typed thanks to [find_best_match], if any. If there is no match, `notes` is left +/// unchanged. +pub fn add_suggestion(notes: &mut Vec, symbols: &[S], user_input: &I) +where + S: AsRef, + I: AsRef, +{ + if let Some(best_match) = find_best_match(symbols, user_input) { + notes.push(format!("Did you mean `{best_match}`?")); + } +} From f0b8179c7216b83d14723875ee26efd27b8b1564 Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 15 Nov 2023 18:56:27 +0100 Subject: [PATCH 18/19] Update snapshot tests with new output --- .../snapshot__export_stderr_unknown_field_path.ncl.snap | 3 ++- .../snapshot__export_stderr_unknown_override.ncl.snap | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap index a9f829640d..2a35162a75 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_field_path.ncl.snap @@ -3,6 +3,7 @@ source: cli/tests/snapshot/main.rs expression: err --- error: invalid query: unknown field `unknown.field.path` - = `unknown.field.path` doesn't refer to record field accessible from the root of the configuration. + = `unknown.field.path` doesn't refer to a record field accessible from the root of the configuration. + = Use `nickel [OPTIONS] -- list` to show a list of available fields. diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap index 0aff5b6df4..c502ff83ea 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stderr_unknown_override.ncl.snap @@ -3,6 +3,7 @@ source: cli/tests/snapshot/main.rs expression: err --- error: invalid override: unknown field `unknown.field.path` - = `unknown.field.path` doesn't refer to record field accessible from the root of the configuration. + = `unknown.field.path` doesn't refer to a record field accessible from the root of the configuration. + = Use `nickel [OPTIONS] -- list` to show a list of available fields. From 2ab0124e823959d5960e02c08f32b176791f756c Mon Sep 17 00:00:00 2001 From: Yann Hamdaoui Date: Wed, 15 Nov 2023 23:33:00 +0100 Subject: [PATCH 19/19] Get rid of '-- show', hint at using query instead --- cli/src/customize/mod.rs | 57 ++----------------- cli/src/error.rs | 1 + .../field_path_syntax_error.ncl | 5 +- .../snapshot/inputs/customize-mode/show.ncl | 4 +- .../customize-mode/unknown_field_path.ncl | 5 +- .../snapshot__export_stdout_help.ncl.snap | 1 - .../snapshot__export_stdout_list.ncl.snap | 2 + .../snapshot__export_stdout_show.ncl.snap | 4 -- ...ry_stderr_field_path_syntax_error.ncl.snap | 11 ++++ ...__query_stderr_unknown_field_path.ncl.snap | 22 +++++++ .../snapshot__query_stdout_show.ncl.snap | 9 +++ 11 files changed, 55 insertions(+), 66 deletions(-) create mode 100644 cli/tests/snapshot/snapshots/snapshot__query_stderr_field_path_syntax_error.ncl.snap create mode 100644 cli/tests/snapshot/snapshots/snapshot__query_stderr_unknown_field_path.ncl.snap create mode 100644 cli/tests/snapshot/snapshots/snapshot__query_stdout_show.ncl.snap diff --git a/cli/src/customize/mod.rs b/cli/src/customize/mod.rs index 8c0c89ec3c..b0c628ed1a 100644 --- a/cli/src/customize/mod.rs +++ b/cli/src/customize/mod.rs @@ -11,7 +11,6 @@ use nickel_lang_core::{ eval::cache::lazy::CBNCache, identifier::LocIdent, program::{FieldPath, Program}, - repl::query_print, term::{ record::{Field, RecordData}, LabeledType, MergePriority, RuntimeContract, Term, @@ -81,8 +80,6 @@ struct CustomizeOptions { #[derive(clap::Subcommand)] enum CustomizeCommand { - /// Show the documentation of a particular field. - Show(ShowCommand), /// List the input fields and the overridable fields of the configuration. List(ListCommand), } @@ -130,6 +127,10 @@ impl ListCommand { println!("- {}", override_); } + println!( + "\nUse the `query` subcommand to print a detailed description of a specific field. \ + See `nickel help query`." + ); Ok(()) } } @@ -153,52 +154,6 @@ struct DoCommand { impl DoCommand {} -#[derive(clap::Parser)] -struct ShowCommand { - /// Print the documentation and metadata of a particular field. - #[clap(value_name = "FIELD_PATH")] - field: String, -} - -impl ShowCommand { - fn run( - self, - mut program: Program, - customizable_fields: &CustomizableFields, - ) -> CliResult<()> { - let path = match program.parse_field_path(self.field) { - Ok(path) => path, - Err(parse_error) => { - return CliResult::Err(Error::CliUsage { - error: CliUsageError::FieldPathParseError { error: parse_error }, - program, - }) - } - }; - - let field = match customizable_fields - .inputs - .get(&path) - .or(customizable_fields.overrides.get(&path)) - { - Some(field) => &field.field, - None => { - return CliResult::Err(Error::CliUsage { - error: CliUsageError::UnknownField(UnknownFieldData { - path, - field_list: customizable_fields.field_list(), - }), - program, - }) - } - }; - - query_print::write_query_result(&mut std::io::stdout(), field, Default::default()).unwrap(); - - Ok(()) - } -} - /// Fields of the configuration which aren't themselves records and can be assigned or overridden /// through the customize mode. #[derive(Clone, Debug, Default)] @@ -364,10 +319,6 @@ impl Customize for CustomizeMode { list_command.run(&customizable_fields)?; Err(Error::CustomizeInfoPrinted) } - Some(CustomizeCommand::Show(show_command)) => { - show_command.run(program, &customizable_fields)?; - Err(Error::CustomizeInfoPrinted) - } } } } diff --git a/cli/src/error.rs b/cli/src/error.rs index 2a019ccce7..edd89ce1ea 100644 --- a/cli/src/error.rs +++ b/cli/src/error.rs @@ -15,6 +15,7 @@ pub struct UnknownFieldData { } /// Errors related to mishandling the CLI. +#[allow(dead_code)] pub enum CliUsageError { /// Tried to override a field which doesn't exist. UnknownFieldOverride(UnknownFieldData), diff --git a/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl b/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl index 50fdfc5aa8..9d791c1837 100644 --- a/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/field_path_syntax_error.ncl @@ -1,8 +1,7 @@ # capture = 'stderr' -# command = ['export'] +# command = ['query'] # extra_args = [ -# '--', -# 'show', +# '--path', # 'input.+foo.baz', # ] { diff --git a/cli/tests/snapshot/inputs/customize-mode/show.ncl b/cli/tests/snapshot/inputs/customize-mode/show.ncl index 7fcda1dc7c..4c59a4d59f 100644 --- a/cli/tests/snapshot/inputs/customize-mode/show.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/show.ncl @@ -1,6 +1,6 @@ # capture = 'stdout' -# command = ['export'] -# extra_args = ['--', 'show', 'input.defaulted.subfield'] +# command = ['query'] +# extra_args = ['--path', 'input.defaulted.subfield'] { input.foo.bar | String, input.foo.baz | Array Number, diff --git a/cli/tests/snapshot/inputs/customize-mode/unknown_field_path.ncl b/cli/tests/snapshot/inputs/customize-mode/unknown_field_path.ncl index fbf86775df..2843ee3baa 100644 --- a/cli/tests/snapshot/inputs/customize-mode/unknown_field_path.ncl +++ b/cli/tests/snapshot/inputs/customize-mode/unknown_field_path.ncl @@ -1,8 +1,7 @@ # capture = 'stderr' -# command = ['export'] +# command = ['query'] # extra_args = [ -# '--', -# 'show', +# '--path', # 'unknown.field.path', # ] { diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap index eb93a77454..5aa1e9903c 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stdout_help.ncl.snap @@ -7,7 +7,6 @@ Customize a Nickel configuration through the command line Usage: customize-mode [OPTIONS] [FIELD_PATH=NICKEL_EXPRESSION]... [COMMAND] Commands: - show Show the documentation of a particular field list List the input fields and the overridable fields of the configuration help Print this message or the help of the given subcommand(s) diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap index 341ec7249c..53b9023cdf 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stdout_list.ncl.snap @@ -12,3 +12,5 @@ Overridable fields (require `--override`): - override.second.subsecond.other - override.second.subsecond.subsubsecond +Use the `query` subcommand to print a detailed description of a specific field. See `nickel help query`. + diff --git a/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap index 161059b0cb..9cea209197 100644 --- a/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap +++ b/cli/tests/snapshot/snapshots/snapshot__export_stdout_show.ncl.snap @@ -2,8 +2,4 @@ source: cli/tests/snapshot/main.rs expression: out --- -• contract: std.number.Integer -• default: 2 -• documentation: Some documentation - diff --git a/cli/tests/snapshot/snapshots/snapshot__query_stderr_field_path_syntax_error.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__query_stderr_field_path_syntax_error.ncl.snap new file mode 100644 index 0000000000..a2a5471012 --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__query_stderr_field_path_syntax_error.ncl.snap @@ -0,0 +1,11 @@ +--- +source: cli/tests/snapshot/main.rs +expression: err +--- +error: unexpected token + ┌─ :1:7 + │ +1 │ input.+foo.baz + │ ^ + + diff --git a/cli/tests/snapshot/snapshots/snapshot__query_stderr_unknown_field_path.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__query_stderr_unknown_field_path.ncl.snap new file mode 100644 index 0000000000..8a9ff47f43 --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__query_stderr_unknown_field_path.ncl.snap @@ -0,0 +1,22 @@ +--- +source: cli/tests/snapshot/main.rs +expression: err +--- +error: missing field `unknown` + ┌─ :1:1 + │ + 1 │ unknown.field.path + │ ^^^^^^^ this requires the field `unknown` to exist + │ + ┌─ [INPUTS_PATH]/customize-mode/unknown_field_path.ncl:7:1 + │ + 7 │ ╭ { + 8 │ │ input.foo.bar | String, + 9 │ │ input.foo.baz | Array Number, +10 │ │ input.defaulted.subfield + · │ +20 │ │ } +21 │ │ } + │ ╰─' this record lacks the field `unknown` + + diff --git a/cli/tests/snapshot/snapshots/snapshot__query_stdout_show.ncl.snap b/cli/tests/snapshot/snapshots/snapshot__query_stdout_show.ncl.snap new file mode 100644 index 0000000000..161059b0cb --- /dev/null +++ b/cli/tests/snapshot/snapshots/snapshot__query_stdout_show.ncl.snap @@ -0,0 +1,9 @@ +--- +source: cli/tests/snapshot/main.rs +expression: out +--- +• contract: std.number.Integer +• default: 2 +• documentation: Some documentation + +