Skip to content

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

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

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ChaiTRex
Copy link
Contributor

@ChaiTRex ChaiTRex commented Jul 5, 2025

  • 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

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

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.

@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.

@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.

@rustbot
Copy link
Collaborator

rustbot commented Aug 5, 2025

⚠️ Warning ⚠️

  • The following commits have merge commits (commits with multiple parents) in your changes. We have a no merge policy so these commits will need to be removed for this pull request to be merged.

    You can start a rebase with the following commands:

    $ # rebase
    $ git pull --rebase https://github.com/rust-lang/rust.git master
    $ git push --force-with-lease
    

@rustbot rustbot added the has-merge-commits PR has merge commits, merge with caution. label Aug 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
has-merge-commits PR has merge commits, merge with caution. S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. 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
Development

Successfully merging this pull request may close these issues.

5 participants