Skip to content

Rework char::eq_ignore_ascii_case parameter type #57227

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
czipperz opened this issue Dec 31, 2018 · 5 comments
Open

Rework char::eq_ignore_ascii_case parameter type #57227

czipperz opened this issue Dec 31, 2018 · 5 comments
Labels
A-str Area: str and String 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

@czipperz
Copy link
Contributor

Right now:

pub fn eq_ignore_ascii_case(&self, other: &char) -> bool {
    self.to_ascii_lowercase() == other.to_ascii_lowercase()
}

This means that 'a'.eq_ignore_ascii_case('b') doesn't compile. Instead the user must type 'a'.eq_ignore_ascii_case(&'b'). I would like to allow for both without breaking backwards compatibility.

One option would be to change the signature to the following:

pub fn eq_ignore_ascii_case<C: AsRef<char>>(&self, other: C) -> bool {
    self.to_ascii_lowercase() == other.as_ref().to_ascii_lowercase()
}

This would require us to implement AsRef<char> for char and &char to facilitate this.

@czipperz
Copy link
Contributor Author

In general I'm confused why all the char methods take references to char objects (especially &self) whereas the rest of the primitives take themselves by value.

@czipperz
Copy link
Contributor Author

czipperz commented Dec 31, 2018

Looking over documentation more, Borrow seems like a better fit than AsRef:

pub fn eq_ignore_ascii_case<C: Borrow<char>>(&self, other: C) -> bool {
    self.to_ascii_lowercase() == other.borrow().to_ascii_lowercase()
}

@frewsxcv
Copy link
Member

In general I'm confused why all the char methods take references to char objects (especially &self) whereas the rest of the primitives take themselves by value.

Before the eq_ignore_ascii_case methods were added directly on the char type, the functionality was provided by this trait. Since it's a generic trait, the function definition had a &Self type for the other param. At some point, it was decided the trait was unnecessary and (I'm guessing for backwards compatibility reasons) the function signatures remained the same, which is why we have &chars now.

@estebank estebank added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Dec 31, 2018
@ExpHP
Copy link
Contributor

ExpHP commented Jan 3, 2019

See also #57307 for more unfortunate "fallout" from the backwards compatibility concerns of AsciiExt

@steveklabnik
Copy link
Member

Triage: no changes

@Enselic Enselic added A-str Area: str and String C-enhancement Category: An issue proposing an enhancement or a PR with one. labels Dec 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-str Area: str and String 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

6 participants