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

Rename matcher Contains #45

Open
ianlet opened this issue Jan 4, 2017 · 6 comments
Open

Rename matcher Contains #45

ianlet opened this issue Jan 4, 2017 · 6 comments

Comments

@ianlet
Copy link
Contributor

ianlet commented Jan 4, 2017

Currently, the matcher Contains is used to assert that a Vec contains multiple elements.

Would it be more appropriate to rename it ContainsElements or ContainsAllOf and have another matcher Contains to assert the presence of a single element in a Vec?

Something like:

assert_that!(vec!(1, 2, 3), contains_elements(vec!(1, 2)));
// or
assert_that!(vec!(1, 2, 3), contains_all_of(vec!(1, 2)));

and

assert_that!(vec!(1, 2, 3), contains(1));
@ianlet ianlet changed the title Rename matcher Contains by ContainsElements Rename matcher Contains Jan 5, 2017
@ujh
Copy link
Owner

ujh commented Jan 9, 2017

I wonder if it would be possible to implement From for Vec in such a way that contains would just convert a single element into a vector with one element. I'm not sure when I'll find the time to try that though.

@ianlet
Copy link
Contributor Author

ianlet commented Jan 10, 2017

I'm still a beginner with Rust so I'm might be wrong, but I don't think this is possible to provide a generic implementation of From to convert to a Vec as it would probably collide with other implementations. I've just tried to impl<T> From<T> for Vec<T> but I got an error saying that T must be used as the type parameter for some local type.

On the other hand, what would be possible is changing contains signature to accept a generic T + Into<V<T>> but this is not really convenient as it will force the clients to implement the From trait for each of their types.

@ujh
Copy link
Owner

ujh commented Jan 11, 2017

I guess you're right. I don't know Rust well enough to know if that would be possible or not either.

I'm OK with either name, but I would do it in two steps and just add contains_elements or contains_all_of first and deprecate contains. I would also add contain_elements/contain_all_of as an "alias" so that does_not(contain_all_of(...)) reads nicer.

Once that's in I would do a release and the new contains + (contain) can the go into master.

@povilasb
Copy link
Contributor

povilasb commented Jan 16, 2018

I wonder if it would be possible to implement From for Vec in such a way that contains would just convert a single element into a vector with one element. I'm not sure when I'll find the time to try that though.

That might be possible in the future when RFC-1210 gets implemented.
If interested, you can track the status at rust-lang/rust#31844

@povilasb
Copy link
Contributor

I do have motivation and some spare time, hence I could implement those changes :)

@ujh
Copy link
Owner

ujh commented Jan 17, 2018

That would be pretty cool @povilasb!

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

No branches or pull requests

3 participants