diff --git a/Cargo.lock b/Cargo.lock index b87b4ec242..7bb2fed708 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2617,6 +2617,7 @@ dependencies = [ "num 0.4.0", "num-format", "num-traits", + "once_cell", "pathdiff", "polars", "powierza-coefficient", @@ -3045,9 +3046,9 @@ checksum = "e7461858c5ac9bde3fcdeedc17f58ed0469ec74f2d737b6369fc31c0b0ef576c" [[package]] name = "once_cell" -version = "1.15.0" +version = "1.16.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "e82dad04139b71a90c080c8463fe0dc7902db5192d939bd0950f074d014339e1" +checksum = "86f0b0d4bf799edbc74508c1e8bf170ff5f41238e5f8225603ca7caaae2b7860" [[package]] name = "oorandom" diff --git a/crates/nu-command/Cargo.toml b/crates/nu-command/Cargo.toml index 1d91b7c9d9..2c00a5b619 100644 --- a/crates/nu-command/Cargo.toml +++ b/crates/nu-command/Cargo.toml @@ -61,6 +61,7 @@ mime = "0.3.16" notify = "4.0.17" num = { version = "0.4.0", optional = true } num-traits = "0.2.14" +once_cell = "1.0" pathdiff = "0.2.1" powierza-coefficient = "1.0.1" quick-xml = "0.23.0" diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 8a2cd70434..7c0cab61bb 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -85,6 +85,7 @@ impl Command for FromNuon { let mut error = None; + // Most of the 'work' in converting from Nuon is simply pushing it through the Nu parser. let (lexed, err) = nu_parser::lex(string_input.as_bytes(), 0, &[b'\n', b'\r'], &[], true); error = error.or(err); diff --git a/crates/nu-command/src/formats/to/nuon.rs b/crates/nu-command/src/formats/to/nuon.rs index a07ec34975..7c1f7fb994 100644 --- a/crates/nu-command/src/formats/to/nuon.rs +++ b/crates/nu-command/src/formats/to/nuon.rs @@ -1,4 +1,5 @@ use core::fmt::Write; +use fancy_regex::Regex; use nu_engine::get_columns; use nu_parser::escape_quote_string; use nu_protocol::ast::{Call, RangeInclusion}; @@ -6,6 +7,7 @@ use nu_protocol::engine::{Command, EngineState, Stack}; use nu_protocol::{ Category, Example, IntoPipelineData, PipelineData, ShellError, Signature, Span, Type, Value, }; +use once_cell::sync::Lazy; #[derive(Clone)] pub struct ToNuon; @@ -55,7 +57,7 @@ fn value_to_string(v: &Value, span: Span) -> Result { for byte in val { if write!(s, "{:02X}", byte).is_err() { return Err(ShellError::UnsupportedInput( - "binary could not translate to string".into(), + "could not convert binary to string".into(), span, )); } @@ -63,7 +65,7 @@ fn value_to_string(v: &Value, span: Span) -> Result { Ok(format!("0x[{}]", s)) } Value::Block { .. } => Err(ShellError::UnsupportedInput( - "block not supported".into(), + "blocks are currently not nuon-compatible".into(), span, )), Value::Closure { .. } => Err(ShellError::UnsupportedInput( @@ -78,21 +80,24 @@ fn value_to_string(v: &Value, span: Span) -> Result { } } Value::CellPath { .. } => Err(ShellError::UnsupportedInput( - "cellpath not supported".to_string(), + "cellpaths are currently not nuon-compatible".to_string(), span, )), Value::CustomValue { .. } => Err(ShellError::UnsupportedInput( - "custom not supported".to_string(), + "customs are currently not nuon-compatible".to_string(), span, )), Value::Date { val, .. } => Ok(val.to_rfc3339()), + // FIXME: make duratiobs use the shortest lossless representation. Value::Duration { val, .. } => Ok(format!("{}ns", *val)), Value::Error { .. } => Err(ShellError::UnsupportedInput( - "error not supported".to_string(), + "errors are currently not nuon-compatible".to_string(), span, )), + // FIXME: make filesizes use the shortest lossless representation. Value::Filesize { val, .. } => Ok(format!("{}b", *val)), Value::Float { val, .. } => { + // This serialises these as 'nan', 'inf' and '-inf', respectively. if &val.round() == val && val != &f64::NAN && val != &f64::INFINITY @@ -146,7 +151,7 @@ fn value_to_string(v: &Value, span: Span) -> Result { Ok(format!("[{}]", collection.join(", "))) } } - Value::Nothing { .. } => Ok("$nothing".to_string()), + Value::Nothing { .. } => Ok("null".to_string()), Value::Range { val, .. } => Ok(format!( "{}..{}{}", value_to_string(&val.from, span)?, @@ -172,6 +177,8 @@ fn value_to_string(v: &Value, span: Span) -> Result { } Ok(format!("{{{}}}", collection.join(", "))) } + // All strings outside data structures are quoted because they are in 'command position' + // (could be mistaken for commands by the Nu parser) Value::String { val, .. } => Ok(escape_quote_string(val)), } } @@ -195,26 +202,24 @@ fn to_nuon(call: &Call, input: PipelineData) -> Result { value_to_string(&v, call.head) } +// This hits, in order: +// • Any character of []:`{}#'";()|$, +// • Any digit (\d) +// • Any whitespace (\s) +// • Case-insensitive sign-insensitive float "keywords" inf, infinity and nan. +static NEEDS_QUOTES_REGEX: Lazy = Lazy::new(|| { + Regex::new(r#"[\[\]:`\{\}#'";\(\)\|\$,\d\s]|(?i)^[+\-]?(inf(inity)?|nan)$"#) + .expect("internal error: NEEDS_QUOTES_REGEX didn't compile") +}); + fn needs_quotes(string: &str) -> bool { - string.contains(' ') - || string.contains('[') - || string.contains(']') - || string.contains(':') - || string.contains('`') - || string.contains('{') - || string.contains('}') - || string.contains('#') - || string.contains('\'') - || string.contains(';') - || string.contains('(') - || string.contains(')') - || string.contains('|') - || string.contains('$') - || string.contains(',') - || string.contains('\t') - || string.contains('\n') - || string.contains('\r') - || string.contains('\"') + // These are case-sensitive keywords + match string { + "true" | "false" | "null" => return true, + _ => (), + }; + // All other cases are handled here + NEEDS_QUOTES_REGEX.is_match(string).unwrap_or(false) } #[cfg(test)] diff --git a/crates/nu-command/tests/format_conversions/html.rs b/crates/nu-command/tests/format_conversions/html.rs index 87b92a276a..f8fe618fb7 100644 --- a/crates/nu-command/tests/format_conversions/html.rs +++ b/crates/nu-command/tests/format_conversions/html.rs @@ -84,6 +84,6 @@ fn test_list() { ); assert_eq!( actual.out, - r##"{name: C64, black: "#090300", red: "#883932", green: "#55a049", yellow: "#bfce72", blue: "#40318d", purple: "#8b3f96", cyan: "#67b6bd", white: "#ffffff", brightBlack: "#000000", brightRed: "#883932", brightGreen: "#55a049", brightYellow: "#bfce72", brightBlue: "#40318d", brightPurple: "#8b3f96", brightCyan: "#67b6bd", brightWhite: "#f7f7f7", background: "#40318d", foreground: "#7869c4"}"## + r##"{name: "C64", black: "#090300", red: "#883932", green: "#55a049", yellow: "#bfce72", blue: "#40318d", purple: "#8b3f96", cyan: "#67b6bd", white: "#ffffff", brightBlack: "#000000", brightRed: "#883932", brightGreen: "#55a049", brightYellow: "#bfce72", brightBlue: "#40318d", brightPurple: "#8b3f96", brightCyan: "#67b6bd", brightWhite: "#f7f7f7", background: "#40318d", foreground: "#7869c4"}"## ); } diff --git a/crates/nu-command/tests/format_conversions/nuon.rs b/crates/nu-command/tests/format_conversions/nuon.rs index 94069acc2f..65322253aa 100644 --- a/crates/nu-command/tests/format_conversions/nuon.rs +++ b/crates/nu-command/tests/format_conversions/nuon.rs @@ -297,7 +297,7 @@ fn to_nuon_converts_columns_with_spaces() { } #[test] -fn to_nuon_does_not_quote_unnecessarily() { +fn does_not_quote_strings_unnecessarily() { let actual = nu!( cwd: "tests/fixtures/formats", pipeline( r#" @@ -314,6 +314,30 @@ fn to_nuon_does_not_quote_unnecessarily() { assert_eq!(actual.out, "{\"ro name\": sam, rank: 10}"); } +#[test] +fn quotes_some_strings_necessarily() { + let actual = nu!( + cwd: "tests/fixtures/formats", pipeline( + r#" + ['true','false','null', + 'NaN','NAN','nan','+nan','-nan', + 'inf','+inf','-inf','INF', + 'Infinity','+Infinity','-Infinity','INFINITY', + '+19.99','-19.99', '19.99b', + '19.99kb','19.99mb','19.99gb','19.99tb','19.99pb','19.99eb','19.99zb', + '19.99kib','19.99mib','19.99gib','19.99tib','19.99pib','19.99eib','19.99zib', + '19ns', '19us', '19ms', '19sec', '19min', '19hr', '19day', '19wk', + '-11.0..-15.0', '11.0..-15.0', '-11.0..15.0', + '-11.0..<-15.0', '11.0..<-15.0', '-11.0..<15.0', + '-11.0..', '11.0..', '..15.0', '..-15.0', '..<15.0', '..<-15.0', + '2000-01-01', '2022-02-02T14:30:00', '2022-02-02T14:30:00+05:00' + ] | to nuon | from nuon | describe + "# + )); + + assert_eq!(actual.out, "list"); +} + proptest! { #[test] fn to_nuon_from_nuon(c: char) {