Merge branch 'compare_properties_document'

src/compare/diff.rs

@@ -23,6 +23,7 @@ use super::sexp::unquote;
 use super::sexp::Token;
 use super::util::assert_no_children;
 use super::util::compare_children;
+use super::util::compare_children_iter;
 use super::util::compare_standard_properties;
 use super::util::get_property;
 use super::util::get_property_boolean;
@@ -438,99 +439,45 @@ pub fn compare_document<'b, 's>(
 }
 
 fn _compare_document<'b, 's>(
-    _source: &'s str,
+    source: &'s str,
     emacs: &'b Token<'s>,
     rust: &'b Document<'s>,
 ) -> Result<DiffEntry<'b, 's>, Box<dyn std::error::Error>> {
-    let children = emacs.as_list()?;
-    let mut child_status = Vec::new();
     let mut this_status = DiffStatus::Good;
+    let mut child_status = Vec::new();
     let mut message = None;
 
-    // Compare :path
+    compare_children_iter(
-    // :path is a quoted string to the absolute path of the document.
+        source,
-    let document_path = get_property_quoted_string(emacs, ":path")?;
+        emacs,
-    let rust_document_path = rust.path.as_ref().map(|p| p.to_str()).flatten();
+        rust.into_iter(),
-    match (
+        &mut child_status,
-        document_path.as_ref().map(|s| s.as_str()),
+        &mut this_status,
-        rust_document_path,
+        &mut message,
+    )?;
+
+    for diff in compare_properties!(
+        source,
+        emacs,
+        rust,
+        (
+            EmacsField::Required(":path"),
+            |r| r.path.as_ref().map(|p| p.to_str()).flatten(),
+            compare_property_quoted_string
+        ),
+        (
+            EmacsField::Required(":CATEGORY"),
+            |r| r.category.as_ref(),
+            compare_property_quoted_string
+        )
     ) {
-        (None, None) => {}
+        match diff {
-        (None, Some(_)) | (Some(_), None) => {
+            ComparePropertiesResult::NoChange => {}
-            this_status = DiffStatus::Bad;
+            ComparePropertiesResult::SelfChange(new_status, new_message) => {
-            message = Some(format!(
+                this_status = new_status;
-                "Path mismatch (emacs != rust) {:?} != {:?}",
+                message = new_message
-                document_path, rust_document_path
-            ));
-        }
-        (Some(e), Some(r)) if e != r => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Path mismatch (emacs != rust) {:?} != {:?}",
-                document_path, rust_document_path
-            ));
-        }
-        (Some(_), Some(_)) => {}
-    };
-
-    // Compare category
-    // :CATEGORY is specified either from "#+CATEGORY:" or it is the file name without the ".org" extension.
-    let category = get_property_quoted_string(emacs, ":CATEGORY")?;
-    match (category.as_ref(), rust.category.as_ref()) {
-        (None, None) => {}
-        (None, Some(_)) | (Some(_), None) => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Category mismatch (emacs != rust) {:?} != {:?}",
-                category, rust.category
-            ));
-        }
-        (Some(e), Some(r)) if e != r => {
-            this_status = DiffStatus::Bad;
-            message = Some(format!(
-                "Category mismatch (emacs != rust) {:?} != {:?}",
-                category, rust.category
-            ));
-        }
-        (Some(_), Some(_)) => {}
-    };
-
-    // Skipping "org-data" and its properties
-    for (i, token) in children.iter().skip(2).enumerate() {
-        let section_or_headline = token.as_list()?;
-        let first_cell = section_or_headline
-            .first()
-            .ok_or("Should have at least one child.")?
-            .as_atom()?;
-        if first_cell == "section" {
-            if i != 0 {
-                return Err("Section cannot be after the first child of document.".into());
             }
-            child_status.push(compare_ast_node(
+            ComparePropertiesResult::DiffEntry(diff_entry) => child_status.push(diff_entry),
-                rust.source,
-                token,
-                rust.zeroth_section
-                    .as_ref()
-                    .ok_or("No corresponding zeroth-section")?
-                    .into(),
-            )?);
-        } else if first_cell == "headline" {
-            let corresponding_heading = rust
-                .children
-                .iter()
-                .nth(i - rust.zeroth_section.as_ref().map(|_| 1).unwrap_or(0))
-                .ok_or("Should have a corresponding heading.")?;
-            child_status.push(compare_ast_node(
-                rust.source,
-                token,
-                corresponding_heading.into(),
-            )?);
-        } else {
-            return Err(format!(
-                "Document should only contain sections and headlines, found: {}",
-                first_cell
-            )
-            .into());
         }
     }
 

src/compare/util.rs

@@ -284,6 +284,33 @@ where
     Ok(())
 }
 
+pub(crate) fn compare_children_iter<'b, 's, RC, RI: Iterator<Item = RC> + ExactSizeIterator>(
+    source: &'s str,
+    emacs: &'b Token<'s>,
+    rust_children: RI,
+    child_status: &mut Vec<DiffEntry<'b, 's>>,
+    this_status: &mut DiffStatus,
+    message: &mut Option<String>,
+) -> Result<(), Box<dyn std::error::Error>>
+where
+    AstNode<'b, 's>: From<RC>,
+{
+    let emacs_children = emacs.as_list()?;
+    let emacs_children_length = emacs_children.len() - 2;
+    if emacs_children_length != rust_children.len() {
+        *this_status = DiffStatus::Bad;
+        *message = Some(format!(
+            "Child length mismatch (emacs != rust) {:?} != {:?}",
+            emacs_children_length,
+            rust_children.len()
+        ));
+    }
+    for (emacs_child, rust_child) in emacs_children.iter().skip(2).zip(rust_children) {
+        child_status.push(compare_ast_node(source, emacs_child, rust_child.into())?);
+    }
+    Ok(())
+}
+
 pub(crate) fn assert_no_children<'b, 's>(
     emacs: &'b Token<'s>,
     this_status: &mut DiffStatus,

src/iter/macros.rs

@@ -11,8 +11,15 @@ macro_rules! children_iter {
             fn next(&mut self) -> Option<Self::Item> {
                 self.next.next().map(Into::<AstNode>::into)
             }
+
+            fn size_hint(&self) -> (usize, Option<usize>) {
+                let size = self.next.len();
+                (size, Some(size))
+            }
         }
 
+        impl<'r, 's> ExactSizeIterator for $itertype<'r, 's> {}
+
         impl<'r, 's> IntoIterator for &'r $astnodetype {
             type Item = AstNode<'r, 's>;
 
@@ -42,8 +49,14 @@ macro_rules! empty_iter {
             fn next(&mut self) -> Option<Self::Item> {
                 None
             }
+
+            fn size_hint(&self) -> (usize, Option<usize>) {
+                (0, Some(0))
+            }
         }
 
+        impl<'r, 's> ExactSizeIterator for $itertype<'r, 's> {}
+
         impl<'r, 's> IntoIterator for &'r $astnodetype {
             type Item = AstNode<'r, 's>;
 
@@ -79,8 +92,15 @@ $fieldname: $innertype,
 .or_else(|| self.$fieldname.next().map(Into::<AstNode>::into))
 ),*
             }
+
+            fn size_hint(&self) -> (usize, Option<usize>) {
+                let size = self.$firstfieldname.len()$( + self.$fieldname.len() ),*;
+                (size, Some(size))
+            }
         }
 
+        impl<'r, 's> ExactSizeIterator for $itertype<'r, 's> {}
+
         impl<'r, 's> IntoIterator for &'r $astnodetype {
             type Item = AstNode<'r, 's>;