Select::parse fails due to borrow the css query

[latot]

Hi, I has been using this library from a while, has been great.

Here a example:

fn a(document: &Html) -> Result<(), Box<dyn Error>> {
    let query = format!("div table thead tr:nth-child({}) th", 3);
    let selector = Selector::parse(&query)?;
    Ok(())
}

This function will fails, the reason is pretty simple, the function owns the query, but the error triggered by parse seems to have borrowed the string, so it can't return an error when the function owns the data.

The workaround right now is unwrap instead use error propagation.

There is some ways to handle this, one would be a parse with String which owns the data, other one is instead use a borrowed string for the error, convert &str to String and use that instead.

Thx!

[adamreichold]

You can continue to use error propagation, you just need to downgrade the to a string, e.g.

let selector = Selector::parse(&query).map_err(|err| err.to_string())?;

but note that for selectors known at compile-time, .unwrap() is somewhat reasonable as failing to parse such a selector is a logic error instead of a runtime error. (For your example of a dynamically generated selector, one would have to consider how much leeway there is, i.e. if you really only interpolate an u8 into the nth-child part, then .unwrap() might still be reasonable as calling code should be unable to produce an invalid selector by choosing the value of the u8.)

[latot]

Hi! thx for the answer.

Well, is not ideal change the error type for the error propagation, would be good be able to return the actual one.

unwrap, I think it would depends on the type of app you are constructing, if is a one that only you will use, is more reasonable to panic, just for a user end product we want to handle the error and do not to panic.

[adamreichold]

unwrap, I think it would depends on the type of app you are constructing, if is a one that only you will use, is more reasonable to panic, just for a user end product we want to handle the error and do not to panic.

This is not the distinction I am making: A CSS selector that is fixed when the program is built is not a runtime error but a construction error, i.e. a panic (that should never happen once the program is shipped) is entirely appropriate.

(Think of it this way, we could with sufficient effort create a proc macro which parses such a CSS selector during compilation and embeds some already validated form of it into the program so that no parsing at all needs to happen when it runs. Alas, just using .unwrap() is a convenient way to get almost the same result with very much less implementation effort.)

[latot]

I agree, this is because there is at least two types of CSS selectors.

I one hand, the ones that can be checked at compile time, here unwrap is good, and the best solution is a macro as you says, this could be a nice feature :)

The next one is more complex css selectors, where the string is created by some algorithm, which is impossible to be checked by at compile time, can only be checked at runtime, there the error must be propagated, ideally the original not, not a string for a good error handling.

[LoZack19]

The idea of a macro is indeed interesting, I will leave this issue open in order to remember it. @cfvescovo

[cfvescovo]

Take a look at #53

[cfvescovo]

Since we had more requests for a macro implementation, maybe it's now time to add it to scraper

[LoZack19]

#53 is interesting because it explores the idea of a proc-macro. I think that, if it worked, it could be a cleaner solution than what we have right now. In that discussion, though, concerns had been raised about the necessity of implementing such a macro

[cfvescovo]

#53 is interesting because it explores the idea of a proc-macro. I think that, if it worked, it could be a cleaner solution than what we have right now. In that discussion, though, concerns had been raised about the necessity of implementing such a macro

At the time, I chose to close #53 because another crate was actively being developed to address the issue. Now the repo of that crate is inactive.

I think we should provide an "official" implementation so that we can maintain and improve it over time. Moreover, it could be useful for beginners who don't want to roll out their own macro

[LoZack19]

This claude-generated solution looks very clean and idiomatic to me, even much better than the one provided in the crate scraper-proc-macro

use proc_macro::TokenStream;
use quote::quote;
use syn::{parse_macro_input, LitStr};

#[proc_macro]
pub fn selector(input: TokenStream) -> TokenStream {
    let selector = parse_macro_input!(input as LitStr).value();
    
    match scraper::Selector::parse(&selector) {
        Ok(_) => quote!(
            ::scraper::Selector::parse(#selector).unwrap()
        ).into(),
        Err(e) => syn::Error::new(
            proc_macro2::Span::call_site(),
            format!("Failed to parse CSS selector: {}", e)
        ).to_compile_error().into(),
    }
}

[LoZack19]

If integrated, would you put it inside the selector module or in a dedicated macro module? It could also be integrated as an additional macro feature

[teymour-aldridge]

If integrated, would you put it inside the selector module or in a dedicated macro module? It could also be integrated as an additional macro feature

I imagine it would sit behind a feature flag in a module? The implementation would probably be in a separate crate.

[LoZack19]

I was reading that a proc_macro needs to live necessarely in a separate crate, so yes, this is true, it would be quite invasive as an addition though, but yet useful