-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
Implement PartialEq between &str and OsString #51178
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @bluss (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
The job Click to expand the log.
I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Not sure which stability attributes would be right for this change. |
@GabrielMajeri what you have is fine, although you should change |
Ping from triage @bluss! This PR needs your review. |
Can you add a |
@rust-lang/lang should this be instastable on 1.28? Other than that lgtm. |
That's really a @rust-lang/libs decision, I think. |
Was it formally decided elsewhere to add these impls? I don't see much discussion on #49854 |
I'm nominated for someone on @rust-lang/libs to render a decision of some kind. |
We have all of the following: // & <-> Owned are comparable
impl<'a> PartialEq<&'a str> for String
impl<'a> PartialEq<String> for &'a str
// utf8 <-> OS are comparable
impl PartialEq<str> for OsString
impl PartialEq<OsString> for str So I believe it makes sense to provide these as well. impl<'a> PartialEq<&'a str> for OsString
impl<'a> PartialEq<OsString> for &'a str @rfcbot fcp merge |
Team member @dtolnay has proposed to merge this. The next step is review by the rest of the tagged teams: No concerns currently listed. Once a majority of reviewers approve (and none object), 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. |
🔔 This is now entering its final comment period, as per the review above. 🔔 |
The final comment period, with a disposition to merge, as per the review above, is now complete. |
@bors r+ |
📌 Commit 1e85cb3a595439e9f16163553725099e0c0da60f has been approved by |
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
Allows for example `os_string == "something"`
@bors r=SimonSapin |
📌 Commit fbd3c92 has been approved by |
…onSapin Implement PartialEq between &str and OsString This fixes rust-lang#49854. It allows equality comparison between `OsString` values and `str` references, such as `os_string == "something"`.
Implement PartialEq between &str and OsString This fixes #49854. It allows equality comparison between `OsString` values and `str` references, such as `os_string == "something"`.
☀️ Test successful - status-appveyor, status-travis |
Version 1.29.0 (2018-09-13) ========================== Compiler -------- - [Bumped minimum LLVM version to 5.0.][51899] - [Added `powerpc64le-unknown-linux-musl` target.][51619] - [Added `aarch64-unknown-hermit` and `x86_64-unknown-hermit` targets.][52861] Libraries --------- - [`Once::call_once` now no longer requires `Once` to be `'static`.][52239] - [`BuildHasherDefault` now implements `PartialEq` and `Eq`.][52402] - [`Box<CStr>`, `Box<OsStr>`, and `Box<Path>` now implement `Clone`.][51912] - [Implemented `PartialEq<&str>` for `OsString` and `PartialEq<OsString>` for `&str`.][51178] - [`Cell<T>` now allows `T` to be unsized.][50494] - [`SocketAddr` is now stable on Redox.][52656] Stabilized APIs --------------- - [`Arc::downcast`] - [`Iterator::flatten`] - [`Rc::downcast`] Cargo ----- - [Cargo can silently fix some bad lockfiles ][cargo/5831] You can use `--locked` to disable this behaviour. - [`cargo-install` will now allow you to cross compile an install using `--target`][cargo/5614] - [Added the `cargo-fix` subcommand to automatically move project code from 2015 edition to 2018.][cargo/5723] Misc ---- - [`rustdoc` now has the `--cap-lints` option which demotes all lints above the specified level to that level.][52354] For example `--cap-lints warn` will demote `deny` and `forbid` lints to `warn`. - [`rustc` and `rustdoc` will now have the exit code of `1` if compilation fails, and `101` if there is a panic.][52197] - [A preview of clippy has been made available through rustup.][51122] You can install the preview with `rustup component add clippy-preview` Compatibility Notes ------------------- - [`str::{slice_unchecked, slice_unchecked_mut}` are now deprecated.][51807] Use `str::get_unchecked(begin..end)` instead. - [`std::env::home_dir` is now deprecated for its unintuitive behaviour.][51656] Consider using the `home_dir` function from https://crates.io/crates/dirs instead. - [`rustc` will no longer silently ignore invalid data in target spec.][52330] [52861]: rust-lang/rust#52861 [52656]: rust-lang/rust#52656 [52239]: rust-lang/rust#52239 [52330]: rust-lang/rust#52330 [52354]: rust-lang/rust#52354 [52402]: rust-lang/rust#52402 [52103]: rust-lang/rust#52103 [52197]: rust-lang/rust#52197 [51807]: rust-lang/rust#51807 [51899]: rust-lang/rust#51899 [51912]: rust-lang/rust#51912 [51511]: rust-lang/rust#51511 [51619]: rust-lang/rust#51619 [51656]: rust-lang/rust#51656 [51178]: rust-lang/rust#51178 [51122]: rust-lang/rust#51122 [50494]: rust-lang/rust#50494 [cargo/5614]: rust-lang/cargo#5614 [cargo/5723]: rust-lang/cargo#5723 [cargo/5831]: rust-lang/cargo#5831 [`Arc::downcast`]: https://doc.rust-lang.org/std/sync/struct.Arc.html#method.downcast [`Iterator::flatten`]: https://doc.rust-lang.org/std/iter/trait.Iterator.html#method.flatten [`Rc::downcast`]: https://doc.rust-lang.org/std/rc/struct.Rc.html#method.downcast
This fixes #49854.
It allows equality comparison between
OsString
values andstr
references, such asos_string == "something"
.