feat(Itertools): add strip_prefix and strip_prefix_by methods#1104
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1104 +/- ##
==========================================
- Coverage 94.38% 93.98% -0.41%
==========================================
Files 48 52 +4
Lines 6665 6679 +14
==========================================
- Hits 6291 6277 -14
- Misses 374 402 +28 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hi there, thanks for this. Regarding the API: Currently this only returns Can we - instead of
Returning a bespoke |
|
Makes sense, the failure case is worth exposing. I'll switch the return type to Result<Self, (Self, Self::Item, J::Item, J)> so callers can recover the partially-consumed iterators and the mismatched items. Will push the reworked version shortly. |
|
One edge case before I push: when |
Good catch. Maybe this makes things clear? Aside: Maybe even rename |
|
Reworked to use a public |
phimuemue
left a comment
There was a problem hiding this comment.
Thank you for this. Could you adopt my changes, then I think we're good to go.
| /// | ||
| /// All fields are public so callers can recover the partially-consumed | ||
| /// iterators and the mismatched items. | ||
| #[derive(Debug, Clone, PartialEq, Eq)] |
There was a problem hiding this comment.
Let's avoid PartialEq, Eq if it's unclear if it is really needed. (Comparing iterator is oftentimes not doable via PartialEq oftentimes.)
There was a problem hiding this comment.
Dropped, kept only Debug, Clone.
| let mut prefix = prefix.into_iter(); | ||
| while let Some(wanted) = prefix.next() { | ||
| match self.next() { | ||
| Some(got) if eq(&got, &wanted) => continue, | ||
| got => { | ||
| return Err(StripPrefixError { | ||
| iterator: self, | ||
| prefix, | ||
| mismatch: (got, wanted), | ||
| }); | ||
| } | ||
| } | ||
| } | ||
| Ok(self) |
There was a problem hiding this comment.
We canonically try to use avoid the while let Some(wanted) = prefix.next pattern. Can you change it to this?
let mut prefix = prefix.into_iter();
match prefix.by_ref().try_for_each(|wanted| {
match self.next() {
Some(got) if eq(&got, &wanted) => Ok(()),
got => Err((got, wanted)),
}
}) {
Ok(()) => Ok(self),
Err(mismatch) => Err(StripPrefixError {
iterator: self,
prefix,
mismatch,
}),
}
There was a problem hiding this comment.
Switched to try_for_each per your snippet.
Adds
strip_prefixandstrip_prefix_byto theItertoolstrait, the iterator analogue ofstr::strip_prefix, as discussed in #1096.strip_prefixis implemented in terms ofstrip_prefix_by, which takes an equality predicate so the prefix items may differ in type fromSelf::Item. Includes doctests and quickcheck tests that assert parity withstr::strip_prefix.Closes #1096