diff --git a/crates/uv-python/src/discovery.rs b/crates/uv-python/src/discovery.rs index 51151f3feb74..af382a92a087 100644 --- a/crates/uv-python/src/discovery.rs +++ b/crates/uv-python/src/discovery.rs @@ -2422,7 +2422,7 @@ impl DiscoveryPreferences { .map(ToString::to_string) .collect::>(); match self.environment_preference { - EnvironmentPreference::Any => conjunction( + EnvironmentPreference::Any => disjunction( &["virtual environments"] .into_iter() .chain(python_sources.iter().map(String::as_str)) @@ -2430,23 +2430,23 @@ impl DiscoveryPreferences { ), EnvironmentPreference::ExplicitSystem => { if request.is_explicit_system() { - conjunction( + disjunction( &["virtual environments"] .into_iter() .chain(python_sources.iter().map(String::as_str)) .collect::>(), ) } else { - conjunction(&["virtual environments"]) + disjunction(&["virtual environments"]) } } - EnvironmentPreference::OnlySystem => conjunction( + EnvironmentPreference::OnlySystem => disjunction( &python_sources .iter() .map(String::as_str) .collect::>(), ), - EnvironmentPreference::OnlyVirtual => conjunction(&["virtual environments"]), + EnvironmentPreference::OnlyVirtual => disjunction(&["virtual environments"]), } } } @@ -2471,8 +2471,9 @@ impl fmt::Display for PythonNotFound { } /// Join a series of items with `or` separators, making use of commas when necessary. -fn conjunction(items: &[&str]) -> String { +fn disjunction(items: &[&str]) -> String { match items.len() { + 0 => String::new(), 1 => items[0].to_string(), 2 => format!("{} or {}", items[0], items[1]), _ => { diff --git a/crates/uv/src/commands/mod.rs b/crates/uv/src/commands/mod.rs index b660d6dc46a4..2ef86ee943f0 100644 --- a/crates/uv/src/commands/mod.rs +++ b/crates/uv/src/commands/mod.rs @@ -257,3 +257,39 @@ impl<'a> OutputWriter<'a> { Ok(()) } } + +/// Given a list of names, return a conjunction of the names (e.g., "Alice, Bob, and Charlie"). +pub(super) fn conjunction(names: Vec) -> String { + let mut names = names.into_iter(); + let first = names.next(); + let last = names.next_back(); + match (first, last) { + (Some(first), Some(last)) => { + let mut result = first; + let mut comma = false; + for name in names { + result.push_str(", "); + result.push_str(&name); + comma = true; + } + if comma { + result.push_str(", and "); + } else { + result.push_str(" and "); + } + result.push_str(&last); + result + } + (Some(first), None) => first, + _ => String::new(), + } +} + +/// Capitalize the first letter of a string. +pub(super) fn capitalize(s: &str) -> String { + let mut chars = s.chars(); + match chars.next() { + None => String::new(), + Some(c) => c.to_uppercase().collect::() + chars.as_str(), + } +} diff --git a/crates/uv/src/commands/project/mod.rs b/crates/uv/src/commands/project/mod.rs index 5dce4614bbd0..054035f1477e 100644 --- a/crates/uv/src/commands/project/mod.rs +++ b/crates/uv/src/commands/project/mod.rs @@ -45,7 +45,7 @@ use uv_workspace::{ProjectWorkspace, Workspace}; use crate::commands::pip::loggers::{InstallLogger, ResolveLogger}; use crate::commands::pip::operations::{Changelog, Modifications}; use crate::commands::reporters::{PythonDownloadReporter, ResolverReporter}; -use crate::commands::{pip, SharedState}; +use crate::commands::{capitalize, conjunction, pip, SharedState}; use crate::printer::Printer; use crate::settings::{InstallerSettingsRef, ResolverInstallerSettings, ResolverSettingsRef}; @@ -221,26 +221,77 @@ pub(crate) struct ConflictError { impl std::fmt::Display for ConflictError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - write!( - f, - "{} are incompatible with the declared conflicts: {{{}}}", - self.conflicts - .iter() - .map(|conflict| match conflict { - ConflictPackage::Extra(ref extra) => format!("extra `{extra}`"), - ConflictPackage::Group(ref group) if self.dev.default(group) => - format!("group `{group}` (enabled by default)"), - ConflictPackage::Group(ref group) => format!("group `{group}`"), - }) - .join(", "), - self.set - .iter() - .map(|item| match item.conflict() { - ConflictPackage::Extra(ref extra) => format!("`{}[{}]`", item.package(), extra), - ConflictPackage::Group(ref group) => format!("`{}:{}`", item.package(), group), - }) - .join(", ") - ) + // Format the set itself. + let set = self + .set + .iter() + .map(|item| match item.conflict() { + ConflictPackage::Extra(ref extra) => format!("`{}[{}]`", item.package(), extra), + ConflictPackage::Group(ref group) => format!("`{}:{}`", item.package(), group), + }) + .join(", "); + + // If all the conflicts are of the same kind, show a more succinct error. + if self + .conflicts + .iter() + .all(|conflict| matches!(conflict, ConflictPackage::Extra(..))) + { + write!( + f, + "Extras {} are incompatible with the declared conflicts: {{{set}}}", + conjunction( + self.conflicts + .iter() + .map(|conflict| match conflict { + ConflictPackage::Extra(ref extra) => format!("`{extra}`"), + ConflictPackage::Group(..) => unreachable!(), + }) + .collect() + ) + ) + } else if self + .conflicts + .iter() + .all(|conflict| matches!(conflict, ConflictPackage::Group(..))) + { + write!( + f, + "Groups {} are incompatible with the declared conflicts: {{{set}}}", + conjunction( + self.conflicts + .iter() + .map(|conflict| match conflict { + ConflictPackage::Group(ref group) if self.dev.default(group) => + format!("`{group}` (enabled by default)"), + ConflictPackage::Group(ref group) => format!("`{group}`"), + ConflictPackage::Extra(..) => unreachable!(), + }) + .collect() + ) + ) + } else { + write!( + f, + "{} are incompatible with the declared conflicts: {{{set}}}", + conjunction( + self.conflicts + .iter() + .enumerate() + .map(|(i, conflict)| { + let conflict = match conflict { + ConflictPackage::Extra(ref extra) => format!("extra `{extra}`"), + ConflictPackage::Group(ref group) if self.dev.default(group) => { + format!("group `{group}` (enabled by default)") + } + ConflictPackage::Group(ref group) => format!("group `{group}`"), + }; + (i == 0).then(|| capitalize(&conflict)).unwrap_or(conflict) + }) + .collect() + ) + ) + } } } diff --git a/crates/uv/src/commands/tool/upgrade.rs b/crates/uv/src/commands/tool/upgrade.rs index 6641195e8e33..277f69826059 100644 --- a/crates/uv/src/commands/tool/upgrade.rs +++ b/crates/uv/src/commands/tool/upgrade.rs @@ -25,7 +25,7 @@ use crate::commands::project::{ }; use crate::commands::reporters::PythonDownloadReporter; use crate::commands::tool::common::remove_entrypoints; -use crate::commands::{tool::common::install_executables, ExitStatus, SharedState}; +use crate::commands::{conjunction, tool::common::install_executables, ExitStatus, SharedState}; use crate::printer::Printer; use crate::settings::ResolverInstallerSettings; @@ -356,30 +356,3 @@ async fn upgrade_tool( Ok(outcome) } - -/// Given a list of names, return a conjunction of the names (e.g., "Alice, Bob and Charlie"). -fn conjunction(names: Vec) -> String { - let mut names = names.into_iter(); - let first = names.next(); - let last = names.next_back(); - match (first, last) { - (Some(first), Some(last)) => { - let mut result = first; - let mut comma = false; - for name in names { - result.push_str(", "); - result.push_str(&name); - comma = true; - } - if comma { - result.push_str(", and "); - } else { - result.push_str(" and "); - } - result.push_str(&last); - result - } - (Some(first), None) => first, - _ => String::new(), - } -} diff --git a/crates/uv/tests/it/lock.rs b/crates/uv/tests/it/lock.rs index 5f9ea2ac2161..d1e6cf658bea 100644 --- a/crates/uv/tests/it/lock.rs +++ b/crates/uv/tests/it/lock.rs @@ -2361,7 +2361,7 @@ fn lock_conflicting_extra_basic() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: extra `project1`, extra `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`} + error: Extras `project1` and `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`} "###); // As should exporting them. uv_snapshot!(context.filters(), context.export().arg("--frozen").arg("--all-extras"), @r###" @@ -2370,7 +2370,7 @@ fn lock_conflicting_extra_basic() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: extra `project1`, extra `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`} + error: Extras `project1` and `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`} "###); Ok(()) @@ -2604,7 +2604,7 @@ fn lock_conflicting_extra_multiple_not_conflicting1() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: extra `project1`, extra `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`} + error: Extras `project1` and `project2` are incompatible with the declared conflicts: {`project[project1]`, `project[project2]`} "###); // project3/project4 conflict! uv_snapshot!( @@ -2616,7 +2616,7 @@ fn lock_conflicting_extra_multiple_not_conflicting1() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: extra `project3`, extra `project4` are incompatible with the declared conflicts: {`project[project3]`, `project[project4]`} + error: Extras `project3` and `project4` are incompatible with the declared conflicts: {`project[project3]`, `project[project4]`} "###); // ... but project1/project3 does not. uv_snapshot!( @@ -3759,7 +3759,7 @@ fn lock_conflicting_group_basic() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: group `project1`, group `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} + error: Groups `project1` and `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} "###); Ok(()) @@ -3914,7 +3914,7 @@ fn lock_conflicting_group_default() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: group `project1` (enabled by default), group `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} + error: Groups `project1` (enabled by default) and `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} "###); // If the group is explicitly requested, we should still fail, but shouldn't mark it as @@ -3925,7 +3925,7 @@ fn lock_conflicting_group_default() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: group `project1`, group `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} + error: Groups `project1` and `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} "###); // If we install via `--all-groups`, we should also avoid marking the group as "enabled by @@ -3936,7 +3936,7 @@ fn lock_conflicting_group_default() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: group `project1`, group `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} + error: Groups `project1` and `project2` are incompatible with the declared conflicts: {`project:project1`, `project:project2`} "###); // Disabling the default group should succeed. @@ -4153,7 +4153,7 @@ fn lock_conflicting_mixed() -> Result<()> { ----- stdout ----- ----- stderr ----- - error: group `project1`, extra `project2` are incompatible with the declared conflicts: {`project:project1`, `project[project2]`} + error: Group `project1` and extra `project2` are incompatible with the declared conflicts: {`project:project1`, `project[project2]`} "###); Ok(())