chipsenkbeil / typed-path

Provides typed variants of Path and PathBuf for Unix and Windows
38 stars 6 forks source link

add back missing derives #27

Closed stevefan1999-personal closed 2 months ago

chipsenkbeil commented 2 months ago

@stevefan1999-personal is the implementation of PartialOrd and Ord what you would expect? Namely that variants are compared for order first, and then only if they are the same are the fields compared. Would look something like this, I believe.

impl PartialOrd for TypedPathBuf {
    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
        match (self, other) {
            (TypedPathBuf::Unix(lhs), TypedPathBuf::Unix(rhs)) => lhs.partial_cmp(rhs),
            (TypedPathBuf::Windows(lhs), TypedPathBuf::Windows(rhs)) => lhs.partial_cmp(rhs),
            (TypedPathBuf::Unix(_), TypedPathBuf::Windows(_)) => Some(std::cmp::Ordering::Less),
            (TypedPathBuf::Windows(_), TypedPathBuf::Unix(_)) => Some(std::cmp::Ordering::Greater),
        }
    }
}

impl Ord for TypedPathBuf {
    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
        match (self, other) {
            (TypedPathBuf::Unix(lhs), TypedPathBuf::Unix(rhs)) => lhs.cmp(rhs),
            (TypedPathBuf::Windows(lhs), TypedPathBuf::Windows(rhs)) => lhs.cmp(rhs),
            (TypedPathBuf::Unix(_), TypedPathBuf::Windows(_)) => std::cmp::Ordering::Less,
            (TypedPathBuf::Windows(_), TypedPathBuf::Unix(_)) => std::cmp::Ordering::Greater),
        }
    }
}
stevefan1999-personal commented 2 months ago

Huh, interesting, I never through about the orderings of Unix path and Windows path...but if you don't have PartialOrd then you can't use TypedPathBuf as a key for BTreeMap

chipsenkbeil commented 2 months ago

We're in pre 1.0 release, so I'm fine bringing this in. Can always change the behavior later if needed. Thanks for the contribution!

stevefan1999-personal commented 2 months ago

We're in pre 1.0 release, so I'm fine bringing this in. Can always change the behavior later if needed. Thanks for the contribution!

Thanks! Now I can use this with BTreeMap in no_std context

chipsenkbeil commented 2 months ago

@stevefan1999-personal you'll find this included in the 0.9.1 release :smile: