From 58915da3e64a9aedcdd78fcae43384b51888a81a Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sun, 7 Jul 2024 16:13:08 -0400 Subject: [PATCH] Add some decoration to tool CLI --- crates/uv/src/commands/tool/install.rs | 38 ++++++++----- crates/uv/src/commands/tool/list.rs | 5 +- crates/uv/src/commands/tool/run.rs | 4 +- crates/uv/src/commands/tool/uninstall.rs | 15 +++-- crates/uv/tests/common/mod.rs | 6 +- crates/uv/tests/tool_install.rs | 72 ++++++++++++------------ crates/uv/tests/tool_list.rs | 8 +-- crates/uv/tests/tool_uninstall.rs | 8 +-- 8 files changed, 85 insertions(+), 71 deletions(-) diff --git a/crates/uv/src/commands/tool/install.rs b/crates/uv/src/commands/tool/install.rs index 13b5b4923d6a..84f8e5e0fa24 100644 --- a/crates/uv/src/commands/tool/install.rs +++ b/crates/uv/src/commands/tool/install.rs @@ -5,6 +5,7 @@ use std::str::FromStr; use anyhow::{bail, Context, Result}; use itertools::Itertools; +use owo_colors::OwoColorize; use tracing::debug; use distribution_types::Name; @@ -82,7 +83,7 @@ pub(crate) async fn install( // Parse the positional name. If the user provided more than a package name, it's an error // (e.g., `uv install foo==1.0 --from foo`). let Ok(package) = PackageName::from_str(&package) else { - bail!("Package requirement `{from}` provided with `--from` conflicts with install request `{package}`") + bail!("Package requirement (`{from}`) provided with `--from` conflicts with install request (`{package}`)") }; let from_requirement = resolve_requirements( @@ -105,7 +106,7 @@ pub(crate) async fn install( if from_requirement.name != package { // Determine if it's an entirely different package (e.g., `uv install foo --from bar`). bail!( - "Package name `{}` provided with `--from` does not match install request `{}`", + "Package name (`{}`) provided with `--from` does not match install request (`{}`)", from_requirement.name, package ); @@ -160,12 +161,12 @@ pub(crate) async fn install( .filter(|environment| { python_request.as_ref().map_or(true, |python_request| { if python_request.satisfied(environment.interpreter(), cache) { - debug!("Found existing environment for tool `{}`", from.name); + debug!("Found existing environment for `{}`", from.name); true } else { let _ = writeln!( printer.stderr(), - "Existing environment for `{}` does not satisfy the requested Python interpreter: `{}`", + "Existing environment for `{}` does not satisfy the requested Python interpreter (`{}`)", from.name, python_request ); @@ -187,7 +188,7 @@ pub(crate) async fn install( // And the user didn't request a reinstall or upgrade... if !force && settings.reinstall.is_none() && settings.upgrade.is_none() { // We're done. - writeln!(printer.stderr(), "Tool `{from}` is already installed")?; + writeln!(printer.stderr(), "`{from}` is already installed")?; return Ok(ExitStatus::Failure); } } @@ -269,7 +270,7 @@ pub(crate) async fn install( .context("Failed to create executable directory")?; debug!( - "Installing tool entry points into {}", + "Installing tool executables into: {}", executable_directory.user_display() ); @@ -298,7 +299,7 @@ pub(crate) async fn install( // Clean up the environment we just created installed_tools.remove_environment(&from.name)?; - bail!("No entry points found for tool `{}`", from.name); + bail!("No executables found for `{}`", from.name); } // Check if they exist, before installing @@ -311,7 +312,7 @@ pub(crate) async fn install( // will _not_ remove existing entry points when they are not managed by uv. if force || reinstall_entry_points { for (name, _, target) in existing_entry_points { - debug!("Removing existing entry point `{name}`"); + debug!("Removing existing executable: `{name}`"); fs_err::remove_file(target)?; } } else if existing_entry_points.peek().is_some() { @@ -328,25 +329,34 @@ pub(crate) async fn install( ("s", "exist") }; bail!( - "Entry point{s} for tool already {exists}: {} (use `--force` to overwrite)", - existing_entry_points.iter().join(", ") + "Executable{s} already {exists}: {} (use `--force` to overwrite)", + existing_entry_points + .iter() + .map(|name| name.bold()) + .join(", ") ) } for (name, source_path, target_path) in &target_entry_points { - debug!("Installing `{name}`"); + debug!("Installing executable: `{name}`"); #[cfg(unix)] - replace_symlink(source_path, target_path).context("Failed to install entrypoint")?; + replace_symlink(source_path, target_path).context("Failed to install executable")?; #[cfg(windows)] fs_err::copy(source_path, target_path).context("Failed to install entrypoint")?; } + let s = if target_entry_points.len() == 1 { + "" + } else { + "s" + }; writeln!( printer.stderr(), - "Installed: {}", + "Installed {} executable{s}: {}", + target_entry_points.len(), target_entry_points .iter() - .map(|(name, _, _)| name) + .map(|(name, _, _)| name.bold()) .join(", ") )?; diff --git a/crates/uv/src/commands/tool/list.rs b/crates/uv/src/commands/tool/list.rs index 76c4986bd910..7ce4663de1b9 100644 --- a/crates/uv/src/commands/tool/list.rs +++ b/crates/uv/src/commands/tool/list.rs @@ -1,6 +1,7 @@ use std::fmt::Write; use anyhow::Result; +use owo_colors::OwoColorize; use uv_cache::Cache; use uv_configuration::PreviewMode; @@ -40,11 +41,11 @@ pub(crate) async fn list( } }; - writeln!(printer.stdout(), "{name} v{version}")?; + writeln!(printer.stdout(), "{}", format!("{name} v{version}").bold())?; // Output tool entrypoints for entrypoint in tool.entrypoints() { - writeln!(printer.stdout(), " {}", &entrypoint.name)?; + writeln!(printer.stdout(), "- {}", &entrypoint.name)?; } } diff --git a/crates/uv/src/commands/tool/run.rs b/crates/uv/src/commands/tool/run.rs index 524cd67bc1eb..9557b0b21a89 100644 --- a/crates/uv/src/commands/tool/run.rs +++ b/crates/uv/src/commands/tool/run.rs @@ -288,7 +288,7 @@ fn parse_target(target: &OsString) -> Result<(Cow, Cow)> { // e.g. ignore `git+https://github.com/uv/uv.git@main` if PackageName::from_str(name).is_err() { - debug!("Ignoring non-package name `{}` in command", name); + debug!("Ignoring non-package name `{name}` in command"); return Ok((Cow::Borrowed(target), Cow::Borrowed(target_str))); } @@ -301,6 +301,6 @@ fn parse_target(target: &OsString) -> Result<(Cow, Cow)> { } // e.g. `uv@invalid`, warn and treat the whole thing as the command - debug!("Ignoring invalid version request `{}` in command", version); + debug!("Ignoring invalid version request `{version}` in command"); Ok((Cow::Borrowed(target), Cow::Borrowed(target_str))) } diff --git a/crates/uv/src/commands/tool/uninstall.rs b/crates/uv/src/commands/tool/uninstall.rs index d8a5499a73d3..a23b8b06e409 100644 --- a/crates/uv/src/commands/tool/uninstall.rs +++ b/crates/uv/src/commands/tool/uninstall.rs @@ -2,6 +2,7 @@ use std::fmt::Write; use anyhow::{bail, Result}; use itertools::Itertools; +use owo_colors::OwoColorize; use tracing::debug; use uv_configuration::PreviewMode; @@ -30,12 +31,12 @@ pub(crate) async fn uninstall( Ok(()) => { writeln!( printer.stderr(), - "Removed dangling environment for tool: `{name}` (missing receipt)" + "Removed dangling environment for `{name}`" )?; return Ok(ExitStatus::Success); } Err(uv_tool::Error::IO(err)) if err.kind() == std::io::ErrorKind::NotFound => { - bail!("Tool `{name}` is not installed"); + bail!("`{name}` is not installed"); } Err(err) => { return Err(err.into()); @@ -50,14 +51,14 @@ pub(crate) async fn uninstall( let entrypoints = receipt.entrypoints(); for entrypoint in entrypoints { debug!( - "Removing entrypoint: {}", + "Removing executable: {}", entrypoint.install_path.user_display() ); match fs_err::tokio::remove_file(&entrypoint.install_path).await { Ok(()) => {} Err(err) if err.kind() == std::io::ErrorKind::NotFound => { debug!( - "Entrypoint not found: {}", + "Executable not found: {}", entrypoint.install_path.user_display() ); } @@ -67,12 +68,14 @@ pub(crate) async fn uninstall( } } + let s = if entrypoints.len() == 1 { "" } else { "s" }; writeln!( printer.stderr(), - "Uninstalled: {}", + "Uninstalled {} executable{s}: {}", + entrypoints.len(), entrypoints .iter() - .map(|entrypoint| &entrypoint.name) + .map(|entrypoint| entrypoint.name.bold()) .join(", ") )?; diff --git a/crates/uv/tests/common/mod.rs b/crates/uv/tests/common/mod.rs index 1f2279d4ab6a..06661bf74661 100644 --- a/crates/uv/tests/common/mod.rs +++ b/crates/uv/tests/common/mod.rs @@ -72,10 +72,10 @@ pub struct TestContext { /// The Python version used for the virtual environment, if any. pub python_version: Option, - // All the Python versions available during this test context. + /// All the Python versions available during this test context. pub python_versions: Vec<(PythonVersion, PathBuf)>, - // Standard filters for this test context + /// Standard filters for this test context. filters: Vec<(String, String)>, } @@ -120,7 +120,7 @@ impl TestContext { #[must_use] pub fn with_filtered_exe_suffix(mut self) -> Self { self.filters - .push((std::env::consts::EXE_SUFFIX.to_string(), String::new())); + .push((regex::escape(env::consts::EXE_SUFFIX), String::new())); self } diff --git a/crates/uv/tests/tool_install.rs b/crates/uv/tests/tool_install.rs index 93914e9868f3..ca8217613c1d 100644 --- a/crates/uv/tests/tool_install.rs +++ b/crates/uv/tests/tool_install.rs @@ -42,7 +42,7 @@ fn tool_install() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -118,7 +118,7 @@ fn tool_install() { + jinja2==3.1.3 + markupsafe==2.1.5 + werkzeug==3.0.1 - Installed: flask + Installed 1 executable: flask "###); tool_dir.child("flask").assert(predicate::path::is_dir()); @@ -195,7 +195,7 @@ fn tool_install_version() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -280,7 +280,7 @@ fn tool_install_from() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); // Attempt to install `black` using `--from` with a different package name @@ -296,7 +296,7 @@ fn tool_install_from() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - error: Package name `flask` provided with `--from` does not match install request `black` + error: Package name (`flask`) provided with `--from` does not match install request (`black`) "###); // Attempt to install `black` using `--from` with a different version @@ -312,7 +312,7 @@ fn tool_install_from() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - error: Package requirement `black==24.3.0` provided with `--from` conflicts with install request `black==24.2.0` + error: Package requirement (`black==24.3.0`) provided with `--from` conflicts with install request (`black==24.2.0`) "###); } @@ -345,7 +345,7 @@ fn tool_install_already_installed() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -401,7 +401,7 @@ fn tool_install_already_installed() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Tool `black` is already installed + `black` is already installed "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -451,7 +451,7 @@ fn tool_install_already_installed() { + pathspec==0.12.1 - platformdirs==4.2.0 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); // Install `black` again with `--reinstall-package` for `black` @@ -473,7 +473,7 @@ fn tool_install_already_installed() { Installed [N] packages in [TIME] - black==24.3.0 + black==24.3.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); // Install `black` again with `--reinstall-package` for a dependency @@ -495,7 +495,7 @@ fn tool_install_already_installed() { Installed [N] packages in [TIME] - click==8.1.7 + click==8.1.7 - Installed: black, blackd + Installed 2 executables: black, blackd "###); } @@ -531,7 +531,7 @@ fn tool_install_entry_point_exists() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - error: Entry point for tool already exists: black (use `--force` to overwrite) + error: Executable already exists: black (use `--force` to overwrite) "###); // We should delete the virtual environment @@ -569,7 +569,7 @@ fn tool_install_entry_point_exists() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - error: Entry point for tool already exists: black (use `--force` to overwrite) + error: Executable already exists: black (use `--force` to overwrite) "###); // We should not create a virtual environment @@ -609,7 +609,7 @@ fn tool_install_entry_point_exists() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - error: Entry points for tool already exist: black, blackd (use `--force` to overwrite) + error: Executables already exist: black, blackd (use `--force` to overwrite) "###); // Install `black` with `--force` @@ -632,7 +632,7 @@ fn tool_install_entry_point_exists() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -649,7 +649,7 @@ fn tool_install_entry_point_exists() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -665,7 +665,7 @@ fn tool_install_entry_point_exists() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Tool `black` is already installed + `black` is already installed "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -697,7 +697,7 @@ fn tool_install_entry_point_exists() { + pathspec==0.12.1 - platformdirs==4.2.0 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -786,7 +786,7 @@ fn tool_install_home() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); context @@ -822,7 +822,7 @@ fn tool_install_xdg_data_home() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); context @@ -858,7 +858,7 @@ fn tool_install_xdg_bin_home() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); bin_dir @@ -887,7 +887,7 @@ fn tool_install_no_entrypoints() { Prepared 1 package in [TIME] Installed 1 package in [TIME] + iniconfig==2.0.0 - error: No entry points found for tool `iniconfig` + error: No executables found for `iniconfig` "###); } @@ -918,7 +918,7 @@ fn tool_install_unnamed_package() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -995,7 +995,7 @@ fn tool_install_unnamed_conflict() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - error: Package name `iniconfig` provided with `--from` does not match install request `black` + error: Package name (`iniconfig`) provided with `--from` does not match install request (`black`) "###); } @@ -1028,7 +1028,7 @@ fn tool_install_unnamed_from() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -1114,7 +1114,7 @@ fn tool_install_unnamed_with() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); tool_dir.child("black").assert(predicate::path::is_dir()); @@ -1202,7 +1202,7 @@ fn tool_install_upgrade() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); insta::with_settings!({ @@ -1231,7 +1231,7 @@ fn tool_install_upgrade() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Installed: black, blackd + Installed 2 executables: black, blackd "###); insta::with_settings!({ @@ -1265,7 +1265,7 @@ fn tool_install_upgrade() { Prepared [N] packages in [TIME] Installed [N] packages in [TIME] + iniconfig==2.0.0 (from https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl) - Installed: black, blackd + Installed 2 executables: black, blackd "###); insta::with_settings!({ @@ -1305,7 +1305,7 @@ fn tool_install_upgrade() { - black==24.1.1 + black==24.3.0 - iniconfig==2.0.0 (from https://files.pythonhosted.org/packages/ef/a6/62565a6e1cf69e10f5727360368e451d4b7f58beeac6173dc9db836a5b46/iniconfig-2.0.0-py3-none-any.whl) - Installed: black, blackd + Installed 2 executables: black, blackd "###); insta::with_settings!({ @@ -1354,7 +1354,7 @@ fn tool_install_python_request() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); // Install with Python 3.12 (compatible). @@ -1370,7 +1370,7 @@ fn tool_install_python_request() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Tool `black` is already installed + `black` is already installed "###); // Install with Python 3.11 (incompatible). @@ -1386,7 +1386,7 @@ fn tool_install_python_request() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Existing environment for `black` does not satisfy the requested Python interpreter: `Python 3.11` + Existing environment for `black` does not satisfy the requested Python interpreter (`Python 3.11`) Resolved [N] packages in [TIME] Prepared [N] packages in [TIME] Installed [N] packages in [TIME] @@ -1396,7 +1396,7 @@ fn tool_install_python_request() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); } @@ -1429,7 +1429,7 @@ fn tool_install_preserve_environment() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); // Install `black`, but with an incompatible requirement. @@ -1460,6 +1460,6 @@ fn tool_install_preserve_environment() { ----- stderr ----- warning: `uv tool install` is experimental and may change without warning. - Tool `black==24.1.1` is already installed + `black==24.1.1` is already installed "###); } diff --git a/crates/uv/tests/tool_list.rs b/crates/uv/tests/tool_list.rs index e87b50379842..83412e8e411d 100644 --- a/crates/uv/tests/tool_list.rs +++ b/crates/uv/tests/tool_list.rs @@ -31,9 +31,9 @@ fn tool_list() { exit_code: 0 ----- stdout ----- black v24.2.0 - black - blackd - + - black + - blackd + ----- stderr ----- warning: `uv tool list` is experimental and may change without warning. "###); @@ -131,7 +131,7 @@ fn tool_list_bad_environment() -> Result<()> { exit_code: 0 ----- stdout ----- ruff v0.3.4 - ruff + - ruff ----- stderr ----- warning: `uv tool list` is experimental and may change without warning. diff --git a/crates/uv/tests/tool_uninstall.rs b/crates/uv/tests/tool_uninstall.rs index b9a8bbb5253d..b63c724388a9 100644 --- a/crates/uv/tests/tool_uninstall.rs +++ b/crates/uv/tests/tool_uninstall.rs @@ -31,7 +31,7 @@ fn tool_uninstall() { ----- stderr ----- warning: `uv tool uninstall` is experimental and may change without warning. - Uninstalled: black, blackd + Uninstalled 2 executables: black, blackd "###); // After uninstalling the tool, it shouldn't be listed. @@ -66,7 +66,7 @@ fn tool_uninstall() { + packaging==24.0 + pathspec==0.12.1 + platformdirs==4.2.0 - Installed: black, blackd + Installed 2 executables: black, blackd "###); } @@ -85,7 +85,7 @@ fn tool_uninstall_not_installed() { ----- stderr ----- warning: `uv tool uninstall` is experimental and may change without warning. - error: Tool `black` is not installed + error: `black` is not installed "###); } @@ -115,6 +115,6 @@ fn tool_uninstall_missing_receipt() { ----- stderr ----- warning: `uv tool uninstall` is experimental and may change without warning. - Removed dangling environment for tool: `black` (missing receipt) + Removed dangling environment for `black` "###); }