Apply more suggestions.

This commit is contained in:
Tom Alexander 2023-10-16 19:02:34 -04:00
parent 880b00ef3f
commit acd24d6198
Signed by: talexander
GPG Key ID: D3A179C9A53C0EDE
3 changed files with 19 additions and 33 deletions

View File

@ -56,11 +56,11 @@ impl<'b, 's> ComparePropertiesResult<'b, 's> {
/// Do no comparison. /// 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. /// 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>( pub(crate) fn compare_noop<'b, 's, R, RG>(
_source: &'s str, _source: &'s str,
_emacs: &'b Token<'s>, _emacs: &'b Token<'s>,
_rust_node: R, _rust_node: R,
_emacs_field: &'x str, _emacs_field: &str,
_rust_value_getter: RG, _rust_value_getter: RG,
) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> { ) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> {
Ok(ComparePropertiesResult::NoChange) Ok(ComparePropertiesResult::NoChange)
@ -74,11 +74,11 @@ pub(crate) fn compare_identity() {}
/// Assert that the emacs value is always nil or absent. /// Assert that the emacs value is always nil or absent.
/// ///
/// 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. /// 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>( pub(crate) fn compare_property_always_nil<'b, 's, R, RG>(
_source: &'s str, _source: &'s str,
emacs: &'b Token<'s>, emacs: &'b Token<'s>,
_rust_node: R, _rust_node: R,
emacs_field: &'x str, emacs_field: &str,
_rust_value_getter: RG, _rust_value_getter: RG,
) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> { ) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> {
let value = get_property(emacs, emacs_field)?; let value = get_property(emacs, emacs_field)?;
@ -97,7 +97,6 @@ pub(crate) fn compare_property_always_nil<'b, 's, 'x, R, RG>(
pub(crate) fn compare_property_quoted_string< pub(crate) fn compare_property_quoted_string<
'b, 'b,
's, 's,
'x,
R, R,
RV: AsRef<str> + std::fmt::Debug, RV: AsRef<str> + std::fmt::Debug,
RG: Fn(R) -> Option<RV>, RG: Fn(R) -> Option<RV>,
@ -105,12 +104,12 @@ pub(crate) fn compare_property_quoted_string<
_source: &'s str, _source: &'s str,
emacs: &'b Token<'s>, emacs: &'b Token<'s>,
rust_node: R, rust_node: R,
emacs_field: &'x str, emacs_field: &str,
rust_value_getter: RG, rust_value_getter: RG,
) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> { ) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> {
let value = get_property_quoted_string(emacs, emacs_field)?; let value = get_property_quoted_string(emacs, emacs_field)?;
let rust_value = rust_value_getter(rust_node); 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) { if rust_value.as_ref().map(|s| s.as_ref()) != value.as_deref() {
let this_status = DiffStatus::Bad; let this_status = DiffStatus::Bad;
let message = Some(format!( let message = Some(format!(
"{} mismatch (emacs != rust) {:?} != {:?}", "{} mismatch (emacs != rust) {:?} != {:?}",
@ -178,7 +177,6 @@ where
pub(crate) fn compare_property_list_of_quoted_string< pub(crate) fn compare_property_list_of_quoted_string<
'b, 'b,
's, 's,
'x,
R, R,
RV: AsRef<str> + std::fmt::Debug, RV: AsRef<str> + std::fmt::Debug,
RI: Iterator<Item = RV>, RI: Iterator<Item = RV>,
@ -187,7 +185,7 @@ pub(crate) fn compare_property_list_of_quoted_string<
_source: &'s str, _source: &'s str,
emacs: &'b Token<'s>, emacs: &'b Token<'s>,
rust_node: R, rust_node: R,
emacs_field: &'x str, emacs_field: &str,
rust_value_getter: RG, rust_value_getter: RG,
) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> { ) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> {
let value = get_property(emacs, emacs_field)? let value = get_property(emacs, emacs_field)?
@ -269,10 +267,7 @@ pub(crate) fn compare_property_set_of_quoted_string<
.map(unquote) .map(unquote)
.collect::<Result<Vec<_>, _>>()?; .collect::<Result<Vec<_>, _>>()?;
let value: BTreeSet<&str> = value.iter().map(|e| e.as_str()).collect(); let value: BTreeSet<&str> = value.iter().map(|e| e.as_str()).collect();
let mismatched: Vec<_> = value let mismatched: Vec<_> = value.symmetric_difference(&rust_value).copied().collect();
.symmetric_difference(&rust_value)
.map(|val| *val)
.collect();
if !mismatched.is_empty() { if !mismatched.is_empty() {
let this_status = DiffStatus::Bad; let this_status = DiffStatus::Bad;
let message = Some(format!( let message = Some(format!(
@ -288,7 +283,6 @@ pub(crate) fn compare_property_set_of_quoted_string<
pub(crate) fn compare_property_optional_pair< pub(crate) fn compare_property_optional_pair<
'b, 'b,
's, 's,
'x,
R, R,
RV: AsRef<str> + std::fmt::Debug, RV: AsRef<str> + std::fmt::Debug,
ROV: AsRef<str> + std::fmt::Debug, ROV: AsRef<str> + std::fmt::Debug,
@ -297,7 +291,7 @@ pub(crate) fn compare_property_optional_pair<
_source: &'s str, _source: &'s str,
emacs: &'b Token<'s>, emacs: &'b Token<'s>,
rust_node: R, rust_node: R,
emacs_field: &'x str, emacs_field: &str,
rust_value_getter: RG, rust_value_getter: RG,
) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> { ) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> {
let value = get_property(emacs, emacs_field)? let value = get_property(emacs, emacs_field)?
@ -386,11 +380,11 @@ pub(crate) fn compare_property_optional_pair<
Ok(ComparePropertiesResult::NoChange) Ok(ComparePropertiesResult::NoChange)
} }
pub(crate) fn compare_property_boolean<'b, 's, 'x, R, RG: Fn(R) -> bool>( pub(crate) fn compare_property_boolean<'b, 's, R, RG: Fn(R) -> bool>(
_source: &'s str, _source: &'s str,
emacs: &'b Token<'s>, emacs: &'b Token<'s>,
rust_node: R, rust_node: R,
emacs_field: &'x str, emacs_field: &str,
rust_value_getter: RG, rust_value_getter: RG,
) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> { ) -> Result<ComparePropertiesResult<'b, 's>, Box<dyn std::error::Error>> {
// get_property already converts nil to None. // get_property already converts nil to None.
@ -474,14 +468,7 @@ where
match (value, rust_value) { match (value, rust_value) {
(None, None) => {} (None, None) => {}
(Some(el), None) (Some(el), None)
if el.len() == 1 if el.len() == 1 && el.iter().all(|t| matches!(t.as_atom(), Ok(r#""""#))) => {}
&& el.into_iter().all(|t| {
if let Ok(r#""""#) = t.as_atom() {
true
} else {
false
}
}) => {}
(None, rv @ Some(_)) | (Some(_), rv @ None) => { (None, rv @ Some(_)) | (Some(_), rv @ None) => {
let this_status = DiffStatus::Bad; let this_status = DiffStatus::Bad;
let message = Some(format!( let message = Some(format!(
@ -558,13 +545,13 @@ where
}; };
let mut full_status: Vec<DiffEntry<'b, 's>> = Vec::with_capacity(outer_rust_list.len()); let mut full_status: Vec<DiffEntry<'b, 's>> = Vec::with_capacity(outer_rust_list.len());
for (kw_e, kw_r) in outer_emacs_list.into_iter().zip(outer_rust_list) { for (kw_e, kw_r) in outer_emacs_list.iter().zip(outer_rust_list) {
let kw_e = kw_e.as_list()?; let kw_e = kw_e.as_list()?;
let child_status_length = kw_r.1.len() + kw_r.0.as_ref().map(|opt| opt.len()).unwrap_or(0); let child_status_length = kw_r.1.len() + kw_r.0.as_ref().map(|opt| opt.len()).unwrap_or(0);
let mut child_status: Vec<DiffEntry<'b, 's>> = Vec::with_capacity(child_status_length); let mut child_status: Vec<DiffEntry<'b, 's>> = Vec::with_capacity(child_status_length);
if let Some(or) = &kw_r.0 { if let Some(or) = &kw_r.0 {
// if optional value // if optional value
let mut kw_e = kw_e.into_iter(); let mut kw_e = kw_e.iter();
// First element is a list representing the mandatory value. // First element is a list representing the mandatory value.
if let Some(val_e) = kw_e.next() { if let Some(val_e) = kw_e.next() {
let el = val_e.as_list()?; let el = val_e.as_list()?;
@ -576,7 +563,7 @@ where
)); ));
return Ok(ComparePropertiesResult::SelfChange(this_status, message)); return Ok(ComparePropertiesResult::SelfChange(this_status, message));
} }
for (e, r) in el.into_iter().zip(kw_r.1.iter()) { for (e, r) in el.iter().zip(kw_r.1.iter()) {
child_status.push(compare_ast_node(source, e, r.into())?); child_status.push(compare_ast_node(source, e, r.into())?);
} }
} else { } else {

View File

@ -426,11 +426,9 @@ pub(crate) fn compare_ast_node<'b, 's>(
// PlainText is a special case because upstream Org-Mode uses relative values for the bounds in plaintext rather than absolute so the below checks do not account for that. // PlainText is a special case because upstream Org-Mode uses relative values for the bounds in plaintext rather than absolute so the below checks do not account for that.
if let AstNode::PlainText(_) = rust { if let AstNode::PlainText(_) = rust {
} else { } else if let Err(err) = compare_standard_properties(source, emacs, &rust) {
if let Err(err) = compare_standard_properties(source, emacs, &rust) { compare_result.status = DiffStatus::Bad;
compare_result.status = DiffStatus::Bad; compare_result.message = Some(err.to_string())
compare_result.message = Some(err.to_string())
}
} }
Ok(compare_result.into()) Ok(compare_result.into())

View File

@ -1,3 +1,4 @@
#[allow(clippy::module_inception)]
mod compare; mod compare;
mod compare_field; mod compare_field;
mod diff; mod diff;