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.

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

make_ascii_lower/uppercase for Windows OsString? #69566

Closed
TyPR124 opened this issue Feb 28, 2020 · 1 comment · Fixed by #69937
Closed

make_ascii_lower/uppercase for Windows OsString? #69566

TyPR124 opened this issue Feb 28, 2020 · 1 comment · Fixed by #69937
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@TyPR124
Copy link
Contributor

TyPR124 commented Feb 28, 2020

The standard library seems to assume calling make_ascii_uppercase() on the underlying [u8] is safe on Windows, seen here:

impl From<OsString> for EnvKey {
fn from(k: OsString) -> Self {
let mut buf = k.into_inner().into_inner();
buf.make_ascii_uppercase();
EnvKey(FromInner::from_inner(FromInner::from_inner(buf)))
}
}

Would it be reasonable to add a make_ascii_uppercase() and/or make_ascii_lowercase() to Windows OsStrExt and/or OsStringExt to make this available to users of std?

Even if the internal representation of OsString changes down the road, I would assume the code above would still need to be replicated with the new representation, so it seems like it should be okay to have make_ascii_uppercase a stable/public API.

If this is a reasonable request, I am willing to make a PR with the appropriate methods implemented in the appropriate places.

@jonas-schievink jonas-schievink added C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Feb 28, 2020
@CryZe
Copy link
Contributor

CryZe commented Mar 1, 2020

Considering OsString is a superset of UTF-8, make_ascii_* should work across all platforms.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants