fefit / visdom

A library use jQuery like API for html parsing & node selecting & node mutation, suitable for web scraping and html confusion.
MIT License
110 stars 6 forks source link

Update lib.rs #7

Closed John0x closed 2 years ago

John0x commented 2 years ago

Code isn't tested. The current implementation causes problems in cases like the following:

async fn fetchDocument(url: &String) -> Result<Elements, String> {
    let client = reqwest::Client::new();
    let res = client.get(url).send().await.unwrap();
    let body = res.text().await.unwrap();

    let doc = Vis::load(&body).unwrap();

    Ok(doc)
}

Error:

error: cannot return value referencing local variable `body`

Something like the proposed changes should solve the problem.

fefit commented 2 years ago

Thanks for your pr, i will test the code in the daytime later.

fefit commented 2 years ago

@John0x Change the &str into Into<Cow('_, str)> will not fix the above example when pass the parameter body as a String reference, the body will dropped after the function end and the reference's lifetime is not long enough. So here need to get the ownership of body, maybe a type of Cow::from(body) to owned the body. The Into<Cow<'_, str>> will really cover the cases like below:

async fn fetchDocument(html: &String) -> Result<Elements, String> {
    let doc = Vis::load(&body).unwrap();
    Ok(doc)
}

the above example may look like:

async fn fetchDocument(url: &String) -> Result<Elements, String> {
    let client = reqwest::Client::new();
    let res = client.get(url).send().await.unwrap();
    let body = res.text().await.unwrap();

    let doc = Vis::load(Cow::from(body)).unwrap();

    Ok(doc)
}

I will merge the PR later, thanks for your first PR and good advice!

fefit commented 2 years ago

@John0x the version v0.5.1 has published to update this change. thx again!