Merge multipeek peeknth

[Owen-CH-Leung]

As per #933 , this PR merges MultiPeek and PeekNth into a general private interface MultiPeekGeneral, with public type alias to define the compatible MultiPeek and PeekNth struct.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 78.26087% with 15 lines in your changes missing coverage. Please review.

Project coverage is 94.32%. Comparing base (6814180) to head (4fc8456). Report is 126 commits behind head on master.

Files with missing lines	Patch %	Lines
src/multipeek_general.rs	77.94%	15 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #940      +/-   ##
==========================================
- Coverage   94.38%   94.32%   -0.07%     
==========================================
  Files          48       48              
  Lines        6665     7013     +348     
==========================================
+ Hits         6291     6615     +324     
- Misses        374      398      +24     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[phimuemue]

My suggestion: Deprecate multipeek and unify all the functionality into peek_nth.

Rationale: peek_nth is (advertised as) a drop-in replacement for std::iter::Peekable (nice for users), whereas multipeek offers exactly the same interface, but has subtle semantic differences (not nice for users if they are bitten by this).

And imho we should even un-fuse it to really be in line with std::iter::Peekable.

[Philippe-Cholet]

According to https://github.com/search?q=itertools+multipeek+reset_peek+lang%3ARust&type=code there is some real use of MultiPeek. Deprecate it seems excessive to me.

Maybe the status quo is not so bad.

I'm not opposed to unfuse it but it's a breaking change, I'd prefer to not break things for a while.

[phimuemue]

According to https://github.com/search?q=itertools+multipeek+reset_peek+lang%3ARust&type=code there is some real use of MultiPeek. Deprecate it seems excessive to me. [...] I'm not opposed to unfuse it but it's a breaking change, I'd prefer to not break things for a while.

Yes, we should not deprecate at a whim. But honestly I really like to know if deprecating wasn't for the better in the long run.

My rationale (and the linked examples appear to confirm this to some extent):

• People use MultiPeek even when there are simpler, more efficient alternatives: In particular, cloneable iterators. These could just be cloned - often more efficient than an allocating MultiPeek must allocate. So these programs do not need MultiPeek in the first place. => Maybe depretacing it or offering another API pushes users towards the better alternatives.
• Where it's really needed: MultiPeek requires the users to be careful with the "cursor", so that callers sprinkle reset_peak because they need their iterator in a predictable state. I'm not sure that there's a simple API that avoids the problem, but I find this a recurring (and bad) pattern with this iterator. => Is there any real use case that does not conclude peeking with reset_peek? If not, can we offer an API that does not put this burden onto the callers?
• As said, subtle difference between PeekNth and MultiPeek. => If prefer an API that makes this distinction more explicit.

On top of that, I do not see a problem in keeping a deprecated MultiPeek and motivate users to go with the replacements (if we find them) in PeekNth.