From 1ddae02da7a8b14be2f804e484117f8059ed0e0a Mon Sep 17 00:00:00 2001 From: Bruce Weirdan Date: Mon, 18 Aug 2025 13:34:57 +0200 Subject: [PATCH] Multiple output types for `query xml` (#16459) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is a breaking change. Refs https://discord.com/channels/601130461678272522/615329862395101194/1406413134402551869 ## Release notes summary - What our users need to know Previously, `query xml` always returned a table, even for scalar results. Now scalar results will be returned as scalars. For example ```nushell open -r tests/fixtures/formats/jt.xml | query xml 'false()' ``` used to return ``` ╭───┬─────────╮ │ # │ false() │ ├───┼─────────┤ │ 0 │ false │ ╰───┴─────────╯ ``` and now it will return just `false`. ## Tasks after submitting - [ ] Update the [documentation](https://github.com/nushell/nushell.github.io) --- crates/nu_plugin_query/src/query_xml.rs | 155 ++++++++++++++++-------- 1 file changed, 106 insertions(+), 49 deletions(-) diff --git a/crates/nu_plugin_query/src/query_xml.rs b/crates/nu_plugin_query/src/query_xml.rs index 11ed39299e..953720853d 100644 --- a/crates/nu_plugin_query/src/query_xml.rs +++ b/crates/nu_plugin_query/src/query_xml.rs @@ -84,43 +84,27 @@ pub fn execute_xpath_query( // build_namespaces(&arguments, &mut context); let res = xpath.evaluate(&context, document.root()); - // Some xpath statements can be long, so let's truncate it with ellipsis - let mut key = query_string.clone(); - if query_string.len() >= 20 { - key.truncate(17); - key += "..."; - } else { - key = query_string.to_string(); - }; - match res { - Ok(r) => { - let mut record = Record::new(); - let mut records: Vec = vec![]; - - match r { - sxd_xpath::Value::Nodeset(ns) => { - for n in ns.document_order() { - record.push(key.clone(), Value::string(n.string_value(), call.head)); - } - } - sxd_xpath::Value::Boolean(b) => { - record.push(key, Value::bool(b, call.head)); - } - sxd_xpath::Value::Number(n) => { - record.push(key, Value::float(n, call.head)); - } - sxd_xpath::Value::String(s) => { - record.push(key, Value::string(s, call.head)); - } + Ok(sxd_xpath::Value::Boolean(b)) => Ok(Value::bool(b, call.head)), + Ok(sxd_xpath::Value::Number(n)) => Ok(Value::float(n, call.head)), + Ok(sxd_xpath::Value::String(s)) => Ok(Value::string(s, call.head)), + Ok(sxd_xpath::Value::Nodeset(ns)) => { + // Some xpath statements can be long, so let's truncate it with ellipsis + let mut key = query_string.clone(); + if query_string.len() >= 20 { + key.truncate(17); + key += "..."; + } else { + key = query_string.to_string(); }; - // convert the cols and vecs to a table by creating individual records - // for each item so we can then use a list to make a table - for (k, v) in record { - records.push(Value::record(record! { k => v }, call.head)) + let mut records: Vec = vec![]; + for n in ns.document_order() { + records.push(Value::record( + record! {key.clone() => Value::string(n.string_value(), call.head)}, + call.head, + )); } - Ok(Value::list(records, call.head)) } Err(err) => { @@ -166,14 +150,7 @@ mod tests { let actual = query(&call, &text, Some(spanned_str), None).expect("test should not fail"); - let expected = Value::list( - vec![Value::test_record(record! { - "count(//a/*[posit..." => Value::test_float(1.0), - })], - Span::test_data(), - ); - - assert_eq!(actual, expected); + assert_eq!(actual, Value::test_float(1.0)); } #[test] @@ -196,14 +173,7 @@ mod tests { let actual = query(&call, &text, Some(spanned_str), None).expect("test should not fail"); - let expected = Value::list( - vec![Value::test_record(record! { - "count(//*[contain..." => Value::test_float(1.0), - })], - Span::test_data(), - ); - - assert_eq!(actual, expected); + assert_eq!(actual, Value::test_float(1.0)); } #[test] @@ -243,4 +213,91 @@ mod tests { // and yet it should work regardless assert_eq!(actual, expected); } + + #[test] + fn number_returns_float() { + let call = EvaluatedCall { + head: Span::test_data(), + positional: vec![], + named: vec![], + }; + + let text = Value::test_string(r#""#); + + let spanned_str: Spanned = Spanned { + item: "count(/elt)".to_string(), + span: Span::test_data(), + }; + + let actual = query(&call, &text, Some(spanned_str), None).expect("test should not fail"); + + assert_eq!(actual, Value::test_float(1.0)); + } + + #[test] + fn boolean_returns_bool() { + let call = EvaluatedCall { + head: Span::test_data(), + positional: vec![], + named: vec![], + }; + + let text = Value::test_string(r#""#); + + let spanned_str: Spanned = Spanned { + item: "false()".to_string(), + span: Span::test_data(), + }; + + let actual = query(&call, &text, Some(spanned_str), None).expect("test should not fail"); + + assert_eq!(actual, Value::test_bool(false)); + } + + #[test] + fn string_returns_string() { + let call = EvaluatedCall { + head: Span::test_data(), + positional: vec![], + named: vec![], + }; + + let text = Value::test_string(r#""#); + + let spanned_str: Spanned = Spanned { + item: "local-name(/elt)".to_string(), + span: Span::test_data(), + }; + + let actual = query(&call, &text, Some(spanned_str), None).expect("test should not fail"); + + assert_eq!(actual, Value::test_string("elt")); + } + + #[test] + fn nodeset_returns_table() { + let call = EvaluatedCall { + head: Span::test_data(), + positional: vec![], + named: vec![], + }; + + let text = Value::test_string(r#"hello"#); + + let spanned_str: Spanned = Spanned { + item: "/elt".to_string(), + span: Span::test_data(), + }; + + let actual = query(&call, &text, Some(spanned_str), None).expect("test should not fail"); + + let expected = Value::list( + vec![Value::test_record(record! { + "/elt" => Value::string("hello", Span::test_data()), + })], + Span::test_data(), + ); + + assert_eq!(actual, expected); + } }