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

Make return type of TreeSink::elem_name an associated type. #553

Merged
merged 3 commits into from
Sep 10, 2024

Conversation

nicoburns
Copy link
Contributor

Fixes #552

This allows this type to be owned or contain data protected by a RefCell or Mutex. It can also simply be ExpandedName - the type which was previously the return type of TreeSink::elem_name.

I have not yet had a chance to test this, but I believe this ought to fix my issue.

@nicoburns nicoburns requested a review from jdm September 6, 2024 23:39
@nicoburns
Copy link
Contributor Author

An unrelated clippy lint was failing. I've added an allow, although it seems to be a private item so we could update the variant names.

@nicoburns
Copy link
Contributor Author

Hmm... I don't think this actually works with AsRef (seems impossible to actually implement the trait sensibly). I think we'll need a custom trait.

@nicoburns nicoburns force-pushed the extended-name-associated-type branch 3 times, most recently from 250c59f to da4e240 Compare September 10, 2024 16:12
@nicoburns nicoburns force-pushed the extended-name-associated-type branch 2 times, most recently from 60db03f to c997ad8 Compare September 10, 2024 16:32
Copy link
Member

@jdm jdm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement. Thanks!

markup5ever/interface/tree_builder.rs Show resolved Hide resolved
This allows this type to be owned or contain data protected by a RefCell
or Mutex. It can also simply be `ExpandedName` - the type which was
previously the return type of `TreeSink::elem_name`

Signed-off-by: Nico Burns <[email protected]>
@nicoburns nicoburns added this pull request to the merge queue Sep 10, 2024
Merged via the queue into servo:main with commit 396f68a Sep 10, 2024
6 checks passed
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.

Problem using new immutable API with Slab-based DOM tree
3 participants