Add str.substr() and str.substr_until() methods

[Xinayder]

str.substr(start_index: usize) returns a substring of str starting from the specified point (Hello.substr(1) = ello)

str.substr_until(start_index: usize, length: usize) returns a substring of str starting from the specified point, with the specified length (Hello.substr_until(0, 2) = He)

[rust-highfive]

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @alexcrichton (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.

[oli-obk]

that optimization is unnecessary

[oli-obk]

What's the motivation behind this change? What's the advantage over using the slicing syntax directly?

[Xinayder]

@oli-obk Using the slicing syntax directly can be confusing. For example, someone wanting to retrieve a substring (or, a slice) with length of 2 characters could think this would do it:

let string = "Hello, world!"
let substring = string[7, 2]; // we want to retrieve the 'wo' part

However, this gives a compile error, because the last parameter of the range syntax works like start + desired length. The above example should then be:

let substring = string[7, 9];

It also allows you to substring from String directly, no need to convert it to a &str.

That's where I got the motivation to submit this Pull Request.

EDIT: the above snippet implemented with my code would look like:

let substring = string.substr_until(7, 2);

[oli-obk]

While the range syntax doesn't support this directly, the following works: let substring = &string[7..][..2]; The first slice takes all characters starting at the 7th byte, and then the second slicing takes all the characters up to and including the 2nd byte from the first slice.

[bluss]

We do want some kind of Option-enabled slicing api, but the naming and api needs some adjustment. It should use a range as the argument, just like Index does, and like String::drain() does.

[Xinayder]

@bluss Are you suggesting me to remove the two methods and just go with one instead, that takes a range as an argument?

[bluss]

Yes. I don't know if we have or need an RFC for this, but it's in fact something we want. But not to have an alternative argument style, but to provide careful slicing (which returns None on out of bounds / indexing error).

[Xinayder]

As of now, as @bluss suggested, I decided to go with a RangeArgument as the only argument the function needs. Now, it will return None if no slice could be retrieved from the string.

let string = String::from("Hello, world!");
assert_eq!(Some("world!"), string.substr(7..));

[TimNN]

Looking at PR #31191 it seems as if moving RangeArgument to core::ops (and the using it throughout the standard library) is a little bit more involved than the two commits in this PR, so this PR may want to wait until #31191 has landed.

[bluss]

This needs the condition start <= end too, to be correct (and never panic).

[Xinayder]

@TimNN I don't see how moving RangeArgument to core::ops is more than what I did. Doesn't #31191 allow indexing using RangeArgument instead of the special cases?

[TimNN]

Yeah, I looked over the PR's in more detail again and may habe been a bit hasty in coming to my conclusion when posting the previous comment. I mainly wanted create a link between both PR's (since they both moved RangeArgument to core::ops) and #31191 seemed relatively complex / involved whereas here the move seemed to not be the main focus of the PR but rather a later add-on.

[alexcrichton]

Thanks for the PR @RockyTV! This definitely seems related to rust-lang/rfcs#1325 which @bluss mentioned in terms of an Option-returning slice API as well. I suspect that we're going to want to consider both str and slices at the same time when creating an API such as this.

Otherwise, it looks like (as others have commented here), this API is subsumed by &foo[a..b] today in terms of functionality (minus the return value of Option).

[bors]

☔ The latest upstream changes (presumably #31120) made this pull request unmergeable. Please resolve the merge conflicts.

[alexcrichton]

Closing due to inactivity, but feel free to resubmit with a rebase!