1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-12-25 00:29:09 -05:00

refactor: nicer warning display (#21547)

This commit is contained in:
Bartek Iwańczuk 2023-12-12 15:45:45 +01:00 committed by GitHub
parent 06c5f99a01
commit ece78cfb8a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 92 additions and 23 deletions

View file

@ -53,6 +53,7 @@ struct PreparedPublishPackage {
version: String, version: String,
tarball_hash: String, tarball_hash: String,
tarball: Bytes, tarball: Bytes,
diagnostics: Vec<String>,
} }
#[derive(serde::Deserialize)] #[derive(serde::Deserialize)]
@ -100,7 +101,8 @@ async fn prepare_publish(
let unfurler = ImportMapUnfurler::new(import_map); let unfurler = ImportMapUnfurler::new(import_map);
let tarball = tar::create_gzipped_tarball(directory_path, unfurler) let (tarball, diagnostics) =
tar::create_gzipped_tarball(directory_path, unfurler)
.context("Failed to create a tarball")?; .context("Failed to create a tarball")?;
let tarball_hash_bytes: Vec<u8> = let tarball_hash_bytes: Vec<u8> =
@ -116,6 +118,7 @@ async fn prepare_publish(
version: version.to_string(), version: version.to_string(),
tarball_hash, tarball_hash,
tarball, tarball,
diagnostics,
}) })
} }
@ -221,6 +224,47 @@ struct OidcTokenResponse {
value: String, value: String,
} }
/// Prints diagnostics like so:
/// ```
///
/// Warning
/// ├╌ Dynamic import was not analyzable...
/// ├╌╌ at file:///dev/foo/bar/foo.ts:4:5
/// |
/// ├╌ Dynamic import was not analyzable...
/// ├╌╌ at file:///dev/foo/bar/foo.ts:4:5
/// |
/// ├╌ Dynamic import was not analyzable...
/// └╌╌ at file:///dev/foo/bar/foo.ts:4:5
///
/// ```
fn print_diagnostics(diagnostics: Vec<String>) {
if !diagnostics.is_empty() {
let len = diagnostics.len();
log::warn!("");
log::warn!("{}", crate::colors::yellow("Warning"));
for (i, diagnostic) in diagnostics.iter().enumerate() {
let last_diagnostic = i == len - 1;
let lines = diagnostic.split('\n').collect::<Vec<_>>();
let lines_len = lines.len();
if i != 0 {
log::warn!("|");
}
for (j, line) in lines.iter().enumerate() {
let last_line = j == lines_len - 1;
if j == 0 {
log::warn!("├╌ {}", line);
} else if last_line && last_diagnostic {
log::warn!("└╌╌ {}", line);
} else {
log::warn!("├╌╌ {}", line);
}
}
}
log::warn!("");
}
}
async fn perform_publish( async fn perform_publish(
http_client: &Arc<HttpClient>, http_client: &Arc<HttpClient>,
packages: Vec<PreparedPublishPackage>, packages: Vec<PreparedPublishPackage>,
@ -229,6 +273,12 @@ async fn perform_publish(
let client = http_client.client()?; let client = http_client.client()?;
let registry_url = deno_registry_api_url().to_string(); let registry_url = deno_registry_api_url().to_string();
let diagnostics = packages
.iter()
.flat_map(|p| p.diagnostics.clone())
.collect::<Vec<_>>();
print_diagnostics(diagnostics);
let permissions = packages let permissions = packages
.iter() .iter()
.map(|package| Permission::VersionPublish { .map(|package| Permission::VersionPublish {
@ -267,6 +317,8 @@ async fn perform_publish(
println!(" @{}/{}", packages[0].scope, packages[0].package); println!(" @{}/{}", packages[0].scope, packages[0].package);
} }
// ASCII code for the bell character.
print!("\x07");
println!("{}", colors::gray("Waiting...")); println!("{}", colors::gray("Waiting..."));
let interval = std::time::Duration::from_secs(auth.poll_interval); let interval = std::time::Duration::from_secs(auth.poll_interval);

View file

@ -16,11 +16,12 @@ pub fn create_gzipped_tarball(
// TODO(bartlomieju): this is too specific, factor it out into a callback that // TODO(bartlomieju): this is too specific, factor it out into a callback that
// returns data // returns data
unfurler: ImportMapUnfurler, unfurler: ImportMapUnfurler,
) -> Result<Bytes, AnyError> { ) -> Result<(Bytes, Vec<String>), AnyError> {
let mut tar = TarGzArchive::new(); let mut tar = TarGzArchive::new();
let dir = dir let dir = dir
.canonicalize() .canonicalize()
.map_err(|_| anyhow::anyhow!("Unable to canonicalize path {:?}", dir))?; .map_err(|_| anyhow::anyhow!("Unable to canonicalize path {:?}", dir))?;
let mut diagnostics = vec![];
for entry in walkdir::WalkDir::new(&dir).follow_links(false) { for entry in walkdir::WalkDir::new(&dir).follow_links(false) {
let entry = entry?; let entry = entry?;
@ -37,9 +38,11 @@ pub fn create_gzipped_tarball(
})?; })?;
let data = std::fs::read(entry.path()) let data = std::fs::read(entry.path())
.with_context(|| format!("Unable to read file {:?}", entry.path()))?; .with_context(|| format!("Unable to read file {:?}", entry.path()))?;
let content = unfurler let (content, unfurl_diagnostics) = unfurler
.unfurl(&url, data) .unfurl(&url, data)
.with_context(|| format!("Unable to unfurl file {:?}", entry.path()))?; .with_context(|| format!("Unable to unfurl file {:?}", entry.path()))?;
diagnostics.extend_from_slice(&unfurl_diagnostics);
tar tar
.add_file(relative_path.to_string(), &content) .add_file(relative_path.to_string(), &content)
.with_context(|| { .with_context(|| {
@ -48,12 +51,13 @@ pub fn create_gzipped_tarball(
} else if entry.file_type().is_dir() { } else if entry.file_type().is_dir() {
// skip // skip
} else { } else {
log::warn!("Unsupported file type at path {:?}", entry.path()); diagnostics
.push(format!("Unsupported file type at path {:?}", entry.path()));
} }
} }
let v = tar.finish().context("Unable to finish tarball")?; let v = tar.finish().context("Unable to finish tarball")?;
Ok(Bytes::from(v)) Ok((Bytes::from(v), diagnostics))
} }
struct TarGzArchive { struct TarGzArchive {

View file

@ -25,7 +25,8 @@ impl<'a> ImportMapUnfurler<'a> {
&self, &self,
url: &ModuleSpecifier, url: &ModuleSpecifier,
data: Vec<u8>, data: Vec<u8>,
) -> Result<Vec<u8>, AnyError> { ) -> Result<(Vec<u8>, Vec<String>), AnyError> {
let mut diagnostics = vec![];
let media_type = MediaType::from_specifier(url); let media_type = MediaType::from_specifier(url);
match media_type { match media_type {
@ -48,7 +49,7 @@ impl<'a> ImportMapUnfurler<'a> {
| MediaType::Wasm | MediaType::Wasm
| MediaType::TsBuildInfo => { | MediaType::TsBuildInfo => {
// not unfurlable data // not unfurlable data
return Ok(data); return Ok((data, diagnostics));
} }
} }
@ -94,14 +95,14 @@ impl<'a> ImportMapUnfurler<'a> {
); );
if !success { if !success {
log::warn!( diagnostics.push(
"{} Dynamic import was not analyzable and won't use the import map once published.\n at {}", format!("Dynamic import was not analyzable and won't use the import map once published.\n at {}",
crate::colors::yellow("Warning"),
format_range_with_colors(&deno_graph::Range { format_range_with_colors(&deno_graph::Range {
specifier: url.clone(), specifier: url.clone(),
start: dep.range.start.clone(), start: dep.range.start.clone(),
end: dep.range.end.clone(), end: dep.range.end.clone(),
}) })
)
); );
} }
} }
@ -132,13 +133,14 @@ impl<'a> ImportMapUnfurler<'a> {
&mut text_changes, &mut text_changes,
); );
} }
Ok( Ok((
deno_ast::apply_text_changes( deno_ast::apply_text_changes(
parsed_source.text_info().text_str(), parsed_source.text_info().text_str(),
text_changes, text_changes,
) )
.into_bytes(), .into_bytes(),
) diagnostics,
))
} }
#[cfg(test)] #[cfg(test)]
@ -146,10 +148,10 @@ impl<'a> ImportMapUnfurler<'a> {
&self, &self,
url: &ModuleSpecifier, url: &ModuleSpecifier,
data: Vec<u8>, data: Vec<u8>,
) -> Result<String, AnyError> { ) -> Result<(String, Vec<String>), AnyError> {
let data = self.unfurl(url, data)?; let (data, diagnostics) = self.unfurl(url, data)?;
let content = String::from_utf8(data)?; let content = String::from_utf8(data)?;
Ok(content) Ok((content, diagnostics))
} }
} }
@ -284,9 +286,12 @@ const test5 = await import(`lib${expr}`);
const test6 = await import(`${expr}`); const test6 = await import(`${expr}`);
"#; "#;
let specifier = ModuleSpecifier::parse("file:///dev/mod.ts").unwrap(); let specifier = ModuleSpecifier::parse("file:///dev/mod.ts").unwrap();
let unfurled_source = unfurler let (unfurled_source, d) = unfurler
.unfurl_to_string(&specifier, source_code.as_bytes().to_vec()) .unfurl_to_string(&specifier, source_code.as_bytes().to_vec())
.unwrap(); .unwrap();
assert_eq!(d.len(), 2);
assert!(d[0].starts_with("Dynamic import was not analyzable and won't use the import map once published."));
assert!(d[1].starts_with("Dynamic import was not analyzable and won't use the import map once published."));
let expected_source = r#"import express from "npm:express@5";" let expected_source = r#"import express from "npm:express@5";"
import foo from "./lib/foo.ts"; import foo from "./lib/foo.ts";
import bar from "./lib/bar.ts"; import bar from "./lib/bar.ts";
@ -311,9 +316,10 @@ import bar from "lib/bar.ts";
import fizz from "fizz"; import fizz from "fizz";
"#; "#;
let specifier = ModuleSpecifier::parse("file:///dev/mod").unwrap(); let specifier = ModuleSpecifier::parse("file:///dev/mod").unwrap();
let unfurled_source = unfurler let (unfurled_source, d) = unfurler
.unfurl_to_string(&specifier, source_code.as_bytes().to_vec()) .unfurl_to_string(&specifier, source_code.as_bytes().to_vec())
.unwrap(); .unwrap();
assert!(d.is_empty());
assert_eq!(unfurled_source, source_code); assert_eq!(unfurled_source, source_code);
} }
} }

View file

@ -108,6 +108,13 @@ pub fn cyan<S: AsRef<str>>(s: S) -> impl fmt::Display {
style_spec.set_fg(Some(Cyan)); style_spec.set_fg(Some(Cyan));
style(s, style_spec) style(s, style_spec)
} }
pub fn cyan_with_underline<S: AsRef<str>>(s: S) -> impl fmt::Display {
let mut style_spec = ColorSpec::new();
style_spec.set_fg(Some(Cyan)).set_underline(true);
style(s, style_spec)
}
pub fn cyan_bold<S: AsRef<str>>(s: S) -> impl fmt::Display { pub fn cyan_bold<S: AsRef<str>>(s: S) -> impl fmt::Display {
let mut style_spec = ColorSpec::new(); let mut style_spec = ColorSpec::new();
style_spec.set_fg(Some(Cyan)).set_bold(true); style_spec.set_fg(Some(Cyan)).set_bold(true);