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

implement PartialEq<str> for string new types #479

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

ModProg
Copy link
Contributor

@ModProg ModProg commented Aug 25, 2024

No description provided.

@ModProg
Copy link
Contributor Author

ModProg commented Aug 25, 2024

not sure about allowing both w/ and w/o the trailing \0. As the trailing \0 is an implementation detail, I could also remove the support for comparing with strings containing the \0.

@diwic
Copy link
Owner

diwic commented Aug 25, 2024

The strings are guaranteed to have a trailing nul byte (at least right now), meaning there is no need to do any special handling based on that they maybe have a trailing nul byte.

@ModProg
Copy link
Contributor Author

ModProg commented Aug 26, 2024

The strings are guaranteed to have a trailing nul byte (at least right now), meaning there is no need to do any special handling based on that they maybe have a trailing nul byte.

Yes, the internal string of, e.g., Signature has always a \0.

What I meant was the RHS &str having a trailing \0 or not, i.e., are both Signature::new("s") == "s" && Signature::new("s") == "s\0" or just Signature::new("s") == "s" && Signature::new("s") != "s\0".

@diwic
Copy link
Owner

diwic commented Aug 26, 2024

Right. So then I'll say that, since Signature and friends deref to str without the trailing zero, then what you should be equal to is a str without a trailing zero and nothing else.

@ModProg
Copy link
Contributor Author

ModProg commented Aug 26, 2024

Implementation now is:

    assert_eq!(Signature::new("s").unwrap(), "s");
    assert!(Signature::new("s").unwrap() != "s\0");

@diwic diwic merged commit 9e0fe15 into diwic:master Aug 27, 2024
8 checks passed
@diwic
Copy link
Owner

diwic commented Aug 27, 2024

Thanks!

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.

2 participants