Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turbopack: Implement structured styled text and use it in issue descriptions #6388

Merged
merged 16 commits into from
Nov 14, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
6b1f818
Turbopack: Implement structured styled text and use it in issue descr…
wbinnssmith Nov 8, 2023
d944747
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 8, 2023
0f87d5b
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 8, 2023
016fdf5
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 8, 2023
cf44600
Update crates/turbopack-core/src/issue/mod.rs
wbinnssmith Nov 8, 2023
ffa9e26
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 8, 2023
a2fc442
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 8, 2023
24cd251
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 9, 2023
52e2cd7
Merge remote-tracking branch 'origin/main' into wbinnssmith/styled-st…
wbinnssmith Nov 9, 2023
9ea3c20
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 9, 2023
3a494af
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 9, 2023
8ef9511
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 9, 2023
bd27bee
Merge remote-tracking branch 'origin/main' into wbinnssmith/styled-st…
wbinnssmith Nov 13, 2023
0cc28c0
Merge remote-tracking branch 'origin/main' into wbinnssmith/styled-st…
wbinnssmith Nov 14, 2023
d0bd9fb
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 14, 2023
65a0d81
fixup! Turbopack: Implement structured styled text and use it in issu…
wbinnssmith Nov 14, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions crates/turbo-tasks-fetch/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use anyhow::Result;
use turbo_tasks::Vc;
use turbo_tasks_fs::FileSystemPath;
use turbopack_core::issue::{Issue, IssueSeverity};
use turbopack_core::issue::{Issue, IssueSeverity, StyledString};

pub fn register() {
turbo_tasks::register();
Expand Down Expand Up @@ -154,11 +154,11 @@ impl Issue for FetchIssue {
}

#[turbo_tasks::function]
async fn description(&self) -> Result<Vc<String>> {
async fn description(&self) -> Result<Vc<StyledString>> {
let url = &*self.url.await?;
let kind = &*self.kind.await?;

Ok(Vc::cell(match kind {
Ok(StyledString::String(match kind {
FetchErrorKind::Connect => format!(
"There was an issue establishing a connection while requesting {}.",
url
Expand All @@ -171,7 +171,8 @@ impl Issue for FetchIssue {
}
FetchErrorKind::Timeout => format!("Connection timed out when requesting {}", url),
FetchErrorKind::Other => format!("There was an issue requesting {}", url),
}))
})
.cell())
}

#[turbo_tasks::function]
Expand Down
6 changes: 3 additions & 3 deletions crates/turbo-tasks-fetch/tests/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use turbo_tasks::Vc;
use turbo_tasks_fetch::{fetch, register, FetchErrorKind};
use turbo_tasks_fs::{DiskFileSystem, FileSystem, FileSystemPath};
use turbo_tasks_testing::{register, run};
use turbopack_core::issue::{Issue, IssueSeverity};
use turbopack_core::issue::{Issue, IssueSeverity, StyledString};

register!();

Expand Down Expand Up @@ -115,7 +115,7 @@ async fn errors_on_failed_connection() {
let issue = err_vc.to_issue(IssueSeverity::Error.into(), get_issue_context());
assert_eq!(*issue.severity().await?, IssueSeverity::Error);
assert_eq!(*issue.category().await?, "fetch");
assert_eq!(*issue.description().await?, "There was an issue establishing a connection while requesting https://doesnotexist/foo.woff.");
assert_eq!(*issue.description().await?, StyledString::String("There was an issue establishing a connection while requesting https://doesnotexist/foo.woff.".to_string()));
}
}

Expand All @@ -137,7 +137,7 @@ async fn errors_on_404() {
let issue = err_vc.to_issue(IssueSeverity::Error.into(), get_issue_context());
assert_eq!(*issue.severity().await?, IssueSeverity::Error);
assert_eq!(*issue.category().await?, "fetch");
assert_eq!(*issue.description().await?, format!("Received response with status 404 when requesting {}", &resource_url));
assert_eq!(*issue.description().await?, StyledString::String(format!("Received response with status 404 when requesting {}", &resource_url)));
}
}

Expand Down
30 changes: 27 additions & 3 deletions crates/turbopack-cli-utils/src/issue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use turbo_tasks::{RawVc, ReadRef, TransientInstance, TransientValue, TryJoinIter
use turbo_tasks_fs::{source_context::get_source_context, FileLinesContent};
use turbopack_core::issue::{
CapturedIssues, Issue, IssueReporter, IssueSeverity, PlainIssue, PlainIssueProcessingPathItem,
PlainIssueSource,
PlainIssueSource, StyledString,
};

use crate::source_context::format_source_context_lines;
Expand Down Expand Up @@ -162,7 +162,12 @@ pub fn format_issue(

let description = &plain_issue.description;
if !description.is_empty() {
writeln!(styled_issue, "\n{description}").unwrap();
writeln!(
styled_issue,
"\n{}",
render_styled_string_to_ansi(description)
)
.unwrap();
}

if log_detail {
Expand Down Expand Up @@ -418,7 +423,11 @@ impl IssueReporter for ConsoleUi {

let description = &plain_issue.description;
if !description.is_empty() {
writeln!(&mut styled_issue, "\n{description}")?;
writeln!(
&mut styled_issue,
"\n{}",
render_styled_string_to_ansi(description)
)?;
}

if log_detail {
Expand Down Expand Up @@ -570,3 +579,18 @@ fn show_all_message_with_shown_count(
.bold()
}
}

fn render_styled_string_to_ansi(styled_string: &StyledString) -> String {
match styled_string {
StyledString::Line(parts) => {
let mut string = String::new();
for part in parts {
string.push_str(&render_styled_string_to_ansi(part));
}
string
}
StyledString::String(string) => string.to_string(),
wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved
StyledString::Pre(string) => string.blue().to_string(),
StyledString::Strong(string) => string.bold().to_string(),
}
}
6 changes: 3 additions & 3 deletions crates/turbopack-core/src/issue/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use turbo_tasks::Vc;
use turbo_tasks_fs::FileSystemPath;

use super::{Issue, IssueSeverity, LazyIssueSource, OptionIssueSource};
use super::{Issue, IssueSeverity, LazyIssueSource, OptionIssueSource, StyledString};
use crate::ident::AssetIdent;

#[turbo_tasks::value(shared)]
Expand Down Expand Up @@ -43,8 +43,8 @@ impl Issue for AnalyzeIssue {
}

#[turbo_tasks::function]
fn description(&self) -> Vc<String> {
self.message
async fn description(&self) -> Result<Vc<StyledString>> {
Ok(StyledString::String(self.message.await?.to_string()).cell())
}

#[turbo_tasks::function]
Expand Down
6 changes: 3 additions & 3 deletions crates/turbopack-core/src/issue/code_gen.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
use turbo_tasks::Vc;
use turbo_tasks_fs::FileSystemPath;

use super::{Issue, IssueSeverity};
use super::{Issue, IssueSeverity, StyledString};

#[turbo_tasks::value(shared)]
pub struct CodeGenerationIssue {
pub severity: Vc<IssueSeverity>,
pub path: Vc<FileSystemPath>,
pub title: Vc<String>,
pub message: Vc<String>,
pub message: Vc<StyledString>,
}

#[turbo_tasks::value_impl]
Expand All @@ -34,7 +34,7 @@ impl Issue for CodeGenerationIssue {
}

#[turbo_tasks::function]
fn description(&self) -> Vc<String> {
fn description(&self) -> Vc<StyledString> {
self.message
}
}
42 changes: 39 additions & 3 deletions crates/turbopack-core/src/issue/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,41 @@ impl Display for IssueSeverity {
}
}

#[derive(Clone, Debug)]
#[turbo_tasks::value(shared)]
pub enum StyledString {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not going to suggest to make any changes, just random thoughts that if we could just pass raw markdown or formatted text and consumer (next-swc) can transparently renders it'd be cool

wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved
wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved
Line(Vec<StyledString>),
String(String),
Pre(String),
wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved
Strong(String),
}
wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved

impl DeterministicHash for StyledString {
fn deterministic_hash<H: turbo_tasks_hash::DeterministicHasher>(&self, state: &mut H) {
match self {
StyledString::Line(parts) => {
for part in parts {
part.deterministic_hash(state);
}
}
StyledString::String(s) | StyledString::Pre(s) | StyledString::Strong(s) => {
wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved
s.replace('\\', "/").deterministic_hash(state);
wbinnssmith marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
}

impl StyledString {
pub fn is_empty(&self) -> bool {
match self {
StyledString::Line(parts) => parts.iter().all(|part| part.is_empty()),
StyledString::String(string)
| StyledString::Pre(string)
| StyledString::Strong(string) => string.is_empty(),
}
}
}

#[turbo_tasks::value_trait]
pub trait Issue {
/// Severity allows the user to filter out unimportant issues, with Bug
Expand Down Expand Up @@ -100,7 +135,7 @@ pub trait Issue {
/// A more verbose message of the issue, appropriate for providing multiline
/// information of the issue.
// TODO add Vc<StyledString>
fn description(self: Vc<Self>) -> Vc<String>;
fn description(self: Vc<Self>) -> Vc<StyledString>;

/// Full details of the issue, appropriate for providing debug level
/// information. Only displayed if the user explicitly asks for detailed
Expand Down Expand Up @@ -487,7 +522,7 @@ pub struct PlainIssue {
pub category: String,

pub title: String,
pub description: String,
pub description: StyledString,
pub detail: String,
pub documentation_link: String,

Expand All @@ -503,7 +538,8 @@ fn hash_plain_issue(issue: &PlainIssue, hasher: &mut Xxh3Hash64Hasher, full: boo
hasher.write_ref(&issue.title);
hasher.write_ref(
// Normalize syspaths from Windows. These appear in stack traces.
&issue.description.replace('\\', "/"),
// &issue.description.replace('\\', "/"),
&issue.description,
);
hasher.write_ref(&issue.detail);
hasher.write_ref(&issue.documentation_link);
Expand Down
14 changes: 8 additions & 6 deletions crates/turbopack-core/src/issue/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use anyhow::Result;
use turbo_tasks::{ValueToString, Vc};
use turbo_tasks_fs::FileSystemPath;

use super::{Issue, OptionIssueSource};
use super::{Issue, OptionIssueSource, StyledString};
use crate::{
error::PrettyPrintError,
issue::{IssueSeverity, LazyIssueSource},
Expand Down Expand Up @@ -48,11 +48,13 @@ impl Issue for ResolvingIssue {
}

#[turbo_tasks::function]
async fn description(&self) -> Result<Vc<String>> {
Ok(Vc::cell(format!(
"unable to resolve {module_name}",
module_name = self.request.to_string().await?
)))
async fn description(&self) -> Result<Vc<StyledString>> {
Ok(StyledString::Line(vec![
StyledString::Strong("Module not found".to_string()),
StyledString::String(": Can't resolve".to_string()),
StyledString::Pre(self.request.to_string().await?.to_string()),
])
.cell())
}

#[turbo_tasks::function]
Expand Down
9 changes: 5 additions & 4 deletions crates/turbopack-core/src/issue/unsupported_module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use anyhow::Result;
use turbo_tasks::Vc;
use turbo_tasks_fs::FileSystemPath;

use super::{Issue, IssueSeverity};
use super::{Issue, IssueSeverity, StyledString};

#[turbo_tasks::value(shared)]
pub struct UnsupportedModuleIssue {
Expand Down Expand Up @@ -34,10 +34,11 @@ impl Issue for UnsupportedModuleIssue {
}

#[turbo_tasks::function]
async fn description(&self) -> Result<Vc<String>> {
Ok(Vc::cell(match &self.package_path {
async fn description(&self) -> Result<Vc<StyledString>> {
Ok(StyledString::String(match &self.package_path {
Some(path) => format!("The module {}{} is not yet supported", self.package, path),
None => format!("The package {} is not yet supported", self.package),
}))
})
.cell())
}
}
6 changes: 3 additions & 3 deletions crates/turbopack-core/src/package_json.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use turbo_tasks::{debug::ValueDebugFormat, trace::TraceRawVcs, ReadRef, Vc};
use turbo_tasks_fs::{FileContent, FileJsonContent, FileSystemPath};

use super::issue::Issue;
use crate::issue::IssueExt;
use crate::issue::{IssueExt, StyledString};

/// PackageJson wraps the parsed JSON content of a `package.json` file. The
/// wrapper is necessary so that we can reference the [FileJsonContent]'s inner
Expand Down Expand Up @@ -79,7 +79,7 @@ impl Issue for PackageJsonIssue {
}

#[turbo_tasks::function]
fn description(&self) -> Vc<String> {
Vc::cell(self.error_message.clone())
fn description(&self) -> Vc<StyledString> {
StyledString::String(self.error_message.clone()).cell()
}
}
16 changes: 8 additions & 8 deletions crates/turbopack-css/src/module_asset.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use turbopack_core::{
chunk::{ChunkItem, ChunkItemExt, ChunkType, ChunkableModule, ChunkingContext},
context::AssetContext,
ident::AssetIdent,
issue::{Issue, IssueExt, IssueSeverity},
issue::{Issue, IssueExt, IssueSeverity, StyledString},
module::Module,
reference::{ModuleReference, ModuleReferences},
reference_type::{CssReferenceSubType, ReferenceType},
Expand Down Expand Up @@ -316,12 +316,12 @@ impl EcmascriptChunkItem for ModuleChunkItem {
CssModuleComposesIssue {
severity: IssueSeverity::Error.cell(),
source: self.module.ident(),
message: Vc::cell(formatdoc! {
message: formatdoc! {
r#"
Module {from} referenced in `composes: ... from {from};` can't be resolved.
"#,
from = &*from.await?.request.to_string().await?
}),
},
}.cell().emit();
continue;
};
Expand All @@ -333,12 +333,12 @@ impl EcmascriptChunkItem for ModuleChunkItem {
CssModuleComposesIssue {
severity: IssueSeverity::Error.cell(),
source: self.module.ident(),
message: Vc::cell(formatdoc! {
message: formatdoc! {
r#"
Module {from} referenced in `composes: ... from {from};` is not a CSS module.
"#,
from = &*from.await?.request.to_string().await?
}),
},
}.cell().emit();
continue;
};
Expand Down Expand Up @@ -412,7 +412,7 @@ fn generate_minimal_source_map(filename: String, source: String) -> Vc<ParseResu
struct CssModuleComposesIssue {
severity: Vc<IssueSeverity>,
source: Vc<AssetIdent>,
message: Vc<String>,
message: String,
}

#[turbo_tasks::value_impl]
Expand Down Expand Up @@ -440,7 +440,7 @@ impl Issue for CssModuleComposesIssue {
}

#[turbo_tasks::function]
fn description(&self) -> Vc<String> {
self.message
fn description(&self) -> Vc<StyledString> {
StyledString::String(self.message.clone()).cell()
}
}
7 changes: 4 additions & 3 deletions crates/turbopack-dev-server/src/update/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ use turbopack_core::{
error::PrettyPrintError,
issue::{
Issue, IssueDescriptionExt, IssueSeverity, OptionIssueProcessingPathItems, PlainIssue,
StyledString,
},
server_fs::ServerFileSystem,
version::{
Expand Down Expand Up @@ -55,7 +56,7 @@ async fn get_update_stream_item(
plain_issues.push(
FatalStreamIssue {
resource: resource.to_string(),
description: Vc::cell(format!("{}", PrettyPrintError(&e))),
description: StyledString::String(format!("{}", PrettyPrintError(&e))).cell(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want a helper to pretty print an error directly into a styled string, so we can use formatting during pretty printing. But that's something for a follow-up PR

}
.cell()
.into_plain(OptionIssueProcessingPathItems::none())
Expand Down Expand Up @@ -279,7 +280,7 @@ pub enum UpdateStreamItem {

#[turbo_tasks::value(serialization = "none")]
struct FatalStreamIssue {
description: Vc<String>,
description: Vc<StyledString>,
resource: String,
}

Expand All @@ -306,7 +307,7 @@ impl Issue for FatalStreamIssue {
}

#[turbo_tasks::function]
fn description(&self) -> Vc<String> {
fn description(&self) -> Vc<StyledString> {
self.description
}
}
Loading
Loading