diff --git a/src/compare/compare_field.rs b/src/compare/compare_field.rs index e0f4413..05a1fdd 100644 --- a/src/compare/compare_field.rs +++ b/src/compare/compare_field.rs @@ -1,11 +1,15 @@ use std::fmt::Debug; +use super::diff::artificial_diff_scope; +use super::diff::compare_ast_node; +use super::diff::DiffEntry; use super::diff::DiffStatus; use super::sexp::unquote; use super::sexp::Token; use super::util::get_property; use super::util::get_property_quoted_string; use super::util::get_property_unquoted_atom; +use crate::types::AstNode; #[derive(Debug)] pub(crate) enum EmacsField<'s> { @@ -14,23 +18,30 @@ pub(crate) enum EmacsField<'s> { Optional(&'s str), } -/// Do no comparison. -/// -/// This is for when you want to acknowledge that a field exists in the emacs token, but you do not have any validation for it when using the compare_properties!() macro. Ideally, this should be kept to a minimum since this represents untested values. -#[allow(dead_code)] -pub(crate) fn compare_noop<'b, 's, 'x, R, RG>( - _emacs: &'b Token<'s>, - _rust_node: R, - _emacs_field: &'x str, - _rust_value_getter: RG, -) -> Result)>, Box> { - Ok(None) +#[derive(Debug)] +pub(crate) enum ComparePropertiesResult<'b, 's> { + NoChange, + /// Return when you want the status for "this" node to change (as opposed to collecting child status). + SelfChange(DiffStatus, Option), + DiffEntry(DiffEntry<'b, 's>), +} + +/// Do no comparison. +/// +/// This is for when you want to acknowledge that a field exists in the emacs token, but you do not have any validation for it when using the compare_properties!() macro. Ideally, this should be kept to a minimum since this represents untested values. +pub(crate) fn compare_noop<'b, 's, 'x, R, RG>( + _source: &'s str, + _emacs: &'b Token<'s>, + _rust_node: R, + _emacs_field: &'x str, + _rust_value_getter: RG, +) -> Result, Box> { + Ok(ComparePropertiesResult::NoChange) } /// Do no comparison. /// /// This is for when you want to acknowledge that a field exists in the emacs token, but you do not have any validation for it when using the compare_properties!() macro. Ideally, this should be kept to a minimum since this represents untested values. -#[allow(dead_code)] pub(crate) fn compare_identity() -> () { () } @@ -39,11 +50,12 @@ pub(crate) fn compare_identity() -> () { /// /// This is usually used for fields which, in my testing, are always nil. Using this compare function instead of simply doing a compare_noop will enable us to be alerted when we finally come across an org-mode document that has a value other than nil for the property. pub(crate) fn compare_property_always_nil<'b, 's, 'x, R, RG>( + _source: &'s str, emacs: &'b Token<'s>, _rust_node: R, emacs_field: &'x str, _rust_value_getter: RG, -) -> Result)>, Box> { +) -> Result, Box> { let value = get_property(emacs, emacs_field)?; if value.is_some() { let this_status = DiffStatus::Bad; @@ -51,9 +63,9 @@ pub(crate) fn compare_property_always_nil<'b, 's, 'x, R, RG>( "{} was expected to always be nil: {:?}", emacs_field, value )); - Ok(Some((this_status, message))) + Ok(ComparePropertiesResult::SelfChange(this_status, message)) } else { - Ok(None) + Ok(ComparePropertiesResult::NoChange) } } @@ -65,11 +77,12 @@ pub(crate) fn compare_property_quoted_string< RV: AsRef + std::fmt::Debug, RG: Fn(R) -> Option, >( + _source: &'s str, emacs: &'b Token<'s>, rust_node: R, emacs_field: &'x str, rust_value_getter: RG, -) -> Result)>, Box> { +) -> Result, Box> { let value = get_property_quoted_string(emacs, emacs_field)?; let rust_value = rust_value_getter(rust_node); if rust_value.as_ref().map(|s| s.as_ref()) != value.as_ref().map(String::as_str) { @@ -78,18 +91,19 @@ pub(crate) fn compare_property_quoted_string< "{} mismatch (emacs != rust) {:?} != {:?}", emacs_field, value, rust_value )); - Ok(Some((this_status, message))) + Ok(ComparePropertiesResult::SelfChange(this_status, message)) } else { - Ok(None) + Ok(ComparePropertiesResult::NoChange) } } pub(crate) fn compare_property_unquoted_atom<'b, 's, 'x, R, RG: Fn(R) -> Option<&'s str>>( + _source: &'s str, emacs: &'b Token<'s>, rust_node: R, emacs_field: &'x str, rust_value_getter: RG, -) -> Result)>, Box> { +) -> Result, Box> { let value = get_property_unquoted_atom(emacs, emacs_field)?; let rust_value = rust_value_getter(rust_node); if rust_value != value { @@ -98,9 +112,9 @@ pub(crate) fn compare_property_unquoted_atom<'b, 's, 'x, R, RG: Fn(R) -> Option< "{} mismatch (emacs != rust) {:?} != {:?}", emacs_field, value, rust_value )); - Ok(Some((this_status, message))) + Ok(ComparePropertiesResult::SelfChange(this_status, message)) } else { - Ok(None) + Ok(ComparePropertiesResult::NoChange) } } @@ -113,11 +127,12 @@ pub(crate) fn compare_property_list_of_quoted_string< RI: Iterator, RG: Fn(R) -> Option, >( + _source: &'s str, emacs: &'b Token<'s>, rust_node: R, emacs_field: &'x str, rust_value_getter: RG, -) -> Result)>, Box> { +) -> Result, Box> { let value = get_property(emacs, emacs_field)? .map(Token::as_list) .map_or(Ok(None), |r| r.map(Some))?; @@ -132,7 +147,7 @@ pub(crate) fn compare_property_list_of_quoted_string< "{} mismatch (emacs != rust) {:?} != {:?}", emacs_field, value, rust_value )); - return Ok(Some((this_status, message))); + return Ok(ComparePropertiesResult::SelfChange(this_status, message)); } (Some(el), Some(rl)) if el.len() != rl.len() => { let this_status = DiffStatus::Bad; @@ -140,7 +155,7 @@ pub(crate) fn compare_property_list_of_quoted_string< "{} mismatch (emacs != rust) {:?} != {:?}", emacs_field, value, rust_value )); - return Ok(Some((this_status, message))); + return Ok(ComparePropertiesResult::SelfChange(this_status, message)); } (Some(el), Some(rl)) => { for (e, r) in el.iter().zip(rl) { @@ -152,20 +167,21 @@ pub(crate) fn compare_property_list_of_quoted_string< "{} mismatch (emacs != rust) {:?} != {:?}. Full list: {:?} != {:?}", emacs_field, e, r, value, rust_value )); - return Ok(Some((this_status, message))); + return Ok(ComparePropertiesResult::SelfChange(this_status, message)); } } } } - Ok(None) + Ok(ComparePropertiesResult::NoChange) } pub(crate) fn compare_property_boolean<'b, 's, 'x, R, RG: Fn(R) -> bool>( + _source: &'s str, emacs: &'b Token<'s>, rust_node: R, emacs_field: &'x str, rust_value_getter: RG, -) -> Result)>, Box> { +) -> Result, Box> { // get_property already converts nil to None. let value = get_property(emacs, emacs_field)?.is_some(); let rust_value = rust_value_getter(rust_node); @@ -175,8 +191,62 @@ pub(crate) fn compare_property_boolean<'b, 's, 'x, R, RG: Fn(R) -> bool>( "{} mismatch (emacs != rust) {:?} != {:?}", emacs_field, value, rust_value )); - Ok(Some((this_status, message))) + Ok(ComparePropertiesResult::SelfChange(this_status, message)) } else { - Ok(None) + Ok(ComparePropertiesResult::NoChange) } } + +pub(crate) fn compare_property_list_of_ast_nodes< + 'b, + 's, + 'x: 'b + 's, + R, + RV: std::fmt::Debug, + RI: Iterator, + RG: Fn(R) -> Option, +>( + source: &'s str, + emacs: &'b Token<'s>, + rust_node: R, + emacs_field: &'x str, + rust_value_getter: RG, +) -> Result, Box> +where + AstNode<'b, 's>: From, +{ + let value = get_property(emacs, emacs_field)? + .map(Token::as_list) + .map_or(Ok(None), |r| r.map(Some))?; + let rust_value = rust_value_getter(rust_node); + // TODO: Seems we are needlessly coverting to a vec here. + let rust_value: Option> = rust_value.map(|it| it.collect()); + match (value, rust_value) { + (None, None) => {} + (None, rv @ Some(_)) | (Some(_), rv @ None) => { + let this_status = DiffStatus::Bad; + let message = Some(format!( + "{} mismatch (emacs != rust) {:?} != {:?}", + emacs_field, value, rv + )); + return Ok(ComparePropertiesResult::SelfChange(this_status, message)); + } + (Some(el), Some(rl)) if el.len() != rl.len() => { + let this_status = DiffStatus::Bad; + let message = Some(format!( + "{} mismatch (emacs != rust) {:?} != {:?}", + emacs_field, el, rl + )); + return Ok(ComparePropertiesResult::SelfChange(this_status, message)); + } + (Some(el), Some(rl)) => { + let mut child_status: Vec> = Vec::with_capacity(rl.len()); + for (e, r) in el.iter().zip(rl) { + child_status.push(compare_ast_node(source, e, r.into())?); + } + let diff_scope = artificial_diff_scope(emacs_field, child_status)?; + return Ok(ComparePropertiesResult::DiffEntry(diff_scope)); + } + } + Ok(ComparePropertiesResult::NoChange) +} diff --git a/src/compare/diff.rs b/src/compare/diff.rs index ca79a9e..18d1d67 100644 --- a/src/compare/diff.rs +++ b/src/compare/diff.rs @@ -8,6 +8,7 @@ use super::compare_field::compare_identity; use super::compare_field::compare_noop; use super::compare_field::compare_property_always_nil; use super::compare_field::compare_property_boolean; +use super::compare_field::compare_property_list_of_ast_nodes; use super::compare_field::compare_property_list_of_quoted_string; use super::compare_field::compare_property_quoted_string; use super::compare_field::compare_property_unquoted_atom; @@ -23,6 +24,7 @@ use super::util::get_property_boolean; use super::util::get_property_numeric; use super::util::get_property_quoted_string; use super::util::get_property_unquoted_atom; +use crate::compare::compare_field::ComparePropertiesResult; use crate::compare::compare_field::EmacsField; use crate::compare::macros::compare_properties; use crate::types::AngleLink; @@ -328,8 +330,8 @@ impl<'b, 's> DiffLayer<'b, 's> { } } -fn artificial_diff_scope<'b, 's>( - name: &'static str, +pub(crate) fn artificial_diff_scope<'b, 's>( + name: &'s str, children: Vec>, ) -> Result, Box> { Ok(DiffLayer { @@ -2598,16 +2600,22 @@ fn compare_bold<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!(emacs)? { - this_status = new_status; - message = new_message; + for diff in compare_properties!(emacs) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2632,16 +2640,22 @@ fn compare_italic<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!(emacs)? { - this_status = new_status; - message = new_message; + for diff in compare_properties!(emacs) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2666,16 +2680,22 @@ fn compare_underline<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!(emacs)? { - this_status = new_status; - message = new_message; + for diff in compare_properties!(emacs) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2683,16 +2703,18 @@ fn compare_underline<'b, 's>( } fn compare_verbatim<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b Verbatim<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -2700,16 +2722,22 @@ fn compare_verbatim<'b, 's>( |r| Some(r.contents), compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2717,16 +2745,18 @@ fn compare_verbatim<'b, 's>( } fn compare_code<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b Code<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -2734,16 +2764,22 @@ fn compare_code<'b, 's>( |r| Some(r.contents), compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2768,16 +2804,22 @@ fn compare_strike_through<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!(emacs)? { - this_status = new_status; - message = new_message; + for diff in compare_properties!(emacs) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2802,7 +2844,8 @@ fn compare_regular_link<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -2844,16 +2887,22 @@ fn compare_regular_link<'b, 's>( |r| r.get_search_option(), compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2878,7 +2927,8 @@ fn compare_radio_link<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -2911,16 +2961,22 @@ fn compare_radio_link<'b, 's>( compare_identity, compare_property_always_nil ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2945,7 +3001,8 @@ fn compare_radio_target<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -2953,16 +3010,22 @@ fn compare_radio_target<'b, 's>( |r| Some(r.value), compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -2970,16 +3033,18 @@ fn compare_radio_target<'b, 's>( } fn compare_plain_link<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b PlainLink<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3021,16 +3086,22 @@ fn compare_plain_link<'b, 's>( |r| r.search_option, compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3038,16 +3109,18 @@ fn compare_plain_link<'b, 's>( } fn compare_angle_link<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b AngleLink<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3089,16 +3162,22 @@ fn compare_angle_link<'b, 's>( |r| r.get_search_option(), compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3106,16 +3185,18 @@ fn compare_angle_link<'b, 's>( } fn compare_org_macro<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b OrgMacro<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3137,16 +3218,22 @@ fn compare_org_macro<'b, 's>( }, compare_property_list_of_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3154,16 +3241,18 @@ fn compare_org_macro<'b, 's>( } fn compare_entity<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b Entity<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3207,16 +3296,22 @@ fn compare_entity<'b, 's>( |r| r.use_brackets, compare_property_boolean ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3224,16 +3319,18 @@ fn compare_entity<'b, 's>( } fn compare_latex_fragment<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b LatexFragment<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3241,16 +3338,22 @@ fn compare_latex_fragment<'b, 's>( |r| Some(r.value), compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3258,16 +3361,18 @@ fn compare_latex_fragment<'b, 's>( } fn compare_export_snippet<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b ExportSnippet<'s>, ) -> Result, Box> { let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3280,16 +3385,22 @@ fn compare_export_snippet<'b, 's>( |r| r.contents, compare_property_quoted_string ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3314,7 +3425,8 @@ fn compare_footnote_reference<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3330,9 +3442,15 @@ fn compare_footnote_reference<'b, 's>( }), compare_property_unquoted_atom ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { @@ -3364,7 +3482,8 @@ fn compare_citation<'b, 's>( &mut message, )?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3374,24 +3493,30 @@ fn compare_citation<'b, 's>( ), ( EmacsField::Required(":prefix"), - |r| r.prefix, - compare_property_unquoted_atom + |r| Some(r.prefix.iter()), + compare_property_list_of_ast_nodes ), ( EmacsField::Required(":suffix"), - |r| r.suffix, - compare_property_unquoted_atom + |r| Some(r.suffix.iter()), + compare_property_list_of_ast_nodes ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } @@ -3399,16 +3524,18 @@ fn compare_citation<'b, 's>( } fn compare_citation_reference<'b, 's>( - _source: &'s str, + source: &'s str, emacs: &'b Token<'s>, rust: &'b CitationReference<'s>, ) -> Result, Box> { - let this_status = DiffStatus::Good; - let message = None; + let mut this_status = DiffStatus::Good; + let mut child_status = Vec::new(); + let mut message = None; assert_no_children(emacs, &mut this_status, &mut message)?; - if let Some((new_status, new_message)) = compare_properties!( + for diff in compare_properties!( + source, emacs, rust, ( @@ -3418,24 +3545,30 @@ fn compare_citation_reference<'b, 's>( ), ( EmacsField::Required(":prefix"), - |r| r.prefix, - compare_property_unquoted_atom + |r| Some(r.prefix.iter()), + compare_property_list_of_ast_nodes ), ( EmacsField::Required(":suffix"), - |r| r.suffix, - compare_property_unquoted_atom + |r| Some(r.suffix.iter()), + compare_property_list_of_ast_nodes ) - )? { - this_status = new_status; - message = new_message; + ) { + match diff { + ComparePropertiesResult::NoChange => {} + ComparePropertiesResult::SelfChange(new_status, new_message) => { + this_status = new_status; + message = new_message + } + ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry), + } } Ok(DiffResult { status: this_status, name: rust.get_elisp_name(), message, - children: Vec::new(), + children: child_status, rust_source: rust.get_source(), emacs_token: emacs, } diff --git a/src/compare/macros.rs b/src/compare/macros.rs index c71ce9d..1ebe391 100644 --- a/src/compare/macros.rs +++ b/src/compare/macros.rs @@ -30,10 +30,9 @@ /// } /// ``` macro_rules! compare_properties { - ($emacs:expr, $rust:expr, $(($emacs_field:expr, $rust_value_getter:expr, $compare_fn: expr)),+) => { + ($source:expr, $emacs:expr, $rust:expr, $(($emacs_field:expr, $rust_value_getter:expr, $compare_fn: expr)),+) => { { - let mut this_status = DiffStatus::Good; - let mut message: Option = None; + let mut new_status = Vec::new(); let children = $emacs.as_list()?; let attributes_child = children .iter() @@ -44,10 +43,9 @@ macro_rules! compare_properties { if emacs_keys.contains(":standard-properties") { emacs_keys.remove(":standard-properties"); } else { - this_status = DiffStatus::Bad; - message = Some(format!( + new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!( "Emacs token lacks :standard-properties field.", - )); + )))); } $( match $emacs_field { @@ -58,11 +56,10 @@ macro_rules! compare_properties { emacs_keys.remove(name); }, EmacsField::Required(name) => { - this_status = DiffStatus::Bad; - message = Some(format!( + new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!( "Emacs token lacks required field: {}", name - )); + )))); }, EmacsField::Optional(_name) => {}, } @@ -71,11 +68,10 @@ macro_rules! compare_properties { if !emacs_keys.is_empty() { let unexpected_keys: Vec<&str> = emacs_keys.into_iter().collect(); let unexpected_keys = unexpected_keys.join(", "); - this_status = DiffStatus::Bad; - message = Some(format!( + new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!( "Emacs token had extra field(s): {}", unexpected_keys - )); + )))); } $( @@ -87,33 +83,23 @@ macro_rules! compare_properties { name }, }; - let result = $compare_fn($emacs, $rust, emacs_name, $rust_value_getter)?; + let result = $compare_fn($source, $emacs, $rust, emacs_name, $rust_value_getter)?; match result { - Some((DiffStatus::Good, _)) => unreachable!("No comparison functions should return Some() when DiffStatus is good."), - Some((status, msg)) => { - this_status = status; - message = msg; - }, - _ => {} + ComparePropertiesResult::SelfChange(DiffStatus::Good, _) => unreachable!("No comparison functions should return SelfChange() when DiffStatus is good."), + ComparePropertiesResult::NoChange => {}, + result => { + new_status.push(result); + } } )+ - match this_status { - DiffStatus::Good => { - let result: Result<_, Box> = Ok(None); - result - }, - _ => { - Ok(Some((this_status, message))) - } - } + new_status } }; // Default case for when there are no expected properties except for :standard-properties ($emacs:expr) => { { - let mut this_status = DiffStatus::Good; - let mut message: Option = None; + let mut new_status = Vec::new(); let children = $emacs.as_list()?; let attributes_child = children .iter() @@ -124,30 +110,20 @@ macro_rules! compare_properties { if emacs_keys.contains(":standard-properties") { emacs_keys.remove(":standard-properties"); } else { - this_status = DiffStatus::Bad; - message = Some(format!( + new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!( "Emacs token lacks :standard-properties field.", - )); + )))); } if !emacs_keys.is_empty() { let unexpected_keys: Vec<&str> = emacs_keys.into_iter().collect(); let unexpected_keys = unexpected_keys.join(", "); - this_status = DiffStatus::Bad; - message = Some(format!( + new_status.push(ComparePropertiesResult::SelfChange(DiffStatus::Bad, Some(format!( "Emacs token had extra field(s): {}", unexpected_keys - )); - } - match this_status { - DiffStatus::Good => { - let result: Result<_, Box> = Ok(None); - result - }, - _ => { - Ok(Some((this_status, message))) - } + )))); } + new_status } }; }