aantron / lambdasoup

Functional HTML scraping and rewriting with CSS in OCaml
https://aantron.github.io/lambdasoup
MIT License
380 stars 31 forks source link

Add `Soup.prepend_root` #45

Closed dmbaturin closed 2 years ago

dmbaturin commented 2 years ago

There's Soup.append_root but there's no Soup.prepend_root, and without it, inserting something before the very first node in a soup seems impossible.

This patch fixes that and also adds a test for append/prepend root functions specifically.

aantron commented 2 years ago

Can you prepend a node using insert_before? It would be a bit awkward.

aantron commented 2 years ago

Thank you. Would you like a release soon?

dmbaturin commented 2 years ago

@aantron No, I can't. insert_before only works for inserting a child inside a node, but for children of the root, it fails.

utop # #require "lambdasoup";;

utop # open Soup.Infix;;

utop # let s = Soup.parse "<p>test element</p>" ;;

utop # Soup.insert_before (s $ "p") (Soup.create_text "test stuff") ;;
Exception: Failure "Soup.insert_before: target node has no parent".

Regarding a release, I don't have any immediate use case blocked by this, but if you are planning one, I'll expose this in soupault's plugin API.

dmbaturin commented 2 years ago

By the way, do you consider that insert_before behaviour a bug or a design decision? I think it would be more convenient if it worked for children of an element node and children of the root alike.

aantron commented 2 years ago

It seems like a bug to me right now. In terms of a proper parent, a soup node isn't a real node and shouldn't be considered a parent. But it seems that you should be able to insert_before any non-soup node, including a root node.

dmbaturin commented 2 years ago

Do you plan to work on fixing it, or you'd rather offload it to me? I don't mind looking into it if your time budget is tight.

aantron commented 2 years ago

My time budget unfortunately is tight for at least a few weeks. I don't want to say that I'd rather "offload" it to you :) But I won't plan to fix it myself for now, either. If you do fix it, I will be very glad to merge it :)