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

str::split_once is underpowered #76512

Closed
HeroicKatora opened this issue Sep 9, 2020 · 6 comments
Closed

str::split_once is underpowered #76512

HeroicKatora opened this issue Sep 9, 2020 · 6 comments
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@HeroicKatora
Copy link
Contributor

HeroicKatora commented Sep 9, 2020

This nightly feature (#74773) makes it easy to parse key-value pairs or lists.

I could have used this method recently but required the matching character as well. Problematically it would have used a predicate pattern for finding one of several characters. To retrieve it separately is quite awkward with lots of index logic and some unwrap. Should there be a set of alternative method (or this method changed) that returns the separator as well? I'd implement that new set or change the existing methods but wanted to have some feedback on the idea first.

Rough draft of code I wanted to use:

let key_value: &str = _;
if let Some((lhs, sep, rhs)) = key_value.split_once(|ch| ch == '=' || ch == ':') {
    // handle
}

cc: @matklad

@jonas-schievink jonas-schievink added C-enhancement Category: An issue proposing an enhancement or a PR with one. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 9, 2020
@Nicholas-Baron
Copy link
Contributor

Looking at your example, the type signature would be roughly

fn split_once<'a>(&'a self, p: impl Pattern<'a>) -> Option<(&'a str, &'a str, &'a str)>

Pattern already has the ability to take a predicate, so only changing the return type is required.

@HeroicKatora
Copy link
Contributor Author

HeroicKatora commented Sep 9, 2020

Right. I think the information already exists in the implementation as well. We only need to construct the middle between start and end as returned by the pattern. The unknowns for me are:

  • Is this sound? I think so as both need to be char boundaries already.
  • Is this wanted? As as mentioned there are workarounds so it may be more useful to keep the interface for it being the 'common case'.
  • Is it ergonomic? The type signature is harder to read, quite a lot. For three items the order is less obvious.

@Nicholas-Baron
Copy link
Contributor

There is a split_inclusive function that does something akin to what you want. Maybe the function you want is something like a split_once_inclusive.

@HeroicKatora
Copy link
Contributor Author

It seems somewhat similar. Curiously it also discards some information, namely the start of the pattern match, but it wouldn't be too tragic as the reconstruction is much simpler. One could do .chars().last().unwrap().

@matklad
Copy link
Member

matklad commented Sep 24, 2020

It's interesting that basically all string APIs are underpowered in this way? Both find and split don't return the matched pattern.

@matklad
Copy link
Member

matklad commented May 13, 2021

Closing this issue, we stabilized split_once with the same API shape as the rest of string methods.

@matklad matklad closed this as completed May 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: An issue proposing an enhancement or a PR with one. 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

4 participants