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

Add is_whitespace and is_alphanumeric to str. #49381

Merged
merged 2 commits into from
Mar 28, 2018
Merged

Add is_whitespace and is_alphanumeric to str. #49381

merged 2 commits into from
Mar 28, 2018

Conversation

withoutboats
Copy link
Contributor

The other methods from UnicodeStr are already stable inherent
methods on str, but these have not been included.

r? @SimonSapin

Add is_whitespace and is_alphanumeric to str.
1e2458e 
The other methods from `UnicodeStr` are already stable inherent
methods on str, but these have not been included.
@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 26, 2018
@SimonSapin
Copy link
Contributor

Looks good.

@bors: r+

@bors
Copy link
Contributor

bors commented Mar 26, 2018

📌 Commit 1e2458e has been approved by SimonSapin

@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 Mar 26, 2018
@SimonSapin SimonSapin added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Mar 26, 2018
@withoutboats
Copy link
Contributor Author

@bors r=SimonSapin

@bors
Copy link
Contributor

bors commented Mar 26, 2018

📌 Commit 5fc7e0a has been approved by SimonSapin

@kennytm
Copy link
Member

kennytm commented Mar 27, 2018

@bors rollup

kennytm added a commit to kennytm/rust that referenced this pull request Mar 27, 2018
@kennytm
Rollup merge of rust-lang#49381 - withoutboats:str_unicode, r=SimonSapin
1c45f6c 
Add is_whitespace and is_alphanumeric to str.

The other methods from `UnicodeStr` are already stable inherent
methods on str, but these have not been included.

r? @SimonSapin
@kennytm kennytm mentioned this pull request Mar 27, 2018
Merged
@CryZe
Copy link
Contributor

CryZe commented Mar 27, 2018 •
edited
Loading

Is there a reason why these methods are not available on core? Is there anything inherently preventing them from being available there?

@SimonSapin
Copy link
Contributor

These methods are based on (potentially-large) Unicode tables. The std_unicode crate exists to contain these tables, but I think we might be able to move all of this into libcore? That’s out of scope for this PR though.

bors added a commit that referenced this pull request Mar 27, 2018
@bors
Auto merge of #49406 - kennytm:rollup, r=kennytm
35f368f 
Rollup of 11 pull requests

- Successful merges: #48639, #49223, #49333, #49369, #49381, #49395, #49399, #49401, #49417, #49202, #49426
- Failed merges:
bors added a commit that referenced this pull request Mar 28, 2018
@bors
Auto merge of #49406 - kennytm:rollup, r=kennytm
59ec9bf 
Rollup of 11 pull requests

- Successful merges: #48639, #49223, #49333, #49369, #49381, #49395, #49399, #49401, #49417, #49202, #49426
- Failed merges:
@bors bors merged commit 5fc7e0a into rust-lang:master Mar 28, 2018
@LukasKalbertodt
Copy link
Member

When we were talking about which methods from AsciiExt to copy to str, we purposefully excluded methods like is_ascii_alphabetic because we preferred the explicit version (cc @dtolnay). I guess we should be consistent here... 😕

I mean, the meaning of s.is_whitespace() is a lot clearer than s.is_ascii_hexdigit(), but still. Just wanted to bring this up, what do you think?

@SimonSapin
Copy link
Contributor

That’s a good point. @rust-lang/libs, any opinion?

@BurntSushi
Copy link
Member

I also personally like the explicit variant to the exclusion of the higher level methods. Basically for reasons already stated.

@dtolnay
Copy link
Member

dtolnay commented Apr 4, 2018

I would prefer not to have these methods.

@dtolnay dtolnay mentioned this pull request Apr 4, 2018
Closed
@dtolnay
Copy link
Member

dtolnay commented Apr 4, 2018

I filed #49657 with regression-from-stable-to-beta tag to follow up. @LukasKalbertodt I'm not sure how you caught this but thanks! 🍻

@LukasKalbertodt
Copy link
Member

@dtolnay In case that's not a rhetoric question: this PR was linked on TWiR. I only landed one big PR in this repo (the AsciiExt thing) and I thus take a closer look at everything that is vaguely related to my PR 😁

@alexcrichton alexcrichton mentioned this pull request Apr 19, 2018
Merged
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Apr 19, 2018
@alexcrichton
Tweak some stabilizations in libstd
ca79ba3 
This commit tweaks a few stable APIs in the `beta` branch before they hit
stable. The `str::is_whitespace` and `str::is_alphanumeric` functions were
deleted (added in rust-lang#49381, issue at rust-lang#49657). The `and_modify` APIs added
in rust-lang#44734 were altered to take a `FnOnce` closure rather than a `FnMut` closure.

Closes rust-lang#49581
Closes rust-lang#49657
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Apr 19, 2018
@alexcrichton
[beta] Tweak some stabilizations in libstd
222ca2d 
This commit tweaks a few stable APIs in the `beta` branch before they hit
stable. The `str::is_whitespace` and `str::is_alphanumeric` functions were
deleted (added in rust-lang#49381, issue at rust-lang#49657). The `and_modify` APIs added
in rust-lang#44734 were altered to take a `FnOnce` closure rather than a `FnMut` closure.

Closes rust-lang#49581
Closes rust-lang#49657
bors added a commit that referenced this pull request Apr 20, 2018
@bors
Auto merge of #50088 - alexcrichton:std-tweaks, r=sfackler
a10bb6b 
Tweak some stabilizations in libstd

This commit tweaks a few stable APIs in the `beta` branch before they hit
stable. The `str::is_whitespace` and `str::is_alphanumeric` functions were
deleted (added in #49381, issue at #49657). The `and_modify` APIs added
in #44734 were altered to take a `FnOnce` closure rather than a `FnMut` closure.

Closes #49581
Closes #49657
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@SimonSapin SimonSapin

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. 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

Successfully merging this pull request may close these issues.
9 participants
@withoutboats @SimonSapin @bors @kennytm @CryZe @LukasKalbertodt @BurntSushi @dtolnay @rust-highfive