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.

Sign up for GitHub

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

Jump to bottom

impl PartialOrd<str> for String #82990

Open
tmandry opened this issue Mar 10, 2021 · 1 comment
Open

impl PartialOrd<str> for String #82990

tmandry opened this issue Mar 10, 2021 · 1 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.

Comments

@tmandry
Copy link
Member

tmandry commented Mar 10, 2021 

    let a = "a".to_string();
    // compiles
    println!("{}", "b" > &a);
    // doesn't compile
    // println!("{}", &a < "b");

We have impl PartialEq<str> for String and impl PartialEq<&str> for String, but no matching impls for PartialOrd. Can we add these impls to round it out?
@tmandry tmandry added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-str Area: str and String labels Mar 10, 2021
@JohnTitor JohnTitor added C-enhancement Category: An issue proposing an enhancement or a PR with one. and removed C-bug Category: This is a bug. labels Mar 18, 2021
@ecton
Copy link
Contributor

ecton commented May 29, 2023

I ran into this today. From looking at this, Vec and slice need to have this implemented as well, and then String can use its implementation. OsStr/OsString and therefore Path/PathBuf already implement PartialOrd.

Would a pull request for these missing implementations be accepted or does some approval process need to happen before I submit a PR? I've never contributed to any of the official Rust projects.

ecton added a commit to khonsulabs/kempt that referenced this issue May 29, 2023
@ecton
Improved the entry api + refactored sorting
0cef67f 
This first goal of the commit was to make it so that `&str` could be
used to search for `String` keys. All of my testing had been against
numeric keys, and I had wrongly assumed that `String` implemented
`PartialOrd<str>` (<rust-lang/rust#82990>).

The new `Sort` trait works around this in better ways than I had
expected -- not only can it provide automatic implementations for all
Ord/PartialOrd implementers, it also can have specialized
implementations for String/Vec. This means that end users will only have
to implement Ord/PartialOrd for their key types, and almost no one
should need to manually implement this trait.

The next goal was to change the entry API into a form that accepts owned or
borrowed representations of the `Key` type and delays the conversion to
the owned form until after the key isn't found in the collection. The
net effect is that passing a str into entry() will use the borrowed
representation to search for the key, and only convert to a String if
the key isn't found. And, when the key is already in its owned
representation, no extra clone is made.
@ecton ecton mentioned this issue May 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
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
Milestone
No milestone
Development

No branches or pull requests
3 participants
@ecton @tmandry @JohnTitor