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

impl a From instead of an Into #836

Closed
mightyiam opened this issue Jan 8, 2024 · 1 comment · Fixed by #843
Closed

impl a From instead of an Into #836

mightyiam opened this issue Jan 8, 2024 · 1 comment · Fixed by #843

Comments

@mightyiam
Copy link
Contributor

impl<A, B> Into<Option<Either<A, B>>> for EitherOrBoth<A, B> {
fn into(self) -> Option<Either<A, B>> {
match self {
EitherOrBoth::Left(l) => Some(Either::Left(l)),
EitherOrBoth::Right(r) => Some(Either::Right(r)),
_ => None,
}
}
}

@mightyiam mightyiam mentioned this issue Jan 8, 2024
@Philippe-Cholet
Copy link
Member

It originated in #740 (From should be prefered to Into implementation as the first gives the second but it's not true the other way) where I said #740 (comment)

I don't see a point of not doing it so I suggest you convert it to a From implementation in a separate commit.

EDIT: After searching, it originated in #327 and nobody said it should not be From instead.

In #327, there is

A fallible method of converting EitherOrBoth into an Either would be very useful providing exclusive (not including Both variant) methods. Converting should probably wait for rust-lang/rust#33417.

So I guess the question is: Instead of Into<Option<_>>, do we want a TryFrom or a From implementation?
I would vote for From (better than Into, no breaking change) over TryFrom (breaking change, what kind of error?).

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 a pull request may close this issue.

2 participants