Review feedback

astral-sh · Jun 17, 2024 · 35a2730 · 35a2730
1 parent 8b2c5bb
commit 35a2730
Show file tree

Hide file tree

Showing 4 changed files with 48 additions and 26 deletions.
diff --git a/crates/uv-toolchain/src/discovery.rs b/crates/uv-toolchain/src/discovery.rs
@@ -594,9 +594,9 @@ pub fn find_toolchains<'a>(
                 .filter(move |result| result_satisfies_system_python(result, system))
                 .filter(|result| match result {
                     Err(_) => true,
-                    Ok((_source, interpreter)) => {
-                        interpreter.implementation_name() == implementation.as_str()
-                    }
+                    Ok((_source, interpreter)) => interpreter
+                        .implementation_name()
+                        .eq_ignore_ascii_case(implementation.into()),
                 })
                 .map(|result| result.map(Toolchain::from_tuple).map(ToolchainResult::Ok))
         }),
@@ -608,7 +608,9 @@ pub fn find_toolchains<'a>(
                     Err(_) => true,
                     Ok((_source, interpreter)) => {
                         version.matches_interpreter(interpreter)
-                            && interpreter.implementation_name() == implementation.as_str()
+                            && interpreter
+                                .implementation_name()
+                                .eq_ignore_ascii_case(implementation.into())
                     }
                 })
                 .map(|result| result.map(Toolchain::from_tuple).map(ToolchainResult::Ok))
@@ -937,7 +939,7 @@ impl ToolchainRequest {
         for implementation in ImplementationName::iter() {
             if let Some(remainder) = value
                 .to_ascii_lowercase()
-                .strip_prefix(implementation.as_str())
+                .strip_prefix(Into::<&str>::into(implementation))
             {
                 // e.g. `pypy`
                 if remainder.is_empty() {
@@ -1069,12 +1071,14 @@ impl ToolchainRequest {
                 }
                 false
             }
-            ToolchainRequest::Implementation(implementation) => {
-                interpreter.implementation_name() == implementation.as_str()
-            }
+            ToolchainRequest::Implementation(implementation) => interpreter
+                .implementation_name()
+                .eq_ignore_ascii_case(implementation.into()),
             ToolchainRequest::ImplementationVersion(implementation, version) => {
                 version.matches_interpreter(interpreter)
-                    && interpreter.implementation_name() == implementation.as_str()
+                    && interpreter
+                        .implementation_name()
+                        .eq_ignore_ascii_case(implementation.into())
             }
             ToolchainRequest::Key(request) => request.satisfied_by_interpreter(interpreter),
         }
@@ -1126,7 +1130,7 @@ impl VersionRequest {
             .into_iter()
             .flat_map(move |implementation| {
                 let extension = std::env::consts::EXE_SUFFIX;
-                let name = implementation.as_str();
+                let name: &str = implementation.into();
                 let (python, python3) = if extension.is_empty() {
                     (Cow::Borrowed(name), Cow::Owned(format!("{name}3")))
                 } else {

diff --git a/crates/uv-toolchain/src/implementation.rs b/crates/uv-toolchain/src/implementation.rs
@@ -30,10 +30,37 @@ impl ImplementationName {
         NAMES.iter()
     }
 
-    pub fn as_str(self) -> &'static str {
+    pub fn pretty(self) -> &'static str {
         match self {
-            Self::CPython => "cpython",
-            Self::PyPy => "pypy",
+            Self::CPython => "CPython",
+            Self::PyPy => "PyPy",
+        }
+    }
+}
+
+impl LenientImplementationName {
+    pub fn pretty(&self) -> &str {
+        match self {
+            Self::Known(implementation) => implementation.pretty(),
+            Self::Unknown(name) => name,
+        }
+    }
+}
+
+impl From<&ImplementationName> for &'static str {
+    fn from(v: &ImplementationName) -> &'static str {
+        match v {
+            ImplementationName::CPython => "cpython",
+            ImplementationName::PyPy => "pypy",
+        }
+    }
+}
+
+impl<'a> From<&'a LenientImplementationName> for &'a str {
+    fn from(v: &'a LenientImplementationName) -> &'a str {
+        match v {
+            LenientImplementationName::Known(implementation) => implementation.into(),
+            LenientImplementationName::Unknown(name) => name,
         }
     }
 }
@@ -52,10 +79,7 @@ impl FromStr for ImplementationName {
 
 impl Display for ImplementationName {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        match self {
-            Self::CPython => f.write_str("cpython"),
-            Self::PyPy => f.write_str("pypy"),
-        }
+        f.write_str(self.into())
     }
 }
 

diff --git a/crates/uv-toolchain/src/lib.rs b/crates/uv-toolchain/src/lib.rs
@@ -248,7 +248,7 @@ mod tests {
                 )
                 .replace("{FULL_VERSION}", &version.to_string())
                 .replace("{VERSION}", &version.without_patch().to_string())
-                .replace("{IMPLEMENTATION}", implementation.as_str());
+                .replace("{IMPLEMENTATION}", (&implementation).into());
 
             fs_err::create_dir_all(path.parent().unwrap())?;
             fs_err::write(

diff --git a/crates/uv-toolchain/src/toolchain.rs b/crates/uv-toolchain/src/toolchain.rs
@@ -13,7 +13,7 @@ use crate::discovery::{
     ToolchainSources,
 };
 use crate::downloads::{DownloadResult, PythonDownload, PythonDownloadRequest};
-use crate::implementation::{LenientImplementationName};
+use crate::implementation::LenientImplementationName;
 use crate::managed::{InstalledToolchain, InstalledToolchains};
 use crate::platform::{Arch, Libc, Os};
 use crate::{Error, Interpreter, PythonVersion, ToolchainSource};
@@ -313,13 +313,7 @@ impl fmt::Display for ToolchainKey {
         write!(
             f,
             "{}-{}.{}.{}-{}-{}-{}",
-            self.implementation,
-            self.major,
-            self.minor,
-            self.patch,
-            self.os,
-            self.arch,
-            self.libc
+            self.implementation, self.major, self.minor, self.patch, self.os, self.arch, self.libc
         )
     }
 }