Skip to content

Add ASCII-related methods from u8 and MIN/MAX to core::ascii::Char#143467

Merged
bors merged 2 commits intorust-lang:masterfrom
ChaiTRex:ascii_char_is_ascii
Aug 13, 2025
Merged

Add ASCII-related methods from u8 and MIN/MAX to core::ascii::Char#143467
bors merged 2 commits intorust-lang:masterfrom
ChaiTRex:ascii_char_is_ascii

Conversation

@ChaiTRex
Copy link
Contributor

@ChaiTRex ChaiTRex commented Jul 5, 2025 •
edited by rustbot
Loading

  • Add ASCII-related methods from u8 to core::ascii::Char.
  • Add core::ascii::Char::MIN and core::ascii::Char::MAX.

Tracking issue #110998.

Can someone please ping @rust-lang/libs-api? These additions were not in the original ACP (rust-lang/libs-team#179).

@rustbot
Copy link
Collaborator

rustbot commented Jul 5, 2025

r? @ibraheemdev

rustbot has assigned @ibraheemdev.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Jul 5, 2025
@ibraheemdev
Copy link
Member

ibraheemdev commented Jul 5, 2025

r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Jul 5, 2025
@rustbot rustbot assigned BurntSushi and unassigned ibraheemdev Jul 5, 2025
@tgross35
Copy link
Contributor

tgross35 commented Jul 26, 2025 •
edited
Loading

Suggested an ACP on Zulip. A few notes

  • I don't know if we need to exactly replicate naming from char/u8 here. E.g. I think the compact and non-redundant names to_uppercase and is_digit outweighs consistency with to_ascii_uppercase and is_ascii_digit.
  • Similarly, I don't know when is_ascii would be needed except for somebody migrating from char/u8 to AsciiChar. In which case, they should remove whatever they're doing with the check rather than accidentally using this.
  • All of these could take self/Self rather than &self/&Self since it's Copy (and trivially so). Arguably it should have been the same for u8 and char, not sure what to do with consistency here.

@ChaiTRex ChaiTRex mentioned this pull request Jul 26, 2025
Closed
@tgross35
Copy link
Contributor

tgross35 commented Aug 5, 2025

ACP was accepted with changes, for that:
@rustbot author

r? tgross35

@rustbot rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 5, 2025
@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2025

Reminder, once the PR becomes ready for a review, use @rustbot ready.

@rustbot rustbot assigned tgross35 and unassigned BurntSushi Aug 5, 2025
@rustbot

This comment has been minimized.

Sign in to view
@ChaiTRex ChaiTRex force-pushed the ascii_char_is_ascii branch from 381b5b3 to 16b8f73 Compare August 5, 2025 18:46
@rustbot

This comment has been minimized.

Sign in to view
@rustbot

This comment has been minimized.

Sign in to view
@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Aug 5, 2025
@ChaiTRex ChaiTRex force-pushed the ascii_char_is_ascii branch from 67cd427 to d811581 Compare August 5, 2025 21:44
@rustbot rustbot removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. has-merge-commits PR has merge commits, merge with caution. labels Aug 5, 2025
@ChaiTRex
Copy link
Contributor Author

ChaiTRex commented Aug 6, 2025

@rustbot ready

@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 6, 2025
tgross35
tgross35 approved these changes Aug 13, 2025
View reviewed changes
library/core/src/ascii/ascii_char.rs
without modifying the original"]
#[unstable(feature = "ascii_char", issue = "110998")]
#[inline]
pub fn escape_ascii(self) -> super::EscapeDefault {
Copy link
Contributor

@tgross35 tgross35 Aug 13, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just noting that the function on char is called escape_unicode, so keeping ascii in the name seems fine here

Copy link
Contributor Author

@ChaiTRex ChaiTRex Aug 13, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To add to that, I left that in for that reason and also because it clearly distinguishes its use of ASCII escapes (\x04) rather than Unicode escapes (\u{4}). I should probably better document what output should be expected for that particular character by adding a few lines to the doctest in another pull request.

@tgross35
Copy link
Contributor

tgross35 commented Aug 13, 2025

Looks good, thank you!

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Aug 13, 2025

📌 Commit d811581 has been approved by tgross35

It is now in the queue for this repository.

@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 Aug 13, 2025
@Kobzol Kobzol mentioned this pull request Aug 13, 2025
Merged
bors added a commit that referenced this pull request Aug 13, 2025
@bors
Auto merge of #145334 - Kobzol:rollup-fs5a133, r=Kobzol
1c9952f 
Rollup of 11 pull requests

Successful merges:

 - #143467 (Add ASCII-related methods from `u8` and `MIN`/`MAX` to `core::ascii::Char`)
 - #144519 (Constify `SystemTime` methods)
 - #144642 (editorconfig: don't trim trailing whitespace in tests)
 - #144870 (Stabilize `path_file_prefix` feature)
 - #145269 (Deprecate RUST_TEST_* env variables)
 - #145274 (Remove unused `#[must_use]`)
 - #145289 (chore(ci): upgrade checkout to v5)
 - #145303 (Docs: Link to payload_as_str() from payload().)
 - #145308 (Adjust documentation of `dangling`)
 - #145320 (Allow cross-compiling the Cranelift dist component)
 - #145325 (Add `cast_init` and `cast_uninit` methods for pointers)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit fb9cd24 into rust-lang:master Aug 13, 2025
10 checks passed
@rustbot rustbot added this to the 1.91.0 milestone Aug 13, 2025
rust-timer added a commit that referenced this pull request Aug 13, 2025
@rust-timer
Unrolled build for #143467
4dafa65 
Rollup merge of #143467 - ChaiTRex:ascii_char_is_ascii, r=tgross35

Add ASCII-related methods from `u8` and `MIN`/`MAX` to `core::ascii::Char`

* Add ASCII-related methods from `u8` to `core::ascii::Char`.
* Add `core::ascii::Char::MIN` and `core::ascii::Char::MAX`.
github-actions bot pushed a commit to model-checking/verify-rust-std that referenced this pull request Aug 18, 2025
@Kobzol
Rollup merge of rust-lang#143467 - ChaiTRex:ascii_char_is_ascii, r=tg…
ff12cbd 
…ross35

Add ASCII-related methods from `u8` and `MIN`/`MAX` to `core::ascii::Char`

* Add ASCII-related methods from `u8` to `core::ascii::Char`.
* Add `core::ascii::Char::MIN` and `core::ascii::Char::MAX`.
@ChaiTRex ChaiTRex mentioned this pull request Aug 23, 2025
Open
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tgross35 tgross35 tgross35 approved these changes

Assignees

@tgross35 tgross35

Labels

S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Milestone

1.91.0

Development

Successfully merging this pull request may close these issues.

6 participants

@ChaiTRex @rustbot @ibraheemdev @tgross35 @bors @BurntSushi

Comments