make many extend into other collections that Vec #1621
Conversation
Pull Request Test Coverage Report for Build 3925742307Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
| @@ -491,7 +491,7 @@ mod test { | |||
| let b = "ababcd"; | |||
|
|
|||
| fn f(i: &str) -> IResult<&str, &str> { | |||
| recognize(many(1.., alt((tag("a"), tag("b")))))(i) | |||
| recognize::<_, Vec<&str>, _, _>(many(1.., alt((tag("a"), tag("b")))))(i) | |||
There was a problem hiding this comment.
I've been going back and forth on this idea partly because of type inference issues when fold_many* easily let's people do the same thing. I was surprised; they came up less than I expected.
This will likely remove the optimizatio of preallocating vector capacity, but we can get this back with a wrapper type if needed, and being able to created directly a hashmap or other collections instead of going through a vec first will be way more useful
fold_many is likely lighter weight than a newtype (code wise). Maybe we should provide a fold_many example that sets the capacity and have many link out to it.
There was a problem hiding this comment.
It makes sense that there would be inference issues in the tests calling many directly and the ones using recognize , but it worked surprisingly well everywhere else, so this is giving me confidence to make this change.
The example pointing towards fold_many is a good idea, that will let people define their own capacity allocation
| opt(multispace), | ||
| map( | ||
| many(0.., terminated(key_value, opt(multispace))), | ||
| |vec: Vec<_>| vec.into_iter().collect(), |
There was a problem hiding this comment.
How come this into_iter().collect() was left in?
There was a problem hiding this comment.
I missed it, fixed in 4c6e45e
Fix #1409
This will likely remove the optimizatio of preallocating vector capacity, but we can get this back with a wrapper type if needed, and being able to created directly a hashmap or other collections instead of going through a vec first will be way more useful