From 1fc03780f9812fcb847680927d5085356bb411fa Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 18 Oct 2023 14:24:09 -0400 Subject: [PATCH] Use `miette` for `puffin add` diagnostics (#119) Experiment in using `miette` for better user-facing diagnostics in the CLI crate: Screen Shot 2023-10-18 at 2 11 54 PM For now, only the `add` command has been migrated, and all the library crates continue to use `anyhow`. --- Cargo.lock | 93 ++++++++++++++++++++++++ Cargo.toml | 1 + crates/puffin-cli/Cargo.toml | 4 +- crates/puffin-cli/src/commands/add.rs | 61 ++++++++++++++-- crates/puffin-cli/src/commands/venv.rs | 6 +- crates/puffin-workspace/src/lib.rs | 2 + crates/puffin-workspace/src/verbatim.rs | 23 ++++++ crates/puffin-workspace/src/workspace.rs | 22 +++--- 8 files changed, 188 insertions(+), 24 deletions(-) create mode 100644 crates/puffin-workspace/src/verbatim.rs diff --git a/Cargo.lock b/Cargo.lock index f1b35a78a..27b2874d4 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -182,6 +182,15 @@ dependencies = [ "rustc-demangle", ] +[[package]] +name = "backtrace-ext" +version = "0.2.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "537beee3be4a18fb023b570f80e3ae28003db9167a751266b259926e25539d50" +dependencies = [ + "backtrace", +] + [[package]] name = "base64" version = "0.13.1" @@ -1243,6 +1252,12 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "is_ci" +version = "1.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "616cde7c720bb2bb5824a224687d8f77bfd38922027f01d825cd7453be5099fb" + [[package]] name = "itertools" version = "0.11.0" @@ -1366,8 +1381,17 @@ version = "5.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "59bb584eaeeab6bd0226ccf3509a69d7936d148cf3d036ad350abe35e8c6856e" dependencies = [ + "backtrace", + "backtrace-ext", + "is-terminal", "miette-derive", "once_cell", + "owo-colors", + "supports-color", + "supports-hyperlinks", + "supports-unicode", + "terminal_size", + "textwrap", "thiserror", "unicode-width", ] @@ -1543,6 +1567,12 @@ version = "0.1.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b15813163c1d831bf4a13c3610c05c0d03b39feb07f7e09fa234dac9b15aaf39" +[[package]] +name = "owo-colors" +version = "3.5.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c1b04fb49957986fdce4d6ee7a65027d55d4b6d2265e5848bbb507b58ccfdb6f" + [[package]] name = "parking_lot" version = "0.11.2" @@ -1822,6 +1852,7 @@ dependencies = [ "indicatif", "install-wheel-rs", "itertools", + "miette", "pep440_rs 0.3.12", "pep508_rs", "platform-host", @@ -1834,6 +1865,7 @@ dependencies = [ "puffin-resolver", "puffin-workspace", "tempfile", + "thiserror", "tokio", "tracing", "tracing-subscriber", @@ -2529,6 +2561,12 @@ version = "1.11.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "942b4a808e05215192e39f4ab80813e599068285906cc91aa64f923db842bd5a" +[[package]] +name = "smawk" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" + [[package]] name = "socket2" version = "0.4.9" @@ -2585,6 +2623,34 @@ version = "0.10.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "73473c0e59e6d5812c5dfe2a064a6444949f089e20eec9a2e5506596494e4623" +[[package]] +name = "supports-color" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d6398cde53adc3c4557306a96ce67b302968513830a77a95b2b17305d9719a89" +dependencies = [ + "is-terminal", + "is_ci", +] + +[[package]] +name = "supports-hyperlinks" +version = "2.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f84231692eb0d4d41e4cdd0cabfdd2e6cd9e255e65f80c9aa7c98dd502b4233d" +dependencies = [ + "is-terminal", +] + +[[package]] +name = "supports-unicode" +version = "2.0.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "4b6c2cb240ab5dd21ed4906895ee23fe5a48acdbd15a3ce388e7b62a9b66baf7" +dependencies = [ + "is-terminal", +] + [[package]] name = "syn" version = "1.0.109" @@ -2667,6 +2733,16 @@ dependencies = [ "windows-sys 0.48.0", ] +[[package]] +name = "terminal_size" +version = "0.1.17" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "633c1a546cee861a1a6d0dc69ebeca693bf4296661ba7852b9d21d159e0506df" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "test-case" version = "3.2.1" @@ -2711,6 +2787,17 @@ dependencies = [ "log", ] +[[package]] +name = "textwrap" +version = "0.15.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "b7b3e525a49ec206798b40326a44121291b530c963cfb01018f63e135bac543d" +dependencies = [ + "smawk", + "unicode-linebreak", + "unicode-width", +] + [[package]] name = "thiserror" version = "1.0.49" @@ -3030,6 +3117,12 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" +[[package]] +name = "unicode-linebreak" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" + [[package]] name = "unicode-normalization" version = "0.1.22" diff --git a/Cargo.toml b/Cargo.toml index 387c0ff21..ff200c6d0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ indoc = { version = "2.0.4" } itertools = { version = "0.11.0" } mailparse = { version = "0.14.0" } memchr = { version = "2.6.4" } +miette = { version = "5.10.0" } once_cell = { version = "1.18.0" } platform-info = { version = "2.0.2" } plist = { version = "1.5.0" } diff --git a/crates/puffin-cli/Cargo.toml b/crates/puffin-cli/Cargo.toml index 357bb812d..e0357afd7 100644 --- a/crates/puffin-cli/Cargo.toml +++ b/crates/puffin-cli/Cargo.toml @@ -32,10 +32,12 @@ fs-err = { workspace = true, features = ["tokio"] } futures = { workspace = true } indicatif = { workspace = true } itertools = { workspace = true } +miette = { workspace = true, features = ["fancy"] } tempfile = { workspace = true } +thiserror = { workspace = true } tokio = { workspace = true } tracing = { workspace = true } tracing-subscriber = { workspace = true } tracing-tree = { workspace = true } url = { workspace = true } -which = { workspace = true} +which = { workspace = true } diff --git a/crates/puffin-cli/src/commands/add.rs b/crates/puffin-cli/src/commands/add.rs index cbd6ed22c..d46c182a6 100644 --- a/crates/puffin-cli/src/commands/add.rs +++ b/crates/puffin-cli/src/commands/add.rs @@ -1,29 +1,74 @@ +use std::path::PathBuf; + use anyhow::Result; +use miette::{Diagnostic, IntoDiagnostic}; +use thiserror::Error; use tracing::info; +use puffin_workspace::WorkspaceError; + use crate::commands::ExitStatus; use crate::printer::Printer; /// Add a dependency to the workspace. +#[allow(clippy::unnecessary_wraps)] pub(crate) fn add(name: &str, _printer: Printer) -> Result { + match add_impl(name) { + Ok(status) => Ok(status), + Err(err) => { + #[allow(clippy::print_stderr)] + { + eprint!("{err:?}"); + } + Ok(ExitStatus::Failure) + } + } +} + +#[derive(Error, Debug, Diagnostic)] +enum AddError { + #[error( + "Could not find a `pyproject.toml` file in the current directory or any of its parents" + )] + #[diagnostic(code(puffin::add::workspace_not_found))] + WorkspaceNotFound, + + #[error("Failed to parse requirement: `{0}`")] + #[diagnostic(code(puffin::add::invalid_requirement))] + InvalidRequirement(String, #[source] pep508_rs::Pep508Error), + + #[error("Failed to parse `pyproject.toml` at: `{0}`")] + #[diagnostic(code(puffin::add::parse_error))] + ParseError(PathBuf, #[source] WorkspaceError), + + #[error("Failed to write `pyproject.toml` to: `{0}`")] + #[diagnostic(code(puffin::add::write_error))] + WriteError(PathBuf, #[source] WorkspaceError), +} + +fn add_impl(name: &str) -> miette::Result { + let requirement = puffin_workspace::VerbatimRequirement::try_from(name) + .map_err(|err| AddError::InvalidRequirement(name.to_string(), err))?; + // Locate the workspace. - let Some(workspace_root) = puffin_workspace::find_pyproject_toml(std::env::current_dir()?) - else { - return Err(anyhow::anyhow!( - "Could not find a `pyproject.toml` file in the current directory or any of its parents" - )); + let cwd = std::env::current_dir().into_diagnostic()?; + let Some(workspace_root) = puffin_workspace::find_pyproject_toml(cwd) else { + return Err(AddError::WorkspaceNotFound.into()); }; info!("Found workspace at: {}", workspace_root.display()); // Parse the manifest. - let mut manifest = puffin_workspace::Workspace::try_from(workspace_root.as_path())?; + let mut manifest = puffin_workspace::Workspace::try_from(workspace_root.as_path()) + .map_err(|err| AddError::ParseError(workspace_root.clone(), err))?; // Add the dependency. - manifest.add_dependency(name)?; + manifest.add_dependency(&requirement); // Write the manifest back to disk. - manifest.save(&workspace_root)?; + manifest + .save(&workspace_root) + .map_err(|err| AddError::WriteError(workspace_root.clone(), err))?; Ok(ExitStatus::Success) } diff --git a/crates/puffin-cli/src/commands/venv.rs b/crates/puffin-cli/src/commands/venv.rs index 6a66c8db7..b0e6d60b3 100644 --- a/crates/puffin-cli/src/commands/venv.rs +++ b/crates/puffin-cli/src/commands/venv.rs @@ -1,7 +1,7 @@ use std::fmt::Write; use std::path::Path; -use anyhow::Result; +use anyhow::{Context, Result}; use colored::Colorize; use fs_err::tokio as fs; @@ -29,8 +29,8 @@ pub(crate) async fn venv( )?; // If the path already exists, remove it. - fs::remove_file(path).await.ok(); - fs::remove_dir_all(path).await.ok(); + fs::remove_file(path).await.context("Foo")?; + fs::remove_dir_all(path).await?; writeln!( printer, diff --git a/crates/puffin-workspace/src/lib.rs b/crates/puffin-workspace/src/lib.rs index 9a34c2aec..249d58a51 100644 --- a/crates/puffin-workspace/src/lib.rs +++ b/crates/puffin-workspace/src/lib.rs @@ -1,10 +1,12 @@ use std::path::{Path, PathBuf}; pub use error::WorkspaceError; +pub use verbatim::VerbatimRequirement; pub use workspace::Workspace; mod error; mod toml; +mod verbatim; mod workspace; /// Find the closest `pyproject.toml` file to the given path. diff --git a/crates/puffin-workspace/src/verbatim.rs b/crates/puffin-workspace/src/verbatim.rs new file mode 100644 index 000000000..ac1e07897 --- /dev/null +++ b/crates/puffin-workspace/src/verbatim.rs @@ -0,0 +1,23 @@ +use std::str::FromStr; + +use pep508_rs::Requirement; + +#[derive(Debug)] +pub struct VerbatimRequirement<'a> { + /// The name of the requirement as provided by the user. + pub given_name: &'a str, + /// The normalized requirement. + pub requirement: Requirement, +} + +impl<'a> TryFrom<&'a str> for VerbatimRequirement<'a> { + type Error = pep508_rs::Pep508Error; + + fn try_from(s: &'a str) -> Result { + let requirement = Requirement::from_str(s)?; + Ok(Self { + given_name: s, + requirement, + }) + } +} diff --git a/crates/puffin-workspace/src/workspace.rs b/crates/puffin-workspace/src/workspace.rs index e267d8369..7ef37a604 100644 --- a/crates/puffin-workspace/src/workspace.rs +++ b/crates/puffin-workspace/src/workspace.rs @@ -11,6 +11,7 @@ use pep508_rs::Requirement; use puffin_package::package_name::PackageName; use crate::toml::format_multiline_array; +use crate::verbatim::VerbatimRequirement; use crate::WorkspaceError; /// Unlike [`pyproject_toml::PyProjectToml`], in our case `build_system` is also optional @@ -36,9 +37,7 @@ pub struct Workspace { impl Workspace { /// Add a dependency to the workspace. - pub fn add_dependency(&mut self, dependency: &str) -> Result<(), WorkspaceError> { - let requirement = Requirement::from_str(dependency)?; - + pub fn add_dependency(&mut self, requirement: &VerbatimRequirement<'_>) { let Some(project) = self .document .get_mut("project") @@ -46,7 +45,7 @@ impl Workspace { else { // No `project` table. let mut dependencies = toml_edit::Array::new(); - dependencies.push(dependency); + dependencies.push(requirement.given_name); format_multiline_array(&mut dependencies); let mut project = toml_edit::Table::new(); @@ -58,7 +57,7 @@ impl Workspace { self.document .insert("project", toml_edit::Item::Table(project)); - return Ok(()); + return; }; let Some(dependencies) = project @@ -67,14 +66,14 @@ impl Workspace { else { // No `dependencies` array. let mut dependencies = toml_edit::Array::new(); - dependencies.push(dependency); + dependencies.push(requirement.given_name); format_multiline_array(&mut dependencies); project.insert( "dependencies", toml_edit::Item::Value(toml_edit::Value::Array(dependencies)), ); - return Ok(()); + return; }; // TODO(charlie): Awkward `drop` pattern required to work around destructors, apparently. @@ -88,19 +87,18 @@ impl Workspace { return false; }; - PackageName::normalize(&requirement.name) == PackageName::normalize(existing.name) + PackageName::normalize(&requirement.requirement.name) + == PackageName::normalize(existing.name) }); drop(iter); if let Some(index) = index { - dependencies.replace(index, dependency); + dependencies.replace(index, requirement.given_name); } else { - dependencies.push(dependency); + dependencies.push(requirement.given_name); } format_multiline_array(dependencies); - - Ok(()) } /// Save the workspace to disk.