-
Notifications
You must be signed in to change notification settings - Fork 147
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
Add support overriding block id/tag/hash
with a named fork binding
#2475
base: master
Are you sure you want to change the base?
Changes from 26 commits
2b30e7e
65d538a
e72575f
93570c5
211ed29
5c2b75a
31ecfd4
ae95ac9
b9eae03
d645eae
ce464c1
1f145bf
aa84686
1ab55ae
cfe6cd5
0afafe8
db9d4a0
599c287
33e7fe5
1be79cc
22ad6e6
bc71b0b
fb13499
a81b886
bb35f24
b1d0a8f
b24957c
7abdc48
0a05d66
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,30 +1,55 @@ | ||
use cairo_lang_macro::{Diagnostic, Severity}; | ||
use indoc::formatdoc; | ||
|
||
fn higher_severity(a: Severity, b: Severity) -> Severity { | ||
pub fn higher_severity(a: Severity, b: Severity) -> Severity { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why was this done? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This has to be changed because the macro uses this function |
||
match (a, b) { | ||
(Severity::Warning, Severity::Warning) => Severity::Warning, | ||
_ => Severity::Error, | ||
} | ||
} | ||
pub fn format_error_message(variants: &[Diagnostic]) -> String { | ||
let formatted_variants: Vec<String> = variants | ||
.iter() | ||
.map(|variant| format!("- variant: {}", variant.message)) | ||
.collect(); | ||
|
||
pub fn branch( | ||
left: Result<String, Diagnostic>, | ||
right: impl Fn() -> Result<String, Diagnostic>, | ||
) -> Result<String, Diagnostic> { | ||
left.or_else(|error| { | ||
right().map_err(|next_error| Diagnostic { | ||
severity: higher_severity(error.severity, next_error.severity), | ||
message: formatdoc!( | ||
" | ||
Both options failed | ||
First variant: {} | ||
Second variant: {} | ||
Resolve at least one of them | ||
", | ||
error.message, | ||
next_error.message | ||
), | ||
}) | ||
}) | ||
formatdoc! {" | ||
All options failed | ||
{} | ||
Resolve at least one of them | ||
", formatted_variants.join("\n")} | ||
} | ||
|
||
/// The `branch` macro is used to evaluate multiple expressions and return the first successful result. | ||
/// If all expressions fail, it collects the error messages and returns a combined error. | ||
/// | ||
/// This macro is used instead of a function because it can perform lazy evaluation and has better readability. | ||
#[macro_export] | ||
macro_rules! branch { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know why this is a macro. It would be more readable as a function. pub fn branch(results: Vec<Result<String, Diagnostic>>) -> Result<String, Diagnostic> {
let mut messages: Vec<Diagnostic> = Vec::new();
for result in results {
match result {
Ok(val) => return Ok(val),
Err(err) => messages.push(err),
}
}
Err(Diagnostic {
message: format_error_message(&messages),
severity: messages.iter().fold(Severity::Warning, |acc, diagnostic| {
higher_severity(acc, diagnostic.severity)
}),
})
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Macro is lazy There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see it now. At lease give a comment that this is way as it is not obvious to caller There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Still can be done with function though pub fn branch(results: Vec<Box<dyn Fn() -> Result<String, Diagnostic>>>) -> Result<String, Diagnostic> {
let mut messages: Vec<Diagnostic> = Vec::new();
for result in results {
match result() {
Ok(val) => return Ok(val),
Err(err) => messages.push(err),
}
}
Err(Diagnostic {
message: format_error_message(&messages),
severity: messages.iter().fold(Severity::Warning, |acc, diagnostic| {
higher_severity(acc, diagnostic.severity)
}),
})
} There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll stay with the macro because it has better readability There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem 😄. But I think you are mistaking verbosity for readability. |
||
($($result:expr),+) => {{ | ||
let mut messages = Vec::new(); | ||
let mut result = None; | ||
|
||
$( | ||
if result.is_none() { | ||
match $result { | ||
Ok(val) => { | ||
result = Some(val); | ||
}, | ||
Err(err) => { | ||
messages.push(err); | ||
}, | ||
} | ||
} | ||
)+ | ||
|
||
if let Some(result) = result { | ||
Ok(result) | ||
} else { | ||
Err(Diagnostic { | ||
message: $crate::utils::format_error_message(&messages), | ||
severity: messages.into_iter().fold(Severity::Warning, |acc, diagnostic| $crate::utils::higher_severity(acc, diagnostic.severity)) | ||
}) | ||
} | ||
}}; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd change it to something stable. Let's use specific block number, just different that it is used to override in test.