Skip to content

Tracking Issue for peek_index #130711

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

Open
x4exr opened this issue Sep 22, 2024 · 4 comments
Open

Tracking Issue for peek_index #130711

x4exr opened this issue Sep 22, 2024 · 4 comments
Assignees
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@x4exr
Copy link
Contributor

x4exr commented Sep 22, 2024

Feature gate: #![feature(peek_index)]

This is a tracking issue for the new peek_index function on the Enumerate iterator.

Public API

let e = vec![1, 2, 3].into_iter().enumerate();
assert_eq!(e.peek_index(), 0);
e.next();
assert_eq!(e.peek_index(), 1);

Steps / History

  • Created ACP issue.
  • Opened pull request following the ACP approval.

Unresolved Questions

  • None yet.
@x4exr x4exr added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 22, 2024
@workingjubilee workingjubilee changed the title Tracking Issue for https://github.com/rust-lang/libs-team/issues/435#issuecomment-2356672385 Tracking Issue for peek_index Sep 22, 2024
@GrigorenkoPV
Copy link
Contributor

I suspect that the code example is slightly incorrect, as the .0 part after .peek_index() implies it returns a tuple.

Also I think a signature of the added function(s) would be a better fit than a usage example for the "Public API" section, (I'm not a team member though, so I might be wrong).

@x4exr
Copy link
Contributor Author

x4exr commented Sep 26, 2024

That was a typo on my part lol, thanks for catching that

@tgross35 tgross35 added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Apr 7, 2025
@jogru0
Copy link
Contributor

jogru0 commented Apr 8, 2025

I'll take a look at the closed PR.

@rustbot claim

ChrisDenton added a commit to ChrisDenton/rust that referenced this issue Apr 19, 2025
add next_index to Enumerate

Proposal: rust-lang/libs-team#435
Tracking Issue: rust-lang#130711

This basically just reopens rust-lang#130682 but squashed and with the new function and the feature gate renamed to `next_index.`

There are two questions I have already:
- Shouldn't we add test coverage for that? I'm happy to provide some, but I might need a pointer to where these test would be.
  - Maybe I could actually also add a doctest?
- For now, I just renamed the feature name in the unstable attribute to `next_index`, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Apr 20, 2025
Rollup merge of rust-lang#139533 - jogru0:130711, r=Mark-Simulacrum

add next_index to Enumerate

Proposal: rust-lang/libs-team#435
Tracking Issue: rust-lang#130711

This basically just reopens rust-lang#130682 but squashed and with the new function and the feature gate renamed to `next_index.`

There are two questions I have already:
- Shouldn't we add test coverage for that? I'm happy to provide some, but I might need a pointer to where these test would be.
  - Maybe I could actually also add a doctest?
- For now, I just renamed the feature name in the unstable attribute to `next_index`, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this issue Apr 22, 2025
add next_index to Enumerate

Proposal: rust-lang/libs-team#435
Tracking Issue: rust-lang#130711

This basically just reopens rust-lang#130682 but squashed and with the new function and the feature gate renamed to `next_index.`

There are two questions I have already:
- Shouldn't we add test coverage for that? I'm happy to provide some, but I might need a pointer to where these test would be.
  - Maybe I could actually also add a doctest?
- For now, I just renamed the feature name in the unstable attribute to `next_index`, as well, so it matches the new name of the function. Is that okay? And can I just do that and use any string, or is there a sealed list of features defined somewhere where I also need to change the name?
@jdahlstrom
Copy link

(commencing bikeshedding…)

There's some prior art on naming: The method returning the "next" offset of CharIndices was stabilized as just offset in 1.82. Before stabilization there was quite a bit of discussion on whether the name should contain "next", but in the end dtolnay's arguments convinced the team that it shouldn't. The arguments seem to apply here too, and now there's the consistency angle as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants