Replace io::Cursor::{remaining_slice, is_empty}

[soerenmeier]

This is a late follow up to the concerns raised in #86369.

#86369 (comment)

This API seems focussed on the Read side of things. When Seeking around and Writeing data, is_empty becomes confusing and remaining_slice is not very useful. When writing, the part of the slice before the cursor is much more interesting. Maybe we should have functions for both? Or a single function that returns both slices? (If we also have a mut version, a single function would be useful to allow mutable access to both sides at once.)

New feature name: cursor_remaining > cursor_split.
Added functions:

fn split(&self) -> (&[u8], &[u8]);
// fn before(&self) -> &[u8];
// fn after(&self) -> &[u8];
fn split_mut(&mut self) -> (&mut [u8], &mut [u8]);
// fn before_mut(&mut self) -> &mut [u8];
// fn after_mut(&mut self) -> &mut [u8];

A question was raised in #86369 (comment) about whether to return a lifetime that would reflect the lifetime of the underlying bytes (impl Cursor<&'a [u8]> { fn after(&self) -> &'a [u8] }). The downside of doing this would be that it would not be possible to implement these functions generically over T: AsRef<[u8]>.

Update

Based on the review, before* and after* methods where removed.

[rustbot]

r? @thomcc

(rustbot has picked a reviewer for you, use r? to override)

[thomcc]

r? libs

[workingjubilee]

Do you think these after_ and before_ functions really significantly improve the ergonomics, or should we just keep it to split and split_mut? I feel like the latter names are very much "what I would expect to see" and the others less so.

[workingjubilee]

I mean, the fact that it's inherently stateful might make the before* and after* set of functions make more sense I suppose? It does feel a bit like, due to the fact that before_mut and after_mut will both exclude invoking the other thanks to Rust not being able to understand that they actually borrow "different things", it will lead to more annoying refactors if people want to call before_mut and then realize they wanted after_mut also. So we get still-clear but easy-to-refactor code if we just expect people to write

let (before, _) = cursor.split_mut();

This is a fairly common pattern in existing code that uses the slice functions with split_at_mut and the like. It's not the loveliest idiom, but it seems fine.

r? @workingjubilee
@rustbot author

[soerenmeier]

Yeah that's a valid point. But I think most of the times only the remaining bytes are of interest.
For example if i want to know how many bytes are left, it's clearer to write cursor.after().len() instead of cursor.split().1.len().
Another example might be (yes you could do that with just slices).

while !cursor.after().is_empty() {
     let written = stream.write(cursor.after())?;
    cursor.consume(written);
}

[workingjubilee]

Hm. Well. I guess?

Is there a reason to not start with split and split_mut and see how it goes?

[JohnCSimon]

@soerenmeier

ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks!

FYI: when a PR is ready for review, send a message containing
@rustbot ready to switch to S-waiting-on-review so the PR is in the reviewer's backlog.

[soerenmeier]

@jkugelman Since you initially proposed to add the split functions. What is your take on the points raised by @workingjubilee?

[jkugelman]

My instinct is that before and after would be more convenient and more commonly used, while split is good to round out the API but wouldn't be what I would usually reach for.

That said, I haven't used Cursor in quite a while, so my opinion isn't as freshly informed as it was 2 years ago.

[soerenmeier]

Thanks for your comment.
@workingjubilee should I remove the other methods to get this merged?

@rustbot ready

[workingjubilee]

My opinion remains the same, yes.

[soerenmeier]

@JohnCSimon Sorry for the inactivity. I can't reopen this PR, probably don't have the necessary rights. I pushed the requested changes to another branch, if you can reopen it, I will do a force push on this PR.

[soerenmeier]

Thanks.

@rustbot ready

[dtolnay]

Thanks!

[dtolnay]

@bors r+

[bors]

📌 Commit 10da555 has been approved by dtolnay

It is now in the queue for this repository.