Support is_letter() on char
#38125
Support is_letter() on char
#38125
Conversation
|
r? @nrc (rust_highfive has picked a reviewer for you, use r? to override) |
| /// assert!(!c.is_letter()); | ||
| /// assert!(!'5'.is_letter()); | ||
| /// ``` | ||
| #[stable(feature = "rust1", since = "1.15.0")] |
There was a problem hiding this comment.
I wasn't sure about this annotation.
There was a problem hiding this comment.
- First of all new additions are almost never instantly stable. You want the
unstable. - You cannot use
rust1feature name. Rather you need a name unique for this feature.
There was a problem hiding this comment.
I see. Marked unstable and gave it its own name. There's no tracking issue for marking this stable, so set it to "0", as I saw with other features.
|
r? @aturon (this is libs stuff, but I've no idea who should review) |
estebank
force-pushed
the
is_letter
branch
2 times, most recently
from
f3a4c4e to
543b031
Compare
|
Thanks! Like some of the other unicode properties could the documentation also define what it means to be a letter? (basically just say what class it's in) Also, if you want to open an issue for this, feel free to just cc me and I'll tag it (and then you can update the reference here) |
| /// A `char` is alphabetic if it belongs to any of [the following | ||
| /// categories](http://unicode.org/reports/tr44/#Alphabetic): | ||
| /// | ||
| /// Lu | Ll | Lt | Lm | Lo | Nl | Other_Alphabetic |
There was a problem hiding this comment.
As per the errors in Travis CI, rustdoc will treat this indented section as code that should be tested. You could do something like this though:
```ignore
Lu | Ll | Lt | Lm | Lo | Nl | Other_Alphabetic
```
https://doc.rust-lang.org/book/documentation.html#running-documentation-tests
There was a problem hiding this comment.
Good call with your latest commit. ignore probably still does Rust syntax highlighting.
estebank
force-pushed
the
is_letter
branch
2 times, most recently
from
7d3e8c4 to
6541928
Compare
|
☔ The latest upstream changes (presumably #38049) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Why does this need to be in the standard library rather than in an external crate? This PR adds 4336 bytes of static data, and I don’t know if linkers are Sufficiently Advanced to remove this data from programs that don’t use it. In my opinion the fact that |
alexcrichton
added
the
T-libs-api
|
@SimonSapin I would consider "commonly used properties" to be candidates for being in std, but we shouldn't be exhausted by any means. This extra data should always be eliminated if not used, however. Due to our usage of @SimonSapin this is still unstable, though, but do you still think this should start externally first? |
Alright. My point is that, in the past, the bar for inclusion has been higher than having one person say "I think it makes sense to have this in std" and sending a PR. |
|
@SimonSapin sure yeah I agree, but other standard library having such properties is a good indication as well. |
|
Should I close this PR then? (Given that #38137 was closed.) |
|
Discussed during libs triage today and our conclusion was that is_alphabetic is close enough for now for "useful" and we'd prefer to hold off on adding a new category until there's perhaps more widespread motivation. As a result, closing. |
Fix #37069.