Can we make a release incorporating the latest lambdasoup version?

[jbhoot]

The latest release of lambdasoup now supports :has() selector.

Is incorporating it in soupault only a matter of updating the lambdasoup dependency?

Happy to make a PR with your guidance.

[dmbaturin]

Yes, I saw the release of lambdasoup and was waiting for it to appear in opam. I want to add HTML.is_text and HTML.is_document to the API, that was impossible until aantron/lambdasoup/pull/52.

If you want to try your hand at it, you can follow the example of HTML.is_element.

It looks like those those functions work well with nodes coerced to "general nodes":

utop # Soup.create_text "test!" |> Soup.coerce |> Soup.is_text ;;
- : bool = true

utop # Soup.create_text "test!" |> Soup.coerce |> Soup.is_document ;;
- : bool = false

utop # Soup.create_soup () |> Soup.coerce |> Soup.is_document ;;
- : bool = true

Thus it should be fine to just use to_general to coerce them and do the respective test.

In fact, I wonder if HTML.is_element can be safely rewritten without a special case for ElementNode, or if the wrapper type is still necessary at all now that is_document is a built-in test. But it's a question for a bigger future release, I suppose.

[dmbaturin]

Then again, updating the dependency to support :has() and adding those new functions are separate tasks, so if you feel like making a PR to update the dependency first, it's really just a matter of updating the version in soupault.opam. Then the new functions can be in a separate PR if you feel like doing it, or I can do it myself over the weekend if you don't have the time.

[jbhoot]

Right. Let me update the dependency first, then I will help in incorporating the newer functions too.

[jbhoot]

@dmbaturin Are you fine with making an release that only contains the updated dep on lambdasoup?

Or do you want to make a single release after incorporating the new functions?

[dmbaturin]

Since making a release involves building binaries for all platforms, I'd rather bundle those changes.

[jbhoot]

Agreed. Less grunt work that way. I will see if I can take it up tomorrow or in the next week.

[jbhoot]

Regarding HTML.is_text implementation

Based on HTML.is_element as you suggested, I wrote the following function:

let is_text n =
  match n with
  | GeneralNode n -> Soup.is_text n
  | _ -> false

But, from what you wrote:

Thus it should be fine to just use to_general to coerce them and do the respective test.

it seems that you want an implementation like:

  let is_text n =
    n |> to_general |> Soup.is_text 

Comments?

[jbhoot]

Regarding is_document function

src/plugin_api.ml already has an is_document function:

  let is_document n =
    match n with
    | SoupNode _ -> true
    | _ -> false

Perhaps you want to rewrite it to the following?

  let is_document n =
    n |> to_general |> Soup.is_document

[dmbaturin]

@jbhoot Sorry for the late reply, I've been a bit overwhelmed. I merged the PR, I think the change is reasonable and any concerns that made me write it that way no longer apply (if they even existed at all).

Regarding the new release, aantron/lambdasoup/pull/61 was merged today and that may be relevant for some people. Let's see if @aantron plans to make a new release soon.

[aantron]

The :has selector was already released in Lambda Soup 1.1.0 (as, I understand, you spotted). And yes, I will do a new release now with latest PR.

[dmbaturin]

It's official now, thanks for the help and cooperation!

https://github.com/PataphysicalSociety/soupault/releases/tag/4.11.0