Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add io::Cursor::{remaining, remaining_slice, is_empty} #86037

Merged
merged 3 commits into from
Jun 22, 2021

Conversation

soerenmeier
Copy link
Contributor

@soerenmeier soerenmeier commented Jun 5, 2021

Tracking issue: #86369

I came across an inconvenience when answering the following Stack Overflow question.
To get the remaining slice you have to call buff.fill_buf().unwrap(). Which in my opinion doesn't really tell you what is returned (in the context of Cursor). To improve readability and convenience when using Cursor i propose adding the method remaining.

The next thing i found inconvenient (unnecessary long) was detecting if the cursor reached the end. There are a few ways this can be achieved right now:

  • buff.fill_buf().unwrap().is_empty()
  • buff.position() >= buff.get_ref().len()
  • buff.bytes().next().is_none()

Which all seem a bit unintuitive, hidden in trait documentations or just a bit long for such a simple task.
Therefor i propose another method called is_empty, maybe with another name, since this one may leave room for interpretation on what really is empty (the underlying slice, the remaining slice or maybe the position).

Since it seemed easier to create this PR instead of an RFC i did that, if an RFC is wanted, i can close this PR and write an RFC first.

@rust-highfive
Copy link
Collaborator

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @yaahc (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 5, 2021
@yaahc
Copy link
Member

yaahc commented Jun 14, 2021

Since it seemed easier to create this PR instead of an RFC i did that, if an RFC is wanted, i can close this PR and write an RFC first.

No RFC is necessary I think, but we should setup a tracking issue. The APIs look good to me and I'm happy to have these methods. I'm a little bothered by the inconsistency with the bytes API that inspired this, since their remaining returns the remaining length but ours is going to return the remaining slice. The best alternative I can think of is as_buf but it doesn't make it as clear that this a subslice from the current position. Possibly remaining_slice/remaining_buf/remaining_bytes?

Curious what the rest of @rust-lang/libs thinks.

@soerenmeier soerenmeier changed the title Add io::Cursor::{remaining, is_empty} Add io::Cursor::{remaining, remaining_slice, is_empty} Jun 17, 2021
@yaahc
Copy link
Member

yaahc commented Jun 21, 2021

Looks good, thank you again @soerenmeier.

@bors r+

@bors
Copy link
Contributor

bors commented Jun 21, 2021

📌 Commit 664bde0 has been approved by yaahc

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jun 21, 2021
Add `io::Cursor::{remaining, remaining_slice, is_empty}`

Tracking issue: rust-lang#86369

I came across an inconvenience when answering the following [Stack Overflow](https://stackoverflow.com/questions/67831170) question.
To get the remaining slice you have to call `buff.fill_buf().unwrap()`. Which in my opinion doesn't really tell you what is returned (in the context of Cursor). To improve readability and convenience when using Cursor i propose adding the method `remaining`.

The next thing i found inconvenient (unnecessary long) was detecting if the cursor reached the end. There are a few ways this can be achieved right now:
- `buff.fill_buf().unwrap().is_empty()`
- `buff.position() >= buff.get_ref().len()`
- `buff.bytes().next().is_none()`

Which all seem a bit unintuitive, hidden in trait documentations or just a bit long for such a simple task.
Therefor i propose another method called `is_empty`, maybe with another name, since this one may leave room for interpretation on what really is empty (the underlying slice, the remaining slice or maybe the position).

Since it seemed easier to create this PR instead of an RFC i did that, if an RFC is wanted, i can close this PR and write an RFC first.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 22, 2021
Rollup of 11 pull requests

Successful merges:

 - rust-lang#85054 (Revert SGX inline asm syntax)
 - rust-lang#85182 (Move `available_concurrency` implementation to `sys`)
 - rust-lang#86037 (Add `io::Cursor::{remaining, remaining_slice, is_empty}`)
 - rust-lang#86114 (Reopen rust-lang#79692 (Format symbols under shared frames))
 - rust-lang#86297 (Allow to pass arguments to rustdoc-gui tool)
 - rust-lang#86334 (Resolve type aliases to the type they point to in intra-doc links)
 - rust-lang#86367 (Fix comment about rustc_inherit_overflow_checks in abs().)
 - rust-lang#86381 (Add regression test for issue rust-lang#39161)
 - rust-lang#86387 (Remove `#[allow(unused_lifetimes)]` which is now unnecessary)
 - rust-lang#86398 (Add regression test for issue rust-lang#54685)
 - rust-lang#86493 (Say "this enum variant takes"/"this struct takes" instead of "this function takes")

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 4463b08 into rust-lang:master Jun 22, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 22, 2021
JohnTitor added a commit to JohnTitor/rust that referenced this pull request Jul 12, 2021
Remove unstable `io::Cursor::remaining`

Adding `io::Cursor::remaining` in rust-lang#86037 caused a conflict with the implementation of `bytes::Buf` for `io::Cursor`, leading to an error in nightly, see rust-lang#86369 (comment).

This fixes the error by temporarily removing the `remaining` function.

r? `@yaahc`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants