Implement two-way iterators and peekables

[blt-r]

Closes #82.

I decided to make a struct TwoWayPeekable which will contain the iterator and the peeked value, similar to std::iter::Peekable. This allows to eliminate code duplication and this way the peeking-related overhead will affect only those who need to peek. As a downside, this approach increases the API surface area. It introduces a new trait TwoWayIterator, which probably should be a standard library feature.

I named the method that makes iterator peekable .two_way_peekable(), because .peekable() would conflict with std::iter::Iterator::peekable. Admittedly, two_way_peekable is not a very good name, so it probably should be changed, but I don't know to what. We could just name it peekable, but that would be a breaking change, and would require a major version bump, so probably not worth it.

[cessen]

It introduces a new trait TwoWayIterator, which probably should be a standard library feature.

Yeah, at some point I want to submit an RFC for that. It really feels like a missing API in Rust's iterators. It's also pretty surprising to me that DoubleEndedIterator is in std when something like TwoWayIterator isn't. The former seems really niche to me, whereas the latter seems more broadly useful. Dunno.

In any case, I don't think it makes much sense to create traits for these things in Ropey itself. The point of traits in this case would be interoperability, and bespoke traits in Ropey doesn't achieve that.

I think what probably makes the most sense is to implement std::iter::Peekable for forward peeking, and then just directly implement a .peek_prev() method on each of the iterators' impls. Basically, following the same pattern as .next() and .prev() already do.

[cessen]

I think what probably makes the most sense is to implement std::iter::Peekable for forward peeking

Oops. I misremembered what std::iter::Peekable was. I thought it was a trait, but indeed it's a struct that wraps the iterator.

Hmm. That makes me wonder if this functionality even belongs directly in Ropey, then. Maybe a separate crate that provides a TwoWayPeekable struct and the relevant traits?

I mean, I'm still happy to have .peek_next() and .peek_prev() methods on Ropey's iterators, but I think just making them direct impls makes the most sense for Ropey's API.

[blt-r]

Hmm. That makes me wonder if this functionality even belongs directly in Ropey, then. Maybe a separate crate that provides a TwoWayPeekable struct and the relevant traits?

Yeah, this whole thing is very generic and could be in a separate crate. (or the standard library?)

I mean, I'm still happy to have .peek_next() and .peek_prev() methods on Ropey's iterators, but I think just making them direct impls makes the most sense for Ropey's API.

I agree. We can put the peeked field and these methods directly on the iterators, but .prev(), .next(), .peek_prev(), and .peek_next() on TwoWayPeekable are like 100 lines of code in total and duplicating that 4 times seems not ideal to me. I think, when the peeking functionality is implemented like this, with remembering the peeked value, it makes more sense to have them in a separate struct, like it was done is std.

I think the best implementation for ropey would be if peek methods would actually get the value, but not advance the iterator. But that is not trivial to implement, and I don't have the knowledge to do that. If you think you might do that in the future, I can make a PR with the "placeholder" implementation

[cessen]

I've been thinking about this more, and I'm actually thinking that maybe this functionality just doesn't belong directly in Ropey after all. The pattern in std is to have a separate wrapping iterator for peeking, and I think it's best to stick with that pattern. However, at the same time, having such a wrapper in Ropey itself doesn't feel right.

Ideally std will eventually have something like a BidirectionalIterator trait, and then a bidir peeking wrapper could be built on top of that. But at the moment no such trait exists in std.

So as a compromise in the mean time, I think the best thing to do would be to add a peeking wrapper to examples/, similar to the grapheme iterator/functions. That way there is a known good reference implementation for those that need the functionality.

Admittedly, this isn't ideal. But I think it's a reasonable compromise for the time being.