Repository navigation
Support chunked text processing via StreamSafe - #120
maratik123 wants to merge 2 commits into
Conversation
|
I want to ask about |
|
Derive debug is fine. |
| /// on the underlying iterator once. A character it returns is still | ||
| /// yielded by the returned iterator. | ||
| #[inline] | ||
| pub fn try_into_state(mut self) -> Result<StreamSafeState, Self> { |
There was a problem hiding this comment.
perhaps try_finish_into_state()?
There was a problem hiding this comment.
But it is not about to finish. It is about to mine state on exhausted iterator. Here we try to consume iterator to transfer state for creating next iterator. On unsuccessful operation we return iterator (it's up to user what to do with it - to drop or to use).
There was a problem hiding this comment.
hm we're "finishing" the current iterator. I kind of want the function name to make it clear that this is a finished iterator.
I do agree that we're not really finishing here. Have suggestions?
There was a problem hiding this comment.
How about try_into_final_state()? The try_into_ part keeps the Arc::try_unwrap-style convention (consume self, hand it back as Err on failure), and "final" says it's the state of a finished iterator without implying that the call finishes anything. If you'd rather have the condition spelled out, into_state_if_exhausted() works too, but we lost here try_ convention. try_into_state_if_exhausted() is too wordly.
There was a problem hiding this comment.
try_into_state_if_exhausted or try_exhausted_into_state maybe. I'm ok if it's a bit wordy, it's a poweruser API
| /// assert!(out.contains('\u{034F}')); | ||
| /// ``` | ||
| #[inline] | ||
| pub fn with_state(iter: I, state: StreamSafeState) -> Self { |
There was a problem hiding this comment.
Hmm, "new" and "resume" could confuse user if both used nearby. And what means "resume iterator from state"? For me it cumbersome.
There was a problem hiding this comment.
We're making a new iterator and resuming from existing state. Open to other name suggestions
There was a problem hiding this comment.
What about just resume(iter, state)? It's an associated function returning Self, so it already reads as a constructor without the new_ prefix, and the call site reads naturally: StreamSafe::resume(piece.chars(), state). It also pairs well with the state-taking method.
Closes #119