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.

Sign up for GitHub

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

Jump to bottom

Tracking issue for [T]::rsplit and [T]::rsplit_mut #41020

Closed
jorendorff opened this issue Apr 2, 2017 · 12 comments
Closed

Tracking issue for [T]::rsplit and [T]::rsplit_mut #41020

jorendorff opened this issue Apr 2, 2017 · 12 comments
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@jorendorff
Copy link
Contributor

jorendorff commented Apr 2, 2017 •
edited by SimonSapin
Loading

Update (@SimonSapin): this is now a tracking issue for methods implemented in #41065.

In @brson's tech review of Programming Rust, when he got to the bit where we list all the slice methods, he wrote: "What about 'rsplit' (no 'n')?"

I thought, oh no, I missed a method. But there really isn't a [T]::rsplit method!

str [T]
.split(p) .split(p)
.rsplit(p) nope
.splitn(p) .splitn(p)
.rsplitn(p) .rsplitn(p)

Slices don't have .rsplit(p), I guess maybe because .split(p).rev() does exactly what .rsplit(p) would do. (The two are subtly different for strings, in cases where the pattern can have overlapping matches.)

But is that a good enough reason to "break symmetry"?

@brson either really expected rsplit to exist, or he was speaking up for a hypothetical reader, new to Rust, who might expect it. Either way, I think it should be added, just to reduce by 1 the number of curious wrinkles in libcore. I'll write an RFC if this seems reasonable.
@bluss
Copy link
Member

bluss commented Apr 2, 2017

I don't think this improvement needs an RFC. It seems like Rsplit needs to use a distinct iterator type for encapsulation so that it can be distinct when subslice splitting finally comes around?

@jorendorff
Copy link
Contributor Author

OK, no RFC. Working on a patch. @bluss Is a feature flag needed for this?

@bluss
Copy link
Member

bluss commented Apr 4, 2017

Yes, the usual way is that you invent a new feature name for it and introduce as unstable.

@jorendorff
Copy link
Contributor Author

OK, thanks. I don't need to add it to libsyntax/feature_gate.rs though, right?

@bluss
Copy link
Member

bluss commented Apr 4, 2017

Correct, that's for language (this is library) feature gates.

jorendorff added a commit to jorendorff/rust that referenced this issue Apr 4, 2017
@jorendorff
add [T]::rsplit() and rsplit_mut() rust-lang#41020
2e3f0d8
jorendorff added a commit to jorendorff/rust that referenced this issue Apr 4, 2017
@jorendorff
simplify implementation of [T]::splitn and friends rust-lang#41020
a45fedf
@jorendorff
Copy link
Contributor Author

PR #41065.

arielb1 pushed a commit to arielb1/rust that referenced this issue Apr 5, 2017
Rollup merge of rust-lang#41065 - jorendorff:slice-rsplit-41020, r=al…
d8b6109 
…excrichton

[T]::rsplit() and rsplit_mut(), rust-lang#41020
@SimonSapin SimonSapin changed the title Add methods [T]::rsplit and [T]::rsplit_mut Tracking issue for [T]::rsplit and [T]::rsplit_mut Mar 17, 2018
@SimonSapin
Copy link
Contributor

This should not have been closed since it is the tracking issue for unstable APIs.

@SimonSapin SimonSapin reopened this Mar 17, 2018
@SimonSapin
Copy link
Contributor

SimonSapin commented Mar 17, 2018 •
edited
Loading

By itself this API seems unnecessary to me as it is the same as split(…).rev(), but str::rsplit and [T]::rsplitn already exist, so adding it for consistency seems good.

This has been in nightly for 11 months without issue:

@rfcbot fcp merge

@SimonSapin SimonSapin added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. labels Mar 17, 2018
@SimonSapin
Copy link
Contributor

With proper labels…

@rfcbot fcp merge

@rfcbot rfcbot added the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Mar 17, 2018
@rfcbot
Copy link

rfcbot commented Mar 17, 2018 •
edited by aturon
Loading

Team member @SimonSapin has proposed to merge this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Mar 19, 2018
@rfcbot
Copy link

rfcbot commented Mar 19, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot
Copy link

rfcbot commented Mar 29, 2018

The final comment period is now complete.

@tinaun tinaun mentioned this issue Apr 17, 2018
Merged
bors added a commit that referenced this issue Apr 18, 2018
@bors
Auto merge of #50017 - tinaun:stabilize-all-the-things, r=sfackler
ac3c228 
stabilize a bunch of minor api additions

besides `ptr::NonNull::cast` (which is 4 days away from end of FCP) all of these have been finished with FCP for a few weeks now with minimal issues raised

* Closes #41020
* Closes #42818
* Closes #44030
* Closes #44400
* Closes #46507
* Closes #47653
* Closes #46344

the following functions will be stabilized in 1.27:
* `[T]::rsplit`
* `[T]::rsplit_mut`
* `[T]::swap_with_slice`
* `ptr::swap_nonoverlapping`
* `NonNull::cast`
* `Duration::from_micros`
* `Duration::from_nanos`
* `Duration::subsec_millis`
* `Duration::subsec_micros`
* `HashMap::remove_entry`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: A tracking issue for an RFC or an unstable feature. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@jorendorff @SimonSapin @GuillaumeGomez @bluss @rfcbot