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

Tracking Issue for ASCII methods on OsStr #70516

Closed
4 tasks done
TyPR124 opened this issue Mar 28, 2020 · 11 comments · Fixed by #80193
Closed
4 tasks done

Tracking Issue for ASCII methods on OsStr #70516

TyPR124 opened this issue Mar 28, 2020 · 11 comments · Fixed by #80193
Labels
A-FFI Area: Foreign function interface (FFI) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. 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 Mar 28, 2020

Feature gate: #![feature(osstring_ascii)]

Public API

impl OsStr {
    pub fn make_ascii_lowercase(&mut self);
    pub fn make_ascii_uppercase(&mut self);
    pub fn to_ascii_lowercase(&self) -> OsString;
    pub fn to_ascii_uppercase(&self) -> OsString;
    pub fn is_ascii(&self) -> bool;
    pub fn eq_ignore_ascii_case<S: AsRef<OsStr>>(&self, other: S) -> bool;
}

Steps / History

Unresolved Questions

  • None yet.
@TyPR124 TyPR124 added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Mar 28, 2020
@jonas-schievink jonas-schievink added B-unstable Blocker: Implemented in the nightly compiler and unstable. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Mar 28, 2020
@dtolnay dtolnay changed the title Tracking Issue for 'osstring_ascii' Tracking Issue for ASCII methods on OsStr Mar 28, 2020
@matklad
Copy link
Member

matklad commented Jun 12, 2020

Just needed OsStr::to_ascii_lowercase to manipulate paths on windows -- I need to lower case driver letter, C:\Program Files -> c:\Program Files.

@KodrAus KodrAus added A-FFI Area: Foreign function interface (FFI) I-nominated Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@jplatte
Copy link
Contributor

jplatte commented Feb 2, 2021

Currently, SQLx contains

env::var("SQLX_OFFLINE")
    .map(|s| s.eq_ignore_ascii_case("true") || s == "1")
    .unwrap_or(false)

The UTF-8 check that is included in there, while not a problem in practice, seems pointless. Changing to var_os lands me here because of the eq_ignore_ascii_case. Also, if somebody did a similar thing with .ok() instead of .unwrap_or(...), the var_os version could drop that and would then be a bit less noisy.

@tesuji
Copy link
Contributor

tesuji commented Feb 2, 2021

#80193 is a stabilization PR.
Since lib team wants to do fcp in the tracking issue. @m-ou-se could you start one ?

@m-ou-se
Copy link
Member

m-ou-se commented Feb 2, 2021

@rfcbot merge

@rfcbot
Copy link

rfcbot commented Feb 2, 2021

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

Concerns:

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Feb 2, 2021
@dtolnay
Copy link
Member

dtolnay commented Feb 2, 2021

Slight question about eq_ignore_ascii_case -- normally when dealing with AsRef args, they come in without an explicit reference in the signature (https://doc.rust-lang.org/std/process/struct.Command.html#method.new), except when necessary to support a lifetime in the return value (https://doc.rust-lang.org/std/path/struct.Path.html#method.new). Currently https://doc.rust-lang.org/1.49.0/std/ffi/struct.OsStr.html#method.eq_ignore_ascii_case is declared as:

pub fn eq_ignore_ascii_case<S: ?Sized + AsRef<OsStr>>(&self, other: &S) -> bool

Is this supposed to be fn eq_ignore_ascii_case<S: AsRef<OsStr>>(&self, other: S) -> bool instead?

@rfcbot concern eq_ignore_ascii_case &S

@TyPR124
Copy link
Contributor Author

TyPR124 commented Feb 3, 2021

Is this supposed to be fn eq_ignore_ascii_case<S: AsRef<OsStr>>(&self, other: S) -> bool instead?

I think it should be. I'm not sure why I wrote it like I did originally, but this makes more sense to me.

I can make that PR if needed. Or can it be done in the stabilization PR? Not sure what the process is from here.

TyPR124 added a commit to TyPR124/rust that referenced this issue Feb 3, 2021
Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
@dtolnay
Copy link
Member

dtolnay commented Feb 3, 2021

@rfcbot resolve eq_ignore_ascii_case &S
@rfcbot reviewed
Fixed in #81710.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Feb 3, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Feb 4, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Feb 4, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
m-ou-se added a commit to m-ou-se/rust that referenced this issue Feb 4, 2021
OsStr eq_ignore_ascii_case takes arg by value

Per a comment on rust-lang#70516 this changes `eq_ignore_ascii_case` to take the generic parameter `S: AsRef<OsStr>` by value instead of by reference.

This is technically a breaking change to an unstable method. I think the only way it would break is if you called this method with an explicit type parameter, ie `my_os_str.eq_ignore_ascii_case::<str>("foo")` becomes `my_os_str.eq_ignore_ascii_case::<&str>("foo")`.

Besides that, I believe it is overall more flexible since it can now take an owned `OsString` for example.

If this change should be made in some other PR (like rust-lang#80193) then please just close this.
@rfcbot rfcbot added the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label Feb 17, 2021
@rfcbot
Copy link

rfcbot commented Feb 17, 2021

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label Feb 17, 2021
@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label Feb 27, 2021
@rfcbot
Copy link

rfcbot commented Feb 27, 2021

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot added to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Feb 27, 2021
@apiraino apiraino removed the to-announce Announce this issue on triage meeting label Mar 4, 2021
@fogti
Copy link
Contributor

fogti commented Mar 6, 2021

@TyPR124 you can tick the FCP box, as the FCP is complete.

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Mar 21, 2021
…-ou-se

stabilize `feature(osstring_ascii)`

This PR stabilizes `feature(osstring_ascii)`.

Fixes rust-lang#70516.
@bors bors closed this as completed in e9398bc Mar 22, 2021
ericswanson-dfinity added a commit to dfinity/cdk-rs that referenced this issue Aug 16, 2021
beta.4 introduced errors like these:

error[E0658]: arbitrary expressions in key-value attributes are unstable
 --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/lib.rs:8:10
  |
8 | #![doc = include_str!("../README.md")]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #78835 <rust-lang/rust#78835> for more information

error[E0658]: use of unstable library feature 'osstring_ascii'
   --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/parse/matches/matched_arg.rs:130:19
    |
130 |                 v.eq_ignore_ascii_case(val)
    |                   ^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #70516 <rust-lang/rust#70516> for more information
ericswanson-dfinity added a commit to dfinity/cdk-rs that referenced this issue Aug 17, 2021
beta.4 introduced errors like these:

error[E0658]: arbitrary expressions in key-value attributes are unstable
 --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/lib.rs:8:10
  |
8 | #![doc = include_str!("../README.md")]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #78835 <rust-lang/rust#78835> for more information

error[E0658]: use of unstable library feature 'osstring_ascii'
   --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/parse/matches/matched_arg.rs:130:19
    |
130 |                 v.eq_ignore_ascii_case(val)
    |                   ^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #70516 <rust-lang/rust#70516> for more information
ericswanson-dfinity added a commit to dfinity/agent-rs that referenced this issue Aug 23, 2021
Avoids errors like this:

error[E0658]: arbitrary expressions in key-value attributes are unstable
 --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/lib.rs:8:10
  |
8 | #![doc = include_str!("../README.md")]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #78835 <rust-lang/rust#78835> for more information

   Compiling binread v2.1.1
   Compiling logos v0.12.0
error[E0658]: use of unstable library feature 'osstring_ascii'
   --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/parse/matches/matched_arg.rs:130:19
    |
130 |                 v.eq_ignore_ascii_case(val)
    |                   ^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #70516 <rust-lang/rust#70516> for more information
ericswanson-dfinity added a commit to dfinity/agent-rs that referenced this issue Aug 23, 2021
Avoids errors like this:

error[E0658]: arbitrary expressions in key-value attributes are unstable
 --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/lib.rs:8:10
  |
8 | #![doc = include_str!("../README.md")]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #78835 <rust-lang/rust#78835> for more information

   Compiling binread v2.1.1
   Compiling logos v0.12.0
error[E0658]: use of unstable library feature 'osstring_ascii'
   --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/parse/matches/matched_arg.rs:130:19
    |
130 |                 v.eq_ignore_ascii_case(val)
    |                   ^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #70516 <rust-lang/rust#70516> for more information
softpluspro12 pushed a commit to softpluspro12/rust-cdk that referenced this issue Jun 30, 2022
beta.4 introduced errors like these:

error[E0658]: arbitrary expressions in key-value attributes are unstable
 --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/lib.rs:8:10
  |
8 | #![doc = include_str!("../README.md")]
  |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  |
  = note: see issue #78835 <rust-lang/rust#78835> for more information

error[E0658]: use of unstable library feature 'osstring_ascii'
   --> /Users/ericswanson/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/clap-3.0.0-beta.4/src/parse/matches/matched_arg.rs:130:19
    |
130 |                 v.eq_ignore_ascii_case(val)
    |                   ^^^^^^^^^^^^^^^^^^^^
    |
    = note: see issue #70516 <rust-lang/rust#70516> for more information
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-FFI Area: Foreign function interface (FFI) B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Small Libs issues that are considered "small" or self-contained Libs-Tracked Libs issues that are tracked on the team's project board. 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.