query web --query should return list<list<string>> like the scraper crate's ElementRef::text() (#11705)

# Description  ## Problem I tried converting one of my Rust web scrapers to Nushell just to see how it would be done, but quickly ran into an issue that proved annoying to fix without diving into the source. For instance, let's say we have the following HTML ```html <p>Hello there, <span style="color: red;">World</span></p> ``` and we want to extract only the text within the `p` element, but not the `span`. With the current version of nu_plugin_query, if we run this code ```nushell echo `<p>Hello there, <span style="color: red;">World</span></p>` | query web -q "p" | get 0 # returns "Hello there, World" # but we want only "Hello there, " ``` we will get back a `list<string>` that contains 1 string `Hello there, World`. To avoid scraping the span, we would have to do something like this ```nushell const html = `<p>Hello there, <span style="color: red;">World</span></p>` $html | query web -q "p" | get 0 | str replace ($html | query web -q "p > span" | get 0) "" # returns "Hello there, " ``` In other words, we would have to make a sub scrape of the text we *don't* want in order to subtract it from the text we *do* want. ## Solution I didn't like this behavior, so I decided to change it. I modified the `execute_selector_query` function to collect all text nodes in the HTML element matching the query. Now `query web --query` will return a `list<list<string>>` ```nushell echo `<p>Hello there, <span style="color: red;">World</span></p>` | query web -q "p" | get 0 | to json --raw # returns ["Hello there, ","World"] ``` This also brings `query web --query`'s behavior more in line with [scraper's ElementRef::text()](https://docs.rs/scraper/latest/scraper/element_ref/struct.ElementRef.html#method.text) which "Returns an iterator over descendent text nodes", allowing you to choose how much of an element's text you want to scrape without resorting to string substitutions. ## Consequences As this is a user-facing change, the usage examples will produce different results than before. For example ```nushell http get https://phoronix.com | query web --query 'header' ``` will return a list of lists of 1 string each, whereas before it was just a list of strings. I only modified the 3rd example ```nushell # old http get https://www.nushell.sh | query web --query 'h2, h2 + p' | group 2 | each {rotate --ccw tagline description} | flatten # new http get https://www.nushell.sh | query web --query 'h2, h2 + p' | each {str join} | group 2 | each {rotate --ccw tagline description} | flatten ``` to make it behave like before because I thought this one ought to show the same results as before. However, the second reason I changed the 3rd example is because it otherwise panics! If we run the original 3rd example with my modifications, we get a panic ``` thread 'main' panicked at crates/nu-protocol/src/value/record.rs:34:9: assertion `left == right` failed left: 2 right: 17 ``` This happens because `rotate` receives a list of lists where the inner lists have a different number of elements. However this panic is unrelated to the changes I've made, because it can be triggered easily without using the plugin. For instance ```nushell # this is fine [[[one] [two]] [[three] [four]]] | each {rotate --ccw tagline description} # this panics! [[[one] [two]] [[three] [four five]]] | each {rotate --ccw tagline description} ``` Though beyond the scope of this PR, I thought I'd mention this bug since I found it while testing the usage examples. However, I intend to make a proper issue about it tomorrow. # User-Facing Changes  `query web --query "css selector"` now returns a `list<list<string>>` instead of a `list<string>` to make it more in line with [scraper's ElementRef::text()](https://docs.rs/scraper/latest/scraper/element_ref/struct.ElementRef.html#method.text). # Tests + Formatting  I ran `cargo fmt --all -- --check`, `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` and the tests in the plugin. # After Submitting  PR that updates the documentation to match the new 3rd example: https://github.com/nushell/nushell.github.io/pull/1235
2024-11-28 19:33:47 +01:00 · 2024-02-03 03:40:47 +02:00 · 2024-02-03 03:40:47 +02:00 · 0d518bf813
commit 0d518bf813
parent e083b75aef
2 changed files with 37 additions and 5 deletions
--- a/crates/nu_plugin_query/src/nu/mod.rs
+++ b/crates/nu_plugin_query/src/nu/mod.rs
@ -73,7 +73,7 @@ impl Plugin for Query {

 pub fn web_examples() -> Vec<PluginExample> {
    vec![PluginExample {
-        example: "http get https://phoronix.com | query web --query 'header'".into(),
+        example: "http get https://phoronix.com | query web --query 'header' | flatten".into(),
        description: "Retrieve all `<header>` elements from phoronix.com website".into(),
        result: None,
    }, PluginExample {
@ -83,7 +83,7 @@ pub fn web_examples() -> Vec<PluginExample> {
        result: None
    },
    PluginExample {
-        example: "http get https://www.nushell.sh | query web --query 'h2, h2 + p' | group 2 | each {rotate --ccw tagline description} | flatten".into(),
+        example: "http get https://www.nushell.sh | query web --query 'h2, h2 + p' | each {str join} | group 2 | each {rotate --ccw tagline description} | flatten".into(),
        description: "Pass multiple css selectors to extract several elements within single query, group the query results together and rotate them to create a table".into(),
        result: None,
    },
--- a/crates/nu_plugin_query/src/query_web.rs
+++ b/crates/nu_plugin_query/src/query_web.rs
@ -260,10 +260,11 @@ fn execute_selector_query(
        false => doc
            .select(&css(query_string, inspect))
            .map(|selection| {
-                Value::string(
+                Value::list(
                    selection
                        .text()
-                        .fold("".to_string(), |acc, x| format!("{acc}{x}")),
+                        .map(|text| Value::string(text, span))
+                        .collect(),
                    span,
                )
            })
@ -293,6 +294,8 @@ mod tests {
         </ul>
     "#;

+    const NESTED_TEXT: &str = r#"<p>Hello there, <span style="color: red;">World</span></p>"#;
+
    #[test]
    fn test_first_child_is_not_empty() {
        assert!(!execute_selector_query(
@ -316,6 +319,35 @@ mod tests {
        );
        let config = nu_protocol::Config::default();
        let out = item.into_string("\n", &config);
-        assert_eq!("[Coffee]".to_string(), out)
+        assert_eq!("[[Coffee]]".to_string(), out)
+    }
+
+    #[test]
+    fn test_nested_text_nodes() {
+        let item = execute_selector_query(
+            NESTED_TEXT,
+            "p:first-child",
+            false,
+            false,
+            Span::test_data(),
+        );
+        let out = item
+            .as_list()
+            .unwrap()
+            .iter()
+            .map(|matches| {
+                matches
+                    .as_list()
+                    .unwrap()
+                    .iter()
+                    .map(|text_nodes| text_nodes.as_string().unwrap())
+                    .collect::<Vec<String>>()
+            })
+            .collect::<Vec<Vec<String>>>();
+
+        assert_eq!(
+            out,
+            vec![vec!["Hello there, ".to_string(), "World".to_string()]],
+        );
    }
 }