Skip to content

Conversation

nicoburns
Copy link
Contributor

@simonwuelker
Copy link
Contributor

simonwuelker commented Sep 9, 2025

Feels a bit weird to compare tendril bytes without knowing what format they are. This will yield unexpected results when comparing str with tendrils using non-utf8 encodings.

Maybe we could only impl this for the formats that come with tendril out of the box(https://docs.rs/tendril/latest/tendril/fmt/index.html#structs) ? Those that are known to be subsets of utf8.

@mrobinson mrobinson self-requested a review September 9, 2025 12:53
@nicoburns nicoburns force-pushed the tendril-partial-eq-str branch from e399a5b to 1fa7360 Compare September 9, 2025 13:33
@nicoburns nicoburns changed the title Impl PartialEq<str> for Tendril<F, A> Impl PartialEq<str> for Tendril<ASCII> and Tendril<UTF8> Sep 9, 2025
@nicoburns
Copy link
Contributor Author

Feels a bit weird to compare tendril bytes without knowing what format they are. This will yield unexpected results when comparing str with tendrils using non-utf8 encodings.

Maybe we could only impl this for the formats that come with tendril out of the box(https://docs.rs/tendril/latest/tendril/fmt/index.html#structs) ? Those that are known to be subsets of utf8.

I'm inclined to agree with this. I've restricted the impl to Tendril<ASCII> and Tendril<UTF8>.

I think one could argue for an impl for Tendril<WTF8> too. And a comparision to &[u8] for Bytes. But I suspect the UTF8 one is the most useful, and we can always add more later.

@nicoburns nicoburns added this pull request to the merge queue Sep 9, 2025
Merged via the queue into servo:main with commit ea26b82 Sep 9, 2025
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.

StrTendril should implement PartialEq<str>
3 participants