Add various grammar changes to conflict error messages (#9369)

## Summary

If all items are the same kind, we can avoid repeating "extra" and
"group". If there are two, we now use "X and Y", etc.
This commit is contained in:
Charlie Marsh
2024-11-22 17:23:13 -05:00
committed by GitHub
parent 619ec8dcce
commit b2bad8d59d
5 changed files with 125 additions and 64 deletions
+7 -6
View File
@@ -2422,7 +2422,7 @@ impl DiscoveryPreferences {
.map(ToString::to_string)
.collect::<Vec<_>>();
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::<Vec<_>>(),
)
} else {
conjunction(&["virtual environments"])
disjunction(&["virtual environments"])
}
}
EnvironmentPreference::OnlySystem => conjunction(
EnvironmentPreference::OnlySystem => disjunction(
&python_sources
.iter()
.map(String::as_str)
.collect::<Vec<_>>(),
),
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]),
_ => {
+36
View File
@@ -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>) -> 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::<String>() + chars.as_str(),
}
}
+72 -21
View File
@@ -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()
)
)
}
}
}
+1 -28
View File
@@ -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>) -> 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(),
}
}
+9 -9
View File
@@ -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(())