From fb6379a8ba4192139b07cc08989ebd69ceab2976 Mon Sep 17 00:00:00 2001 From: Fabrice Reix Date: Mon, 3 Jun 2024 11:47:05 +0200 Subject: [PATCH] Update fixme to return a StyledString --- packages/hurl/src/output/error.rs | 29 ++-- packages/hurl/src/runner/error.rs | 175 ++++++------------------- packages/hurl_core/src/error/mod.rs | 50 ++++--- packages/hurl_core/src/parser/error.rs | 12 +- packages/hurlfmt/src/cli/logger.rs | 14 +- packages/hurlfmt/src/linter/error.rs | 10 +- 6 files changed, 99 insertions(+), 191 deletions(-) diff --git a/packages/hurl/src/output/error.rs b/packages/hurl/src/output/error.rs index 3006fa962..d7fea6b58 100644 --- a/packages/hurl/src/output/error.rs +++ b/packages/hurl/src/output/error.rs @@ -17,9 +17,9 @@ */ use crate::http::HttpError; -use colored::Colorize; use hurl_core::ast::SourceInfo; use hurl_core::error::DisplaySourceError; +use hurl_core::text::{Style, StyledString}; #[derive(Clone, Debug, PartialEq, Eq)] pub struct OutputError { @@ -54,35 +54,22 @@ impl DisplaySourceError for OutputError { } } - fn fixme(&self, content: &[&str], color: bool) -> String { + fn fixme(&self, content: &[&str]) -> StyledString { match &self.kind { OutputErrorKind::Http(http_error) => { let message = http_error.message(); let message = hurl_core::error::add_carets(&message, self.source_info, content); - - if color { - message.red().bold().to_string() - } else { - message - } + color_red(&message) } OutputErrorKind::Binary => { let message = "Binary output can mess up your terminal. Use \"--output -\" to tell Hurl to output it to your terminal anyway, or consider \"--output\" to save to a file."; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red(&message) } OutputErrorKind::Io(message) => { let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red(&message) } } } @@ -91,3 +78,9 @@ impl DisplaySourceError for OutputError { true } } + +fn color_red(message: &str) -> StyledString { + let mut s = StyledString::new(); + s.push_with(message, Style::new().red().bold()); + s +} diff --git a/packages/hurl/src/runner/error.rs b/packages/hurl/src/runner/error.rs index 76585b675..3fd820b8d 100644 --- a/packages/hurl/src/runner/error.rs +++ b/packages/hurl/src/runner/error.rs @@ -15,11 +15,11 @@ * limitations under the License. * */ -use colored::Colorize; use std::path::PathBuf; use hurl_core::ast::SourceInfo; use hurl_core::error::DisplaySourceError; +use hurl_core::text::{Style, StyledString}; use crate::http::HttpError; @@ -150,16 +150,12 @@ impl DisplaySourceError for RunnerError { } } - fn fixme(&self, content: &[&str], color: bool) -> String { + fn fixme(&self, content: &[&str]) -> StyledString { match &self.kind { RunnerErrorKind::AssertBodyValueError { actual, .. } => { let message = &format!("actual value is <{actual}>"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - color_red_multiline_string(&message) - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::AssertFailure { actual, @@ -173,195 +169,110 @@ impl DisplaySourceError for RunnerError { "" }; let message = format!(" actual: {actual}\n expected: {expected}{additional}"); - if color { - color_red_multiline_string(&message) - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::AssertHeaderValueError { actual } => { let message = &format!("actual value is <{actual}>"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::AssertStatus { actual, .. } => { let message = &format!("actual value is <{actual}>"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::AssertVersion { actual, .. } => { let message = &format!("actual value is <{actual}>"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::FileReadAccess { path } => { let message = &format!("file {} can not be read", path.to_string_lossy()); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::FileWriteAccess { path, error } => { let message = &format!("{} can not be written ({error})", path.to_string_lossy()); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::FilterDecode(encoding) => { let message = &format!("value can not be decoded with <{encoding}> encoding"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::FilterInvalidEncoding(encoding) => { let message = &format!("<{encoding}> encoding is not supported"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::FilterInvalidInput(message) => { let message = &format!("invalid filter input: {message}"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::FilterMissingInput => { let message = "missing value to apply filter"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::Http(http_error) => { let message = http_error.message(); let message = hurl_core::error::add_carets(&message, self.source_info, content); - - if color { - message.red().bold().to_string() - } else { - message - } + color_red_multiline_string(&message) } RunnerErrorKind::InvalidJson { value } => { let message = &format!("actual value is <{value}>"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::InvalidRegex => { let message = "regex expression is not valid"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::NoQueryResult => { let message = "The query didn't return any result"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::QueryHeaderNotFound => { let message = "this header has not been found in the response"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::QueryInvalidJson => { let message = "the HTTP response is not a valid JSON"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::QueryInvalidJsonpathExpression { value } => { let message = &format!("the JSONPath expression '{value}' is not valid"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::QueryInvalidXml => { let message = "the HTTP response is not a valid XML"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::QueryInvalidXpathEval => { let message = "the XPath expression is not valid"; let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::TemplateVariableInvalidType { value, expecting, .. } => { let message = &format!("expecting {expecting}, actual value is <{value}>"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::TemplateVariableNotDefined { name } => { let message = &format!("you must set the variable {name}"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::UnauthorizedFileAccess { path } => { let message = &format!( @@ -369,20 +280,12 @@ impl DisplaySourceError for RunnerError { path.to_string_lossy() ); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } RunnerErrorKind::UnrenderableVariable { name, value } => { let message = &format!("variable <{name}> with value {value} can not be rendered"); let message = hurl_core::error::add_carets(message, self.source_info, content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + color_red_multiline_string(&message) } } } @@ -393,28 +296,24 @@ impl DisplaySourceError for RunnerError { } /// Color each line separately -fn color_red_multiline_string(s: &str) -> String { - let lines = split_lines(s); - lines - .iter() - .map(|line| line.red().bold().to_string()) - .collect::>() - .join("\n") -} - -/// Splits this `text` to a list of LF/CRLF separated lines. -fn split_lines(text: &str) -> Vec<&str> { - regex::Regex::new(r"\n|\r\n").unwrap().split(text).collect() +fn color_red_multiline_string(s: &str) -> StyledString { + let lines = s.split('\n'); + let mut s = StyledString::new(); + for (i, line) in lines.enumerate() { + if i > 0 { + s.push("\n"); + } + s.push_with(line, Style::new().red().bold()); + } + s } #[cfg(test)] mod tests { - use super::*; - use crate::http::HttpError; use crate::runner::{RunnerError, RunnerErrorKind}; use hurl_core::ast::{Pos, SourceInfo}; - use hurl_core::error::{error_string, get_message}; + use hurl_core::error::{error_string, get_message, split_lines}; #[test] fn test_error_timeout() { diff --git a/packages/hurl_core/src/error/mod.rs b/packages/hurl_core/src/error/mod.rs index 636766174..5aff8d5ac 100644 --- a/packages/hurl_core/src/error/mod.rs +++ b/packages/hurl_core/src/error/mod.rs @@ -16,13 +16,14 @@ * */ use crate::ast::SourceInfo; +use crate::text::{Format, StyledString}; use colored::Colorize; use std::cmp::max; pub trait DisplaySourceError { fn source_info(&self) -> SourceInfo; fn description(&self) -> String; - fn fixme(&self, content: &[&str], color: bool) -> String; + fn fixme(&self, content: &[&str]) -> StyledString; fn show_source_line(&self) -> bool; } @@ -196,24 +197,31 @@ pub fn error_string( /// } /// pub fn get_message(error: &E, lines: &[&str], colored: bool) -> String { - let mut text = String::new(); + let mut text = StyledString::new(); if error.show_source_line() { let line = lines.get(error.source_info().start.line - 1).unwrap(); let line = line.replace('\t', " "); - text.push(' '); - text.push_str(&line); - text.push('\n'); + + text.push(" "); + + text.push(&line); + + text.push("\n"); } - let fixme = error.fixme(lines, colored); - let lines = split_lines(&fixme); + let fixme = error.fixme(lines); + let lines = fixme.split('\n'); for (i, line) in lines.iter().enumerate() { if i > 0 { - text.push('\n'); + text.push("\n"); } - text.push_str(line); + text.append(line.clone()); + } + if colored { + text.to_string(Format::Ansi) + } else { + text.to_string(Format::Plain) } - text } /// Splits this `text` to a list of LF/CRLF separated lines. @@ -245,6 +253,7 @@ fn get_carets(line_raw: &str, error_column: usize, width: usize) -> String { mod tests { use super::*; use crate::ast::Pos; + use crate::text::Style; #[test] fn test_add_carets() { @@ -307,14 +316,14 @@ HTTP 200 "Assert body value".to_string() } - fn fixme(&self, _lines: &[&str], _color: bool) -> String { - r#" { - "name": "John", -- "age": 27 -+ "age": 28 - } -"# - .to_string() + fn fixme(&self, _lines: &[&str]) -> StyledString { + let mut diff = StyledString::new(); + diff.push(" {\n \"name\": \"John\",\n"); + diff.push_with("- \"age\": 27", Style::new().red()); + diff.push("\n"); + diff.push_with("+ \"age\": 28", Style::new().green()); + diff.push("\n }\n"); + diff } fn show_source_line(&self) -> bool { @@ -323,6 +332,7 @@ HTTP 200 } let error = E; + colored::control::set_override(true); assert_eq!( get_message(&error, &split_lines(content), false), r#" { @@ -332,6 +342,10 @@ HTTP 200 } "# ); + assert_eq!( + get_message(&error, &split_lines(content), true), + " {\n \"name\": \"John\",\n\u{1b}[31m- \"age\": 27\u{1b}[0m\n\u{1b}[32m+ \"age\": 28\u{1b}[0m\n }\n" + ); assert_eq!( error_string(filename, content, &error, None, false), diff --git a/packages/hurl_core/src/parser/error.rs b/packages/hurl_core/src/parser/error.rs index 22d2929ad..a62cb330b 100644 --- a/packages/hurl_core/src/parser/error.rs +++ b/packages/hurl_core/src/parser/error.rs @@ -17,7 +17,7 @@ */ use crate::ast::{Pos, SourceInfo}; use crate::error::DisplaySourceError; -use colored::Colorize; +use crate::text::{Style, StyledString}; use std::cmp; /// Represents a parser error. @@ -135,7 +135,7 @@ impl DisplaySourceError for ParseError { } } - fn fixme(&self, content: &[&str], color: bool) -> String { + fn fixme(&self, content: &[&str]) -> StyledString { let message = match &self.kind { ParseErrorKind::DuplicateSection => "the section is already defined".to_string(), ParseErrorKind::EscapeChar => "the escaping sequence is not valid".to_string(), @@ -246,11 +246,9 @@ impl DisplaySourceError for ParseError { }; let message = crate::error::add_carets(&message, self.source_info(), content); - if color { - message.red().bold().to_string() - } else { - message.to_string() - } + let mut s = StyledString::new(); + s.push_with(&message, Style::new().red().bold()); + s } fn show_source_line(&self) -> bool { diff --git a/packages/hurlfmt/src/cli/logger.rs b/packages/hurlfmt/src/cli/logger.rs index 71a182a32..d7784378d 100644 --- a/packages/hurlfmt/src/cli/logger.rs +++ b/packages/hurlfmt/src/cli/logger.rs @@ -21,6 +21,7 @@ use crate::linter; use colored::*; use hurl_core::error::DisplaySourceError; use hurl_core::parser; +use hurl_core::text::Format; pub fn make_logger_verbose(verbose: bool) -> impl Fn(&str) { move |message| log_verbose(verbose, message) @@ -101,6 +102,7 @@ fn log_error( } else { error_type.red().bold().to_string() }; + let format = if color { Format::Ansi } else { Format::Plain }; eprintln!("{}: {}", error_type, error.description()); if let Some(filename) = filename { @@ -131,17 +133,15 @@ fn log_error( // specific case for assert errors let lines = lines.iter().map(|s| s.as_str()).collect::>(); if error.source_info().start.column == 0 { - let fix_me = &error.fixme(&lines, color); - let fixme_lines: Vec<&str> = regex::Regex::new(r"\n|\r\n") - .unwrap() - .split(fix_me) - .collect(); + let fix_me = &error.fixme(&lines); + let fixme_lines = fix_me.split('\n'); + // edd an empty line at the end? for line in fixme_lines { eprintln!( "{} | {fixme}", " ".repeat(line_number_size).as_str(), - fixme = line, + fixme = line.to_string(format), ); } } else { @@ -162,7 +162,7 @@ fn log_error( " ".repeat(line_number_size).as_str(), " ".repeat(error.source_info().start.column - 1 + tab_shift * 3), "^".repeat(if width > 1 { width } else { 1 }), - fixme = error.fixme(&lines, color).as_str(), + fixme = error.fixme(&lines).to_string(format), ); } diff --git a/packages/hurlfmt/src/linter/error.rs b/packages/hurlfmt/src/linter/error.rs index 4f9e60945..909e48eba 100644 --- a/packages/hurlfmt/src/linter/error.rs +++ b/packages/hurlfmt/src/linter/error.rs @@ -17,6 +17,7 @@ */ use hurl_core::ast::SourceInfo; use hurl_core::error::DisplaySourceError; +use hurl_core::text::StyledString; use crate::linter; use crate::linter::LinterError; @@ -37,12 +38,15 @@ impl DisplaySourceError for linter::Error { } } - fn fixme(&self, _lines: &[&str], _color: bool) -> String { - match self.inner { + fn fixme(&self, _lines: &[&str]) -> StyledString { + let message = match self.inner { LinterError::UnnecessarySpace => "Remove space".to_string(), LinterError::UnnecessaryJsonEncoding => "Use Simple String".to_string(), LinterError::OneSpace => "Use only one space".to_string(), - } + }; + let mut s = StyledString::new(); + s.push(&message); + s } fn show_source_line(&self) -> bool {