Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added better mutability, removed some clones #8

Merged
merged 6 commits into from
Apr 7, 2023

Conversation

arduano
Copy link
Contributor

@arduano arduano commented Mar 29, 2023

I started using this library in order to modify attributes in my HTML archiver (e.g. modify image src attributes, etc). However I noticed that this library doesn't support mutably querying elements by selectors, so instead of manually implementing it in my code I decided to fork this library instead. Along the way, I modified some other things as well.

Here is the full list of whats changed, off the top of my head:

  • Node::Element now has (Element) instead of { element's fields }. This means we don't need to borrow (or clone) things across when coverting a Node into an Element.
  • The above means that the number of clones went down significantly, previously it would make heaps of unnecessary clones when querying (cloning an entire tree just to get the next child element, etc).
  • Made query functions return references instead of owned+cloned values. This is both better for performance, but also it lets the queries return &mut references, which is useful for editing the html
  • Added a function that mutably executes a closure on all elements that match a selector, this is very useful for editing element attributes.
  • Removed .into_element, instead it's .as_element which returns an option of the borrowed element inside.
  • Implemented .into_node for Element for easier conversion, as well as an Into<Node> just in case.

Also, for the doctest in the remove_by function, I added an extra element in that line so that the IDE's auto formatter (not even rustfmt) doesn't strip the extra spaces from that line. VS Code automatically trims the end of each line, which makes that test fail after saving the file.

@lomirus
Copy link
Owner

lomirus commented Mar 29, 2023

Thanks for your great work. It seems good.

However, I maybe busy in the recent a few days or a week, so it may take some time for me to be able to review this PR. Thanks for your patience :)

src/lib.rs Outdated Show resolved Hide resolved
src/operation/edit.rs Outdated Show resolved Hide resolved
@arduano arduano requested a review from lomirus April 5, 2023 11:53
@lomirus
Copy link
Owner

lomirus commented Apr 6, 2023

Some clippy errors need to be fixed.

@arduano arduano requested a review from lomirus April 6, 2023 13:54
@arduano
Copy link
Contributor Author

arduano commented Apr 6, 2023

Just need your approval to re-run the workflow

@lomirus lomirus merged commit f9c5d7f into lomirus:master Apr 7, 2023
1 check passed
@arduano arduano deleted the dont-clone-when-querying branch April 7, 2023 05:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants